httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ralf S. Engelschall" <...@engelschall.com>
Subject Re: suexec+CGI = zombies in 1.3.28
Date Tue, 29 Jul 2003 19:16:28 GMT
On Sun, Jul 20, 2003, Cliff Woolley wrote:

> For those not watching the bugzilla notices, I direct your attention to
> bug 21737:
>
> http://nagoya.apache.org/bugzilla/show_bug.cgi?id=21737
>
> Apparently there has been a regression in 1.3.28 from 1.3.27 whereby
> CGI scripts are getting left around as zombies when suexec is in use,
> apparently because of a change in src/main/alloc.c that altered the
> behavior when sending SIGTERM to a child process.  With suexec, the
> SIGTERM at line 2862 will fail not because the subprocess is dead already
> but because the httpd uid has no permission to term the cgi process, which
> is running as some other user.
>
> A suggested patch posted by one of the users is below.  It doesn't look
> portable to me, but the idea seems sound.
>
> Thoughts?
> Cliff
>
>
> --- apache_1.3.28/src/main/alloc.c.orig	Sun Jul 20 14:30:30 2003
> +++ apache_1.3.28/src/main/alloc.c	Sun Jul 20 14:33:50 2003
> @@ -2860,7 +2860,14 @@
>  	    || (p->kill_how == kill_only_once)) {
>  	    /* Subprocess may be dead already.  Only need the timeout if not. */
>  	    if (ap_os_kill(p->pid, SIGTERM) == -1) {
> -                p->kill_how = kill_never;
> +		/* If the kill failed, find out why.  If the process does
> +		   not exist then we do not need to kill it.  */
> +		if (errno == ESRCH) {
> +	                p->kill_how = kill_never;
> +		}
> +		else {
> +			need_timeout = 1;
> +		}
>              }
>              else {
>  		need_timeout = 1;

Because we're currently releasing OpenPKG 1.3 with Apache 1.3.28
included, we had to investigate today on this issue and finally think
although the above patch works and could be comitted this way, more
correct and straight-foreward would be the simple:

Index: alloc.c
===================================================================
RCS file: /e/apache/cvs/apache-1.3/src/main/alloc.c,v
retrieving revision 1.145
diff -u -d -r1.145 alloc.c
--- alloc.c	20 Jun 2003 15:05:40 -0000	1.145
+++ alloc.c	29 Jul 2003 19:07:46 -0000
@@ -2859,12 +2859,8 @@
 	if ((p->kill_how == kill_after_timeout)
 	    || (p->kill_how == kill_only_once)) {
 	    /* Subprocess may be dead already.  Only need the timeout if not. */
-	    if (ap_os_kill(p->pid, SIGTERM) == -1) {
-                p->kill_how = kill_never;
-            }
-            else {
-		need_timeout = 1;
-            }
+	    ap_os_kill(p->pid, SIGTERM);
+	    need_timeout = 1;
 	}
 	else if (p->kill_how == kill_always) {
 	    kill(p->pid, SIGKILL);

That is, we don't have to check for the return value of ap_os_kill()
and especially not check for ESRCH, because we _HAVE_ to waitpid() for
it anyway (because it's our child and it either is already terminated
and is waiting as a zombie for our waitpid() or it is still running).

Under Unix it cannot be that a (non-detached in the sense of BSD's
daemon(3)) child of a process just does no longer exists as long as
the parent still exists and as long as the parent still has not done
waitpid() for the child. So ESRCH cannot happen in our situation and the
patch we currently use is fully sufficient. Both are at least portable
enough for Unix, of course...
                                       Ralf S. Engelschall
                                       rse@engelschall.com
                                       www.engelschall.com


Mime
View raw message