Here's the updated (and simpler) version of my patch which uses apr_proc_wait() to determine whether a pid is a valid child. Simplifies the MPM logic a bit since the pid != 0 check is moved into ap_mpm_safe_kill().
Tested for both prefork and worker (on Linux) to fix the vulnerability using mod_scribble: http://people.apache.org/~jorton/mod_scribble.c Index: server/mpm/prefork/prefork.c =================================================================== --- server/mpm/prefork/prefork.c (revision 549489) +++ server/mpm/prefork/prefork.c (working copy) @@ -1127,7 +1127,7 @@ for (index = 0; index < ap_daemons_limit; ++index) { if (ap_scoreboard_image->servers[index][0].status != SERVER_DEAD) { /* Ask each child to close its listeners. */ - kill(MPM_CHILD_PID(index), AP_SIG_GRACEFUL); + ap_mpm_safe_kill(MPM_CHILD_PID(index), AP_SIG_GRACEFUL); active_children++; } } @@ -1165,12 +1165,10 @@ active_children = 0; for (index = 0; index < ap_daemons_limit; ++index) { - if (MPM_CHILD_PID(index) != 0) { - if (kill(MPM_CHILD_PID(index), 0) == 0) { - active_children = 1; - /* Having just one child is enough to stay around */ - break; - } + if (ap_mpm_safe_kill(MPM_CHILD_PID(index), 0) == APR_SUCCESS) { + active_children = 1; + /* Having just one child is enough to stay around */ + break; } } } while (!shutdown_pending && active_children && @@ -1222,7 +1220,7 @@ * piped loggers, etc. They almost certainly won't handle * it gracefully. */ - kill(ap_scoreboard_image->parent[index].pid, AP_SIG_GRACEFUL); + ap_mpm_safe_kill(ap_scoreboard_image->parent[index].pid, AP_SIG_GRACEFUL); } } } Index: server/mpm/worker/worker.c =================================================================== --- server/mpm/worker/worker.c (revision 549489) +++ server/mpm/worker/worker.c (working copy) @@ -1813,12 +1813,10 @@ active_children = 0; for (index = 0; index < ap_daemons_limit; ++index) { - if (MPM_CHILD_PID(index) != 0) { - if (kill(MPM_CHILD_PID(index), 0) == 0) { - active_children = 1; - /* Having just one child is enough to stay around */ - break; - } + if (ap_mpm_safe_kill(MPM_CHILD_PID(index), 0) == APR_SUCCESS) { + active_children = 1; + /* Having just one child is enough to stay around */ + break; } } } while (!shutdown_pending && active_children && Index: server/mpm/experimental/event/event.c =================================================================== --- server/mpm/experimental/event/event.c (revision 549489) +++ server/mpm/experimental/event/event.c (working copy) @@ -1998,12 +1998,10 @@ active_children = 0; for (index = 0; index < ap_daemons_limit; ++index) { - if (MPM_CHILD_PID(index) != 0) { - if (kill(MPM_CHILD_PID(index), 0) == 0) { - active_children = 1; - /* Having just one child is enough to stay around */ - break; - } + if (ap_mpm_safe_kill(MPM_CHILD_PID(index), 0) == APR_SUCCESS) { + active_children = 1; + /* Having just one child is enough to stay around */ + break; } } } while (!shutdown_pending && active_children && Index: server/mpm_common.c =================================================================== --- server/mpm_common.c (revision 549489) +++ server/mpm_common.c (working copy) @@ -305,6 +305,27 @@ cur_extra = next; } } + +apr_status_t ap_mpm_safe_kill(pid_t pid, int sig) +{ + apr_proc_t proc; + apr_status_t waitret; + + /* Ensure the given pid is greater than zero; passing waitpid() a + * zero or negative pid has different semantics. */ + if (pid < 1) { + return APR_EINVAL; + } + + proc.pid = pid; + waitret = apr_proc_wait(&proc, NULL, NULL, APR_NOWAIT); + if (waitret == APR_CHILD_NOTDONE) { + return kill(pid, sig) ? errno : APR_SUCCESS; + } + else { + return APR_EINVAL; + } +} #endif /* AP_MPM_WANT_RECLAIM_CHILD_PROCESSES */ #ifdef AP_MPM_WANT_WAIT_OR_TIMEOUT Index: include/mpm_common.h =================================================================== --- include/mpm_common.h (revision 549489) +++ include/mpm_common.h (working copy) @@ -145,6 +145,17 @@ #endif /** + * Safely signal an MPM child process, if the process is in the + * current process group. Otherwise fail. + * @param pid the process id of a child process to signal + * @param sig the signal number to send + * @return APR_SUCCESS if signal is sent, otherwise an error as per kill(3) + */ +#ifdef AP_MPM_WANT_RECLAIM_CHILD_PROCESSES +apr_status_t ap_mpm_safe_kill(pid_t pid, int sig); +#endif + +/** * Determine if any child process has died. If no child process died, then * this process sleeps for the amount of time specified by the MPM defined * macro SCOREBOARD_MAINTENANCE_INTERVAL.
