Skip to content

Commit ea2df51

Browse files
profiling: block all (non-fault) signals on helper threads
The profiler helper threads (ddprof_time, ddprof_upload) only masked the fixed set of signals the Zend Engine registers (SIGPROF, SIGHUP, SIGINT, SIGTERM, SIGUSR1, SIGUSR2). A PHP script can install an async handler for any other signal via pcntl_async_signals(true) + pcntl_signal(), e.g. SIGCHLD. The kernel could then deliver that signal to a helper thread, where pcntl_signal_handler runs without a PHP/TSRM context and dereferences the thread-local PCNTL_G, segfaulting (ZTS only). Seen on PHP 8.4 ZTS via ext/pcntl/tests/waiting_on_sigchild_pcntl_wait.phpt. Block every signal on the helper threads (sigfillset) so async signals are delivered to a PHP thread instead, leaving only the synchronous fault signals (SIGSEGV/SIGBUS/SIGFPE/SIGILL/SIGABRT/SIGTRAP) deliverable so a real fault on a helper thread is still reported (crashtracker). Add a regression test profiling/tests/phpt/pcntl_async_signal_helper_thread.phpt. Verified: it fails ~15/20 on the old code and passes 20/20 with the fix; full profiler phpt suite still green.
1 parent e0edd47 commit ea2df51

2 files changed

Lines changed: 103 additions & 13 deletions

File tree

profiling/src/profiling/thread_utils.rs

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,25 +20,38 @@ where
2020
let result = std::thread::Builder::new()
2121
.name(name.to_string())
2222
.spawn(move || {
23-
/* Thread must not handle signals intended for PHP threads.
24-
* See Zend/zend_signal.c for which signals it registers.
23+
/* This helper thread has no valid PHP/TSRM context, so it must not
24+
* run any PHP signal handler. The Zend Engine registers a fixed set
25+
* of signals (see Zend/zend_signal.c), but a PHP script can install
26+
* a handler for *any* signal via pcntl_signal() (e.g. SIGCHLD with
27+
* pcntl_async_signals(true)). If such a signal is delivered to this
28+
* thread, pcntl_signal_handler() dereferences PCNTL_G(spares) with
29+
* no thread context and segfaults
30+
* (see ext/pcntl/tests/waiting_on_sigchild_pcntl_wait.phpt).
31+
*
32+
* So block every signal here; async signals are then delivered to a
33+
* PHP thread instead. The synchronous fault signals are left
34+
* unblocked so a genuine fault on this thread is still reported
35+
* (e.g. by the crashtracker) rather than masked.
2536
*/
2637
unsafe {
2738
let mut sigset_mem = MaybeUninit::uninit();
2839
let sigset = sigset_mem.as_mut_ptr();
29-
libc::sigemptyset(sigset);
30-
31-
const SIGNALS: [libc::c_int; 6] = [
32-
libc::SIGPROF, // todo: SIGALRM on __CYGWIN__/__PHASE__
33-
libc::SIGHUP,
34-
libc::SIGINT,
35-
libc::SIGTERM,
36-
libc::SIGUSR1,
37-
libc::SIGUSR2,
40+
libc::sigfillset(sigset);
41+
42+
// Hardware/synchronous fault signals: keep them deliverable to
43+
// this thread.
44+
const KEEP_UNBLOCKED: [libc::c_int; 6] = [
45+
libc::SIGSEGV,
46+
libc::SIGBUS,
47+
libc::SIGFPE,
48+
libc::SIGILL,
49+
libc::SIGABRT,
50+
libc::SIGTRAP,
3851
];
3952

40-
for signal in SIGNALS {
41-
libc::sigaddset(sigset, signal);
53+
for signal in KEEP_UNBLOCKED {
54+
libc::sigdelset(sigset, signal);
4255
}
4356
libc::pthread_sigmask(libc::SIG_BLOCK, sigset, std::ptr::null_mut());
4457
}
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
--TEST--
2+
[profiling] async PHP signal handlers must not run on profiler helper threads
3+
--DESCRIPTION--
4+
Regression test for a crash when a PHP script installs an async signal handler
5+
(pcntl_async_signals(true) + pcntl_signal()) for a signal the Zend Engine does
6+
not itself register, e.g. SIGCHLD.
7+
8+
The profiler's helper threads (`ddprof_time`, `ddprof_upload`) only masked the
9+
fixed set of signals the Zend Engine uses, so the kernel could deliver SIGCHLD
10+
to one of them. pcntl's handler then ran on a thread with no valid PHP/TSRM
11+
context and dereferenced the thread-local PCNTL_G, segfaulting. The fix is to
12+
block every (non-fault) signal on the helper threads so async signals are
13+
delivered to a PHP thread.
14+
15+
Only crashes on ZTS, where PCNTL_G is thread-local. Observed on PHP 8.4 ZTS,
16+
reproduced by ext/pcntl/tests/waiting_on_sigchild_pcntl_wait.phpt:
17+
18+
Thread 2 "ddprof_time" received signal SIGSEGV, Segmentation fault.
19+
#0 pcntl_signal_handler (signo=17, ...) at ext/pcntl/pcntl.c:1289
20+
1289 struct php_pcntl_pending_signal *psig = PCNTL_G(spares);
21+
#1 <signal handler called>
22+
#2 syscall ()
23+
#3 std::sys::pal::unix::futex::futex_wait ()
24+
#4 std::sys::sync::thread_parking::futex::Parker::park_timeout ()
25+
...
26+
#15 run () at profiling/src/profiling/uploader.rs:157
27+
#16 {closure#3} () at profiling/src/profiling/mod.rs:898
28+
#17 ... at profiling/src/profiling/thread_utils.rs:45
29+
--SKIPIF--
30+
<?php
31+
foreach (['datadog-profiling', 'pcntl'] as $extension)
32+
if (!extension_loaded($extension))
33+
echo "skip: test requires {$extension}\n";
34+
if (!ZEND_THREAD_SAFE)
35+
echo "skip: ZTS only (the crash is a thread-local PCNTL_G access from a helper thread)\n";
36+
if (PHP_OS_FAMILY !== 'Linux')
37+
echo "skip: Linux only\n";
38+
if (getenv('SKIP_ASAN'))
39+
die('skip: the profiler leaks on purpose in child of a fork');
40+
?>
41+
--ENV--
42+
DD_PROFILING_ENABLED=yes
43+
DD_PROFILING_LOG_LEVEL=off
44+
--FILE--
45+
<?php
46+
47+
pcntl_async_signals(true);
48+
49+
$processes = [];
50+
pcntl_signal(SIGCHLD, function () use (&$processes) {
51+
while (($pid = pcntl_wait($status, WUNTRACED | WNOHANG)) > 0) {
52+
unset($processes[$pid]);
53+
}
54+
});
55+
56+
// Spawn bursts of short-lived children. They exit at roughly the same time, so
57+
// a burst of SIGCHLD arrives while the main thread is idle in usleep(). If the
58+
// profiler's helper threads do not block SIGCHLD the kernel may deliver it to
59+
// one of them, where pcntl's handler runs without a PHP/TSRM context and
60+
// crashes. Several rounds widen the (timing-dependent) race window.
61+
for ($round = 0; $round < 4; $round++) {
62+
for ($i = 0; $i < 8; $i++) {
63+
$proc = proc_open('sleep 0.3', [], $pipes);
64+
if ($proc !== false) {
65+
$processes[proc_get_status($proc)['pid']] = $proc;
66+
}
67+
}
68+
$iters = 50;
69+
while (!empty($processes) && $iters-- > 0) {
70+
usleep(100_000);
71+
}
72+
}
73+
74+
echo empty($processes) ? "OK\n" : "leftover children\n";
75+
?>
76+
--EXPECT--
77+
OK

0 commit comments

Comments
 (0)