Skip to content

Commit 003cf9d

Browse files
committed
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
1 parent 8ec5a10 commit 003cf9d

File tree

2 files changed

+17
-13
lines changed

2 files changed

+17
-13
lines changed

ext/pcntl/pcntl.c

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,15 +1338,13 @@ static void pcntl_signal_handler(int signo, siginfo_t *siginfo, void *context)
13381338
static void pcntl_signal_handler(int signo)
13391339
#endif
13401340
{
1341-
struct php_pcntl_pending_signal *psig;
1342-
1343-
psig = PCNTL_G(spares);
1344-
if (!psig) {
1341+
struct php_pcntl_pending_signal *psig_first = PCNTL_G(spares);
1342+
if (!psig_first) {
13451343
/* oops, too many signals for us to track, so we'll forget about this one */
13461344
return;
13471345
}
13481346

1349-
struct php_pcntl_pending_signal *psig_first = psig;
1347+
struct php_pcntl_pending_signal *psig = NULL;
13501348

13511349
/* Standard signals may be merged into a single one.
13521350
* POSIX specifies that SIGCHLD has the si_pid field (https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html),
@@ -1365,28 +1363,27 @@ static void pcntl_signal_handler(int signo)
13651363
pid = waitpid(WAIT_ANY, &status, WNOHANG | WUNTRACED);
13661364
} while (pid <= 0 && errno == EINTR);
13671365
if (pid <= 0) {
1368-
if (UNEXPECTED(psig == psig_first)) {
1369-
/* Don't handle multiple, revert back to the single signal handling. */
1370-
goto single_signal;
1366+
if (UNEXPECTED(!psig)) {
1367+
/* The child might've been consumed by another thread and will be handled there. */
1368+
return;
13711369
}
13721370
break;
13731371
}
13741372

1373+
psig = psig ? psig->next : psig_first;
13751374
psig->signo = signo;
13761375

13771376
#ifdef HAVE_STRUCT_SIGINFO_T
13781377
psig->siginfo = *siginfo;
13791378
psig->siginfo.si_pid = pid;
13801379
#endif
13811380

1382-
if (EXPECTED(psig->next)) {
1383-
psig = psig->next;
1384-
} else {
1381+
if (UNEXPECTED(!psig->next)) {
13851382
break;
13861383
}
13871384
}
13881385
} else {
1389-
single_signal:;
1386+
psig = psig_first;
13901387
psig->signo = signo;
13911388

13921389
#ifdef HAVE_STRUCT_SIGINFO_T

ext/pcntl/tests/gh11498.phpt

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,11 @@ $processes = [];
1414

1515
pcntl_async_signals(true);
1616
pcntl_signal(SIGCHLD, function($sig, $info) use (&$processes) {
17+
echo "SIGCHLD\n";
1718
unset($processes[$info['pid']]);
1819
}, false);
1920

20-
foreach (range(0, 5) as $i) {
21+
for ($i = 0; $i <= 5; $i++) {
2122
$process = proc_open('echo $$ > /dev/null', [], $pipes);
2223
$pid = proc_get_status($process)['pid'];
2324
$processes[$pid] = $process;
@@ -32,4 +33,10 @@ while (!empty($processes) && $iters > 0) {
3233
var_dump(empty($processes));
3334
?>
3435
--EXPECT--
36+
SIGCHLD
37+
SIGCHLD
38+
SIGCHLD
39+
SIGCHLD
40+
SIGCHLD
41+
SIGCHLD
3542
bool(true)

0 commit comments

Comments
 (0)