You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ni...@apache.org on 2008/12/27 03:13:47 UTC
svn commit: r729579 - in /httpd/httpd/trunk: CHANGES
modules/generators/mod_cgid.c
Author: niq
Date: Fri Dec 26 18:13:47 2008
New Revision: 729579
URL: http://svn.apache.org/viewvc?rev=729579&view=rev
Log:
PR#39332: fix for segfault problem with mod_cgid on Solaris
Patch by Masaoki Kobayashi
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/generators/mod_cgid.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=729579&r1=729578&r2=729579&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Dec 26 18:13:47 2008
@@ -2,6 +2,9 @@
Changes with Apache 2.3.1
[ When backported to 2.2.x, remove entry from this file ]
+ *) mod_cgid: fix segfault problem on solaris.
+ PR 39332 [Masaoki Kobayashi <masaoki techfirm.co.jp>]
+
*) mod_proxy_scgi: Added. [André Malo]
*) mod_cache: Introduce 'no-cache' per-request environment variable
@@ -17,7 +20,7 @@
PR 46380 [Ruediger Pluem]
*) scoreboard: Remove unused sb_type from process_score.
- [Torsten Foertsch <to...@gmx.net>, Chris Darroch]
+ [Torsten Foertsch <torsten.foertsch gmx.net>, Chris Darroch]
*) mod_ssl: Add SSLRenegBufferSize directive to allow changing the
size of the buffer used for the request-body where necessary
Modified: httpd/httpd/trunk/modules/generators/mod_cgid.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/generators/mod_cgid.c?rev=729579&r1=729578&r2=729579&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/generators/mod_cgid.c (original)
+++ httpd/httpd/trunk/modules/generators/mod_cgid.c Fri Dec 26 18:13:47 2008
@@ -342,6 +342,33 @@
return APR_SUCCESS;
}
+static apr_status_t sock_writev(int fd, request_rec *r, int count, ...)
+{
+ va_list ap;
+ int rc;
+ struct iovec *vec;
+ int i;
+ int total_bytes = 0;
+
+ vec = (struct iovec *)apr_palloc(r->pool, count * sizeof(struct iovec));
+ va_start(ap, count);
+ for(i=0; i<count; i++) {
+ vec[i].iov_base = va_arg(ap, caddr_t);
+ vec[i].iov_len = va_arg(ap, int);
+ total_bytes += vec[i].iov_len;
+ }
+ va_end(ap);
+
+ do {
+ rc = writev(fd, vec, count);
+ } while (rc < 0 && errno == EINTR);
+ if (rc < 0) {
+ return errno;
+ }
+
+ return APR_SUCCESS;
+}
+
static apr_status_t get_req(int fd, request_rec *r, char **argv0, char ***env,
cgid_req_t *req)
{
@@ -470,31 +497,31 @@
req.loglevel = r->server->loglevel;
/* Write the request header */
- if ((stat = sock_write(fd, &req, sizeof(req))) != APR_SUCCESS) {
- return stat;
+ if (req.args_len) {
+ stat = sock_writev(fd, r, 5,
+ &req, sizeof(req),
+ r->filename, req.filename_len,
+ argv0, req.argv0_len,
+ r->uri, req.uri_len,
+ r->args, req.args_len);
+ } else {
+ stat = sock_writev(fd, r, 4,
+ &req, sizeof(req),
+ r->filename, req.filename_len,
+ argv0, req.argv0_len,
+ r->uri, req.uri_len);
}
- /* Write filename, argv0, uri, and args */
- if ((stat = sock_write(fd, r->filename, req.filename_len)) != APR_SUCCESS ||
- (stat = sock_write(fd, argv0, req.argv0_len)) != APR_SUCCESS ||
- (stat = sock_write(fd, r->uri, req.uri_len)) != APR_SUCCESS) {
+ if (stat != APR_SUCCESS) {
return stat;
}
- if (req.args_len) {
- if ((stat = sock_write(fd, r->args, req.args_len)) != APR_SUCCESS) {
- return stat;
- }
- }
/* write the environment variables */
for (i = 0; i < req.env_count; i++) {
apr_size_t curlen = strlen(env[i]);
- if ((stat = sock_write(fd, &curlen, sizeof(curlen))) != APR_SUCCESS) {
- return stat;
- }
-
- if ((stat = sock_write(fd, env[i], curlen)) != APR_SUCCESS) {
+ if ((stat = sock_writev(fd, r, 2, &curlen, sizeof(curlen),
+ env[i], curlen)) != APR_SUCCESS) {
return stat;
}
}
Re: svn commit: r729579 - in /httpd/httpd/trunk: CHANGES modules/generators/mod_cgid.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Tue, Jan 6, 2009 at 11:28 AM, Justin Erenkrantz <ju...@erenkrantz.com>wrote:
> On Tue, Jan 6, 2009 at 7:11 AM, Jeff Trawick <tr...@gmail.com> wrote:
> > AFAIK it is resolved by 120664-01 or later (or x86-equiv patch), from
> > several years ago...
>
> Yah, we already have configure checks for this. I don't like adding
> unnecessary complexity to work around an OS bug that we've detected
> and flagged for years. -- justin
>
I'm happy to see some syscalls disappear, but this patch only goes so far.
--
Born in Roswell... married an alien...
Re: svn commit: r729579 - in /httpd/httpd/trunk: CHANGES modules/generators/mod_cgid.c
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Tue, Jan 6, 2009 at 7:11 AM, Jeff Trawick <tr...@gmail.com> wrote:
> AFAIK it is resolved by 120664-01 or later (or x86-equiv patch), from
> several years ago...
Yah, we already have configure checks for this. I don't like adding
unnecessary complexity to work around an OS bug that we've detected
and flagged for years. -- justin
Re: svn commit: r729579 - in /httpd/httpd/trunk: CHANGES modules/generators/mod_cgid.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Sat, Dec 27, 2008 at 8:12 AM, Nick Kew <ni...@webthing.com> wrote:
>
> On 27 Dec 2008, at 09:52, Ruediger Pluem wrote:
>
>
>>
>> On 12/27/2008 03:13 AM, niq@apache.org wrote:
>>
>>> Author: niq
>>> Date: Fri Dec 26 18:13:47 2008
>>> New Revision: 729579
>>>
>>> URL: http://svn.apache.org/viewvc?rev=729579&view=rev
>>> Log:
>>> PR#39332: fix for segfault problem with mod_cgid on Solaris
>>> Patch by Masaoki Kobayashi
>>>
>>> Modified:
>>> httpd/httpd/trunk/CHANGES
>>> httpd/httpd/trunk/modules/generators/mod_cgid.c
>>>
>>
>> Just for my understanding. This patch works around a bug in Solaris with
>> write
>> and UNIX domain sockets, right?
>> Otherwise I wouldn't get why the new code using writev would be better
>> thatn the
>> old one using write.
>>
>
> It's a bug with a number of reports, though I haven't actually been able
> to reproduce it myself on opensolaris.
AFAIK it is resolved by 120664-01 or later (or x86-equiv patch), from
several years ago...
Re: svn commit: r729579 - in /httpd/httpd/trunk: CHANGES
modules/generators/mod_cgid.c
Posted by Joe Orton <jo...@redhat.com>.
On Sat, Dec 27, 2008 at 01:12:24PM +0000, Nick Kew wrote:
> On 27 Dec 2008, at 09:52, Ruediger Pluem wrote:
>> On 12/27/2008 03:13 AM, niq@apache.org wrote:
>>> Author: niq
>>> Date: Fri Dec 26 18:13:47 2008
>>> New Revision: 729579
>>>
>>> URL: http://svn.apache.org/viewvc?rev=729579&view=rev
>>> Log:
>>> PR#39332: fix for segfault problem with mod_cgid on Solaris
>>> Patch by Masaoki Kobayashi
>>>
>>> Modified:
>>> httpd/httpd/trunk/CHANGES
>>> httpd/httpd/trunk/modules/generators/mod_cgid.c
>>
>> Just for my understanding. This patch works around a bug in Solaris
>> with write and UNIX domain sockets, right? Otherwise I wouldn't get
>> why the new code using writev would be better thatn the old one using
>> write.
>
> It's a bug with a number of reports, though I haven't actually been able
> to reproduce it myself on opensolaris.
>
> Rationale for the patch is that I reviewed it and ruled out the risk of a
> regression(!)
Ah, a zero-risk-of-regression patch, I'll just quote you on that :)
Given that as Justin mentioned, there is already a configure check to
avoid building cgid on Solaris systems which have the bug in question, I
think this leaves unresolved questions:
1) is the configure check not sufficient to catch the set of systems
which have a kernel bug?
2) is there some other bug (cgid or Solaris or whatever) at large here?
I'd rather not see this applied unless this is sorted out, since it
clearly does *not* have a zero risk of regression.
Regards, Joe
Re: svn commit: r729579 - in /httpd/httpd/trunk: CHANGES modules/generators/mod_cgid.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 12/27/2008 02:12 PM, Nick Kew wrote:
>
> On 27 Dec 2008, at 09:52, Ruediger Pluem wrote:
>
>>
>>
>> On 12/27/2008 03:13 AM, niq@apache.org wrote:
>>> Author: niq
>>> Date: Fri Dec 26 18:13:47 2008
>>> New Revision: 729579
>>>
>>> URL: http://svn.apache.org/viewvc?rev=729579&view=rev
>>> Log:
>>> PR#39332: fix for segfault problem with mod_cgid on Solaris
>>> Patch by Masaoki Kobayashi
>>>
>>> Modified:
>>> httpd/httpd/trunk/CHANGES
>>> httpd/httpd/trunk/modules/generators/mod_cgid.c
>>
>> Just for my understanding. This patch works around a bug in Solaris
>> with write
>> and UNIX domain sockets, right?
>> Otherwise I wouldn't get why the new code using writev would be better
>> thatn the
>> old one using write.
>
> It's a bug with a number of reports, though I haven't actually been able
> to reproduce it myself on opensolaris.
>
> Rationale for the patch is that I reviewed it and ruled out the risk of a
> regression(!) So if it fixes a severe bug that affects some users,
> then on balance it's a Good Thing.
Never questioned that it was a good to fix the bug. I just wanted to understand
the patch a little bit better and that the real root cause that makes
this patch necessary is a bug in Solaris and not one that is originally in
the httpd code.
Regards
Rüdiger
Re: svn commit: r729579 - in /httpd/httpd/trunk: CHANGES modules/generators/mod_cgid.c
Posted by Nick Kew <ni...@webthing.com>.
On 27 Dec 2008, at 09:52, Ruediger Pluem wrote:
>
>
> On 12/27/2008 03:13 AM, niq@apache.org wrote:
>> Author: niq
>> Date: Fri Dec 26 18:13:47 2008
>> New Revision: 729579
>>
>> URL: http://svn.apache.org/viewvc?rev=729579&view=rev
>> Log:
>> PR#39332: fix for segfault problem with mod_cgid on Solaris
>> Patch by Masaoki Kobayashi
>>
>> Modified:
>> httpd/httpd/trunk/CHANGES
>> httpd/httpd/trunk/modules/generators/mod_cgid.c
>
> Just for my understanding. This patch works around a bug in Solaris
> with write
> and UNIX domain sockets, right?
> Otherwise I wouldn't get why the new code using writev would be
> better thatn the
> old one using write.
It's a bug with a number of reports, though I haven't actually been able
to reproduce it myself on opensolaris.
Rationale for the patch is that I reviewed it and ruled out the risk
of a
regression(!) So if it fixes a severe bug that affects some users,
then on balance it's a Good Thing.
--
Nick Kew
Re: svn commit: r729579 - in /httpd/httpd/trunk: CHANGES modules/generators/mod_cgid.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 12/27/2008 03:13 AM, niq@apache.org wrote:
> Author: niq
> Date: Fri Dec 26 18:13:47 2008
> New Revision: 729579
>
> URL: http://svn.apache.org/viewvc?rev=729579&view=rev
> Log:
> PR#39332: fix for segfault problem with mod_cgid on Solaris
> Patch by Masaoki Kobayashi
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/generators/mod_cgid.c
Just for my understanding. This patch works around a bug in Solaris with write
and UNIX domain sockets, right?
Otherwise I wouldn't get why the new code using writev would be better thatn the
old one using write.
Regards
Rüdiger