From 003cf9da78bb5dc1af8e6dbe07ef8abaf4e270ef Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Mon, 26 Jun 2023 11:13:40 +0200 Subject: [PATCH] Fix use of uninitialized memory in pcntl SIGCHLD handling psig needs to stay the tail, so that we don't get a dangling element on the end. Closes GH-11536 --- ext/pcntl/pcntl.c | 21 +++++++++------------ ext/pcntl/tests/gh11498.phpt | 9 ++++++++- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c index 73f8d068de6..7a04db4bcc6 100644 --- a/ext/pcntl/pcntl.c +++ b/ext/pcntl/pcntl.c @@ -1338,15 +1338,13 @@ static void pcntl_signal_handler(int signo, siginfo_t *siginfo, void *context) static void pcntl_signal_handler(int signo) #endif { - struct php_pcntl_pending_signal *psig; - - psig = PCNTL_G(spares); - if (!psig) { + struct php_pcntl_pending_signal *psig_first = PCNTL_G(spares); + if (!psig_first) { /* oops, too many signals for us to track, so we'll forget about this one */ return; } - struct php_pcntl_pending_signal *psig_first = psig; + struct php_pcntl_pending_signal *psig = NULL; /* Standard signals may be merged into a single one. * POSIX specifies that SIGCHLD has the si_pid field (https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html), @@ -1365,13 +1363,14 @@ static void pcntl_signal_handler(int signo) pid = waitpid(WAIT_ANY, &status, WNOHANG | WUNTRACED); } while (pid <= 0 && errno == EINTR); if (pid <= 0) { - if (UNEXPECTED(psig == psig_first)) { - /* Don't handle multiple, revert back to the single signal handling. */ - goto single_signal; + if (UNEXPECTED(!psig)) { + /* The child might've been consumed by another thread and will be handled there. */ + return; } break; } + psig = psig ? psig->next : psig_first; psig->signo = signo; #ifdef HAVE_STRUCT_SIGINFO_T @@ -1379,14 +1378,12 @@ static void pcntl_signal_handler(int signo) psig->siginfo.si_pid = pid; #endif - if (EXPECTED(psig->next)) { - psig = psig->next; - } else { + if (UNEXPECTED(!psig->next)) { break; } } } else { -single_signal:; + psig = psig_first; psig->signo = signo; #ifdef HAVE_STRUCT_SIGINFO_T diff --git a/ext/pcntl/tests/gh11498.phpt b/ext/pcntl/tests/gh11498.phpt index f6983c6f8d3..a9dc784ff6a 100644 --- a/ext/pcntl/tests/gh11498.phpt +++ b/ext/pcntl/tests/gh11498.phpt @@ -14,10 +14,11 @@ $processes = []; pcntl_async_signals(true); pcntl_signal(SIGCHLD, function($sig, $info) use (&$processes) { + echo "SIGCHLD\n"; unset($processes[$info['pid']]); }, false); -foreach (range(0, 5) as $i) { +for ($i = 0; $i <= 5; $i++) { $process = proc_open('echo $$ > /dev/null', [], $pipes); $pid = proc_get_status($process)['pid']; $processes[$pid] = $process; @@ -32,4 +33,10 @@ while (!empty($processes) && $iters > 0) { var_dump(empty($processes)); ?> --EXPECT-- +SIGCHLD +SIGCHLD +SIGCHLD +SIGCHLD +SIGCHLD +SIGCHLD bool(true)