fix(profiler): cover all JFR-writing signal handlers in drain

r1viollet · r1viollet · commit 97e9f99563dc · 2026-06-29T18:01:33.000+02:00
jbachorik review (profiler.cpp:1578): the inflight drain previously only
covered CTimer/CTimerJvmti, so the TOCTOU/UAF this PR closes for those
handlers remains open for the other CPU and wall fallback engines.

Add InflightGuard at signal-handler entry (after the foreign-signal
filter passes, before any JFR-writing code) for:

- PerfEvents::signalHandler         (CPU fallback, same risk shape as CTimer)
- ITimer::signalHandler             (CPU fallback when timer_create absent)
- ITimerJvmti::signalHandler        (CPU + jvmtistacks fallback)
- WallClockASGCT::sharedSignalHandler  (default wall handler)
- WallClockJvmti::sharedSignalHandler  (wall + jvmtistacks handler)

For the wall handlers the guard is placed in sharedSignalHandler — the
engine-specific inner signalHandler is dispatched from the shared scope,
so a single guard in the dispatcher covers every JFR-touching code path
in both wall variants.

Risk profile (highest to lowest probability of hitting the race in
practice): CTimer / PerfEvents (per-thread streams, ~10ms interval,
N concurrent in-flight handlers) &gt; Wall (signals from the wall pthread,
~reservoir_size concurrent) &gt; ITimer / ITimerJvmti (process-wide
setitimer, single in-flight at a time). All five plug into the same
SignalInflight counter so Profiler::stop() drains them uniformly.
diff --git a/ddprof-lib/src/main/cpp/itimer.cpp b/ddprof-lib/src/main/cpp/itimer.cpp
@@ -20,6 +20,7 @@
 #include "jvmThread.h"
 #include "os.h"
 #include "profiler.h"
+#include "signalInflight.h"
 #include "stackWalker.h"
 #include "thread.h"
 #include "threadState.inline.h"
@@ -38,6 +39,7 @@ void ITimer::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) {
   // is therefore vulnerable to the foreign-SIGPROF deadlock scenario this
   // feature addresses. Use CTimer (the default) when signal-origin
   // validation is required.
+  InflightGuard inflight;
   if (!_enabled)
     return;
   
@@ -104,6 +106,7 @@ long ITimerJvmti::_interval = 0;
 
 void ITimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) {
   SIGNAL_HANDLER_GUARD();
+  InflightGuard inflight;
   CriticalSection cs;
   if (!cs.entered()) {
     return;
diff --git a/ddprof-lib/src/main/cpp/perfEvents_linux.cpp b/ddprof-lib/src/main/cpp/perfEvents_linux.cpp
@@ -29,6 +29,7 @@
 #include "os.h"
 #include "perfEvents.h"
 #include "profiler.h"
+#include "signalInflight.h"
 #include "spinLock.h"
 #include "stackFrame.h"
 #include "stackWalker.h"
@@ -735,6 +736,7 @@ void PerfEvents::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) {
     // Looks like an external signal; don't treat as a profiling event
     return;
   }
+  InflightGuard inflight;
   // Atomically try to enter critical section - prevents all reentrancy races
   CriticalSection cs;
   if (!cs.entered()) {
diff --git a/ddprof-lib/src/main/cpp/wallClock.cpp b/ddprof-lib/src/main/cpp/wallClock.cpp
@@ -16,6 +16,7 @@
 #include "log.h"
 #include "profiler.h"
 #include "signalCookie.h"
+#include "signalInflight.h"
 #include "thread.h"
 #include "threadState.inline.h"
 #include "guards.h"
@@ -66,6 +67,10 @@ void WallClockASGCT::sharedSignalHandler(int signo, siginfo_t *siginfo,
   }
   Counters::increment(WALLCLOCK_SIGNAL_OWN);
 
+  // Past the foreign-signal filter: any work below this point can write JFR.
+  // Participate in SignalInflight::drain() so Profiler::stop() does not tear
+  // down JFR while this handler is still inside recordSample().
+  InflightGuard inflight;
   WallClockASGCT *engine = reinterpret_cast<WallClockASGCT *>(Profiler::instance()->wallEngine());
   if (signo == SIGVTALRM) {
     engine->signalHandler(signo, siginfo, ucontext, engine->_interval);
@@ -246,6 +251,10 @@ void WallClockJvmti::sharedSignalHandler(int signo, siginfo_t *siginfo,
   }
   Counters::increment(WALLCLOCK_SIGNAL_OWN);
 
+  // Past the foreign-signal filter: any work below this point can write JFR.
+  // Participate in SignalInflight::drain() so Profiler::stop() does not tear
+  // down JFR while this handler is still inside recordSampleDelegated().
+  InflightGuard inflight;
   WallClockJvmti *engine =
       reinterpret_cast<WallClockJvmti *>(Profiler::instance()->wallEngine());
   if (signo == SIGVTALRM) {
