Skip to content

fuzzilli: chain crash signal handlers to JSC/ASAN instead of SIG_DFL #30092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 4 commits into from
Closed

fuzzilli: chain crash signal handlers to JSC/ASAN instead of SIG_DFL #30092

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e71477b
fuzzilli: chain crash signal handlers to JSC/ASAN instead of SIG_DFL
robobun May 2, 2026
3cbb692
test: verify fuzzilli SIGSEGV handler chains to ASAN
robobun May 2, 2026
9394842
[autofix.ci] apply automated fixes
autofix-ci[bot] May 2, 2026
696a1f4
fuzzilli: install signal handlers once; deliver on sigaltstack
robobun May 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 36 additions & 9 deletions src/bun.js/bindings/FuzzilliREPRL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,46 @@

extern "C" {

// Signal handler to ensure output is flushed before crash
static void fuzzilliSignalHandler(int sig)
// Previous handlers (JSC's jscSignalHandler, which itself chains to ASAN's
// handler). We flush stdio so Fuzzilli captures buffered output, then forward
// to the prior handler so JSC's VMTraps/WASM fault handling keeps working and
// ASAN can print its report. Using signal()+SIG_DFL here used to swallow both,
// leaving crash reports with just "TERMSIG: 11" and no stack.
static struct sigaction fuzzilliOldActions[NSIG];

static void fuzzilliSignalHandler(int sig, siginfo_t* info, void* ucontext)
{
// Flush all output
fflush(stdout);
fflush(stderr);
fsync(STDOUT_FILENO);
fsync(STDERR_FILENO);
Comment on lines 32 to 36

@coderabbitai coderabbitai Bot May 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Is fflush async-signal-safe according to POSIX, and what is the recommended way to ensure crash output is visible from a signal handler?

💡 Result:

No, fflush is not async-signal-safe according to POSIX. The official POSIX list of async-signal-safe functions does not include fflush or any stdio functions like printf. Instead, use low-level write to STDERR_FILENO (fd 2) or STDOUT_FILENO (fd 1) for output in signal handlers, followed by fsync if needed to ensure visibility, then _exit. This avoids stdio buffering issues and ensures the output is flushed to the kernel without relying on process termination cleanup, which may not occur reliably on crashes like SIGSEGV.

Citations:

Replace fflush() calls with async-signal-safe write() to avoid deadlock in signal handler.

The fflush(stdout) and fflush(stderr) calls at lines 31–35 are not async-signal-safe. If the signal interrupts code that holds the stdio lock, the handler will deadlock before the chained JSC/ASAN handler runs, defeating the purpose of this patch. Use write(STDOUT_FILENO, ...) and write(STDERR_FILENO, ...) instead; the fsync() calls that follow are async-signal-safe and can remain.

Current code 
    // Flush all output
    fflush(stdout);
    fflush(stderr);
    fsync(STDOUT_FILENO);
    fsync(STDERR_FILENO);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/bun.js/bindings/FuzzilliREPRL.cpp` around lines 31 - 35, Replace the
non-async-signal-safe fflush(stdout) and fflush(stderr) calls with
async-signal-safe write() calls: write(STDOUT_FILENO, <buffer>, <len>) and
write(STDERR_FILENO, <buffer>, <len>) using the appropriate output buffers or a
small newline/flush string; keep the existing fsync(STDOUT_FILENO) and
fsync(STDERR_FILENO) calls. Locate the block containing fflush(stdout);
fflush(stderr); fsync(STDOUT_FILENO); fsync(STDERR_FILENO) in FuzzilliREPRL.cpp
and change the two fflush calls to write calls (using STDOUT_FILENO and
STDERR_FILENO) so the handler remains async-signal-safe.

@robobun robobun May 2, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fflush() calls are pre-existing (present before this PR) and intentional — the whole point of this handler is to drain stdio's userspace buffers so Fuzzilli captures buffered output before the process dies. write(STDOUT_FILENO, …) can only push bytes we already hold; it cannot flush FILE* buffers, so swapping them in would make the handler a no-op for its stated purpose.

The async-signal-safety risk (deadlock if we crash while holding stdio's lock) is accepted here: the process is terminating regardless, and the next handler in the chain (WTF::jscSignalHandler, ASAN's reporter) is already non-signal-safe — they allocate, walk stacks, and write through stdio themselves. This is the standard crash-reporting trade-off.

@coderabbitai coderabbitai Bot May 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robobun, that's a fair and well-reasoned explanation. You're right that write() cannot drain FILE* userspace buffers, so substituting it would silently drop exactly the buffered output Fuzzilli needs to see. And the signal-safety concern is moot in practice here because the process is already on its way out and the downstream handlers (WTF::jscSignalHandler, ASAN) are themselves non-signal-safe by necessity. This is the standard crash-reporter trade-off. Happy to dismiss this comment.

(・ω・)っ🐇

✏️ Learnings added 
Learnt from: robobun
Repo: oven-sh/bun PR: 30092
File: src/bun.js/bindings/FuzzilliREPRL.cpp:31-35
Timestamp: 2026-05-02T02:52:13.970Z
Learning: In `src/bun.js/bindings/FuzzilliREPRL.cpp` (oven-sh/bun), the `fuzzilliSignalHandler` intentionally calls `fflush(stdout)` and `fflush(stderr)` before forwarding to the chained handler. This is by design: the purpose is to drain `FILE*` userspace buffers so Fuzzilli captures buffered output before the process dies. `write(STDOUT_FILENO, …)` cannot flush `FILE*` buffers and would be a no-op for this purpose. The async-signal-safety risk (deadlock if the signal fires while the stdio lock is held) is an accepted trade-off because (a) the process is terminating regardless, and (b) the downstream chained handlers (`WTF::jscSignalHandler`, ASAN's reporter) are themselves non-async-signal-safe. Do not flag these `fflush()` calls as unsafe in future reviews.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: robobun
Repo: oven-sh/bun PR: 29389
File: src/bun.js/bindings/V8HeapSnapshotBuilder.cpp:592-594
Timestamp: 2026-04-17T04:59:15.762Z
Learning: When reviewing C++ code that uses WebKit Template Framework (WTF)/JSC types, do not treat `if (str.isEmpty()) str = String();` as dead code or a no-op. In WTF, `WTF::String::isEmpty()` returns true for both null strings and empty non-null strings, while `String()` constructs a null string. This conditional therefore normalizes empty non-null strings into null strings and may change behavior for downstream callers that distinguish null vs empty—so it should not be flagged as redundant solely on that basis.

Learnt from: robobun
Repo: oven-sh/bun PR: 29867
File: packages/bun-uws/src/App.h:284-285
Timestamp: 2026-04-28T22:49:27.619Z
Learning: When reviewing C++ move constructors/assignments, do not require calling `.clear()` on objects that are being moved-from when they are `std::vector` (or other standard containers) members. Moving a `std::vector` transfers ownership safely; the moved-from vector remains in a valid state and should not cause double-free by itself. Only flag issues if there are non-standard ownership/raw-pointer/double-destruction patterns beyond the default `std::vector` move behavior.


// Re-raise the signal with default handler
struct sigaction& old = fuzzilliOldActions[sig];
if (old.sa_flags & SA_SIGINFO) {
if (old.sa_sigaction) {
old.sa_sigaction(sig, info, ucontext);
return;
}
} else if (old.sa_handler && old.sa_handler != SIG_DFL && old.sa_handler != SIG_IGN) {
old.sa_handler(sig);
return;
}

// No previous handler: re-raise with default handler.
signal(sig, SIG_DFL);
raise(sig);
}

static void installFuzzilliSignalHandler(int sig)
{
struct sigaction action;
action.sa_sigaction = fuzzilliSignalHandler;
sigfillset(&action.sa_mask);
action.sa_flags = SA_SIGINFO;

Check warning on line 58 in src/bun.js/bindings/FuzzilliREPRL.cpp

View check run for this annotation

Claude / Claude Code Review

SA_ONSTACK not set — stack-overflow SIGSEGV still yields bare TERMSIG 11 

Consider adding `SA_ONSTACK` here (`action.sa_flags = SA_SIGINFO | SA_ONSTACK`). ASAN/JSC set up a sigaltstack and install their SIGSEGV handlers with `SA_ONSTACK` so they can run when the fault is a stack overflow; without it, `fuzzilliSignalHandler` is delivered on the already-exhausted stack, immediately re-faults, and the process dies with a bare `TERMSIG: 11` — the symptom this PR is eliminating. Not a regression (the old `signal()` path also lacked it), but it's a one-token fix in code bei

@claude claude Bot May 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Consider adding SA_ONSTACK here (action.sa_flags = SA_SIGINFO | SA_ONSTACK). ASAN/JSC set up a sigaltstack and install their SIGSEGV handlers with SA_ONSTACK so they can run when the fault is a stack overflow; without it, fuzzilliSignalHandler is delivered on the already-exhausted stack, immediately re-faults, and the process dies with a bare TERMSIG: 11 — the symptom this PR is eliminating. Not a regression (the old signal() path also lacked it), but it's a one-token fix in code being rewritten here.

Extended reasoning...

What the bug is

installFuzzilliSignalHandler() installs the new chaining handler with action.sa_flags = SA_SIGINFO, omitting SA_ONSTACK. This means the kernel will deliver SIGSEGV by pushing a signal frame onto the current thread stack rather than the alternate signal stack.

For most faults that's fine. But when the SIGSEGV is caused by stack exhaustion (the guard page below the thread stack is hit), there is no room left on the current stack to push the signal frame. The kernel's attempt to set up the handler invocation itself faults, the kernel gives up and applies the default disposition, and the process dies with a bare SIGSEGV — no fflush, no chain to jscSignalHandler, no ASAN report. That's exactly the "TERMSIG: 11 with nothing on stderr" symptom this PR exists to eliminate.

Why the alt stack is already there

Both layers this PR is chaining to already account for this:

  • ASAN's MaybeInstallSigaction installs its deadly-signal handler with SA_SIGINFO | SA_NODEFER | SA_ONSTACK and calls SetAlternateSignalStack() per thread (default use_sigaltstack=1). This file already #include <sanitizer/asan_interface.h>, so the fuzzilli build is running under ASAN with the alt stack available.
  • JSC's WTF::SignalHandlers::add installs jscSignalHandler with SA_ONSTACK for the same reason.
  • Bun's own crash_handler.zig does the same (sigaltstack + SA.ONSTACK).

So the alternate stack already exists at the point installFuzzilliSignalHandler runs — the new outermost handler simply isn't opting into it. By installing without SA_ONSTACK, the fuzzilli wrapper effectively removes stack-overflow handling that the inner layers had.

Step-by-step proof

  1. Fuzzer generates deeply recursive JS (or native code recurses past JSC's soft stack check — JSC's check catches most JS recursion as RangeError, but native frames under a JS call can blow through it).
  2. Thread stack grows into the guard page → hardware fault → kernel raises SIGSEGV.
  3. Kernel looks up the disposition: fuzzilliSignalHandler, flags SA_SIGINFO (no SA_ONSTACK).
  4. Kernel tries to push siginfo_t + ucontext_t + return trampoline onto the current (exhausted) stack → hits the guard page again.
  5. Fault during signal delivery → kernel force-applies SIG_DFL → process killed with TERMSIG: 11.
  6. fuzzilliSignalHandler never ran → no flush. jscSignalHandler never ran. AsanOnDeadlySignal never ran → no report.

Result: Fuzzilli logs a crash fingerprint with no output — indistinguishable from the bug class this PR is fixing.

Why this isn't a regression

The pre-PR code used signal(SIGSEGV, ...), which on glibc maps to sigaction with SA_RESTART and also lacks SA_ONSTACK. So stack-overflow crashes were equally undiagnosable before. This is a pre-existing limitation in code the PR is actively rewriting, not new breakage.

Fix

One token:

action.sa_flags = SA_SIGINFO | SA_ONSTACK;

(Optionally also | SA_NODEFER to match ASAN, but SA_ONSTACK is the load-bearing one for this case.)

Given the PR's stated goal is "if this fingerprint re-fires it will now come with an ASAN stack", and stack overflow is a common fuzzer finding, it seems worth folding in here.

@robobun robobun May 2, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added SA_ONSTACK in 696a1f4.

sigaction(sig, &action, &fuzzilliOldActions[sig]);
}
Comment on lines +54 to +64

@coderabbitai coderabbitai Bot May 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, verify the file exists and read the relevant section
head -70 src/bun.js/bindings/FuzzilliREPRL.cpp | tail -25

Repository: oven-sh/bun

Length of output: 1030

🏁 Script executed:

# Search for sigaction usage in the codebase to see error handling patterns
rg "sigaction" --type cpp -B2 -A5

Repository: oven-sh/bun

Length of output: 10135

🏁 Script executed:

# Check for similar error handling patterns with system calls in C++ files
rg "if.*!=.*0\)" src/bun.js/bindings --type cpp -B2 -A2 | head -40

Repository: oven-sh/bun

Length of output: 2960

🏁 Script executed:

# Look for struct initialization patterns in the file
rg "struct.*=" src/bun.js/bindings/FuzzilliREPRL.cpp --type cpp

Repository: oven-sh/bun

Length of output: 153

Initialize struct sigaction and handle sigaction() errors.

The function leaves struct sigaction uninitialized and ignores sigaction() return values, creating two issues: uninitialized struct fields can cause undefined behavior when chained in fuzzilliSignalHandler, and silent failures disable handler installation. Other signal handling code in the codebase (c-bindings.cpp, SigintWatcher.cpp, BunProcess.cpp) consistently zero-initializes sigaction structs using either aggregate initialization or memset(), and c-bindings.cpp checks the sigaction() return value.

Proposed fix 
 static void installFuzzilliSignalHandler(int sig)
 {
-    struct sigaction action;
+    struct sigaction action = {};
     action.sa_sigaction = fuzzilliSignalHandler;
     sigfillset(&action.sa_mask);
     // SA_ONSTACK so stack-overflow faults are delivered on the sigaltstack
     // ASAN/JSC already set up; otherwise the kernel can't push the frame and
     // the process dies with a bare SIGSEGV before we reach the chain.
     action.sa_flags = SA_SIGINFO | SA_ONSTACK;
-    sigaction(sig, &action, &fuzzilliOldActions[sig]);
+    if (sigaction(sig, &action, &fuzzilliOldActions[sig]) != 0) {
+        fprintf(stderr, "[fuzzilli] sigaction(%d) failed: %s\n", sig, strerror(errno));
+        std::abort();
+    }
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/bun.js/bindings/FuzzilliREPRL.cpp` around lines 54 - 64, The
installFuzzilliSignalHandler function leaves its local struct sigaction 'action'
uninitialized and ignores the return value of sigaction(), which can cause UB
and silent failures; fix it by zero-initializing the struct (e.g.,
aggregate-init or memset) before setting fields, ensure sa_mask is initialized
(you can keep sigfillset), set sa_sigaction=fuzzilliSignalHandler and
sa_flags=SA_SIGINFO|SA_ONSTACK as done now, then call sigaction(sig, &action,
&fuzzilliOldActions[sig]) and check its return value, logging or handling errors
on failure (consistent with other files like c-bindings.cpp) so handler
installation failures aren’t ignored.


// Implementation of the global fuzzilli() function for Bun
// This function is used by Fuzzilli to:
// 1. Test crash detection with fuzzilli('FUZZILLI_CRASH', type)
Expand Down Expand Up @@ -259,12 +285,13 @@
{
JSC::VM& vm = globalObject->vm();

// Install signal handlers to ensure output is flushed before crashes
// This is important for ASAN output to be captured
signal(SIGABRT, fuzzilliSignalHandler);
signal(SIGSEGV, fuzzilliSignalHandler);
signal(SIGILL, fuzzilliSignalHandler);
signal(SIGFPE, fuzzilliSignalHandler);
// Install signal handlers to ensure output is flushed before crashes.
// Chain to the previous handler (JSC's jscSignalHandler → ASAN) so
// VMTraps/WASM fault handling keeps working and ASAN reports are printed.
installFuzzilliSignalHandler(SIGABRT);
installFuzzilliSignalHandler(SIGSEGV);
installFuzzilliSignalHandler(SIGILL);
installFuzzilliSignalHandler(SIGFPE);

Check failure on line 294 in src/bun.js/bindings/FuzzilliREPRL.cpp

View check run for this annotation

Claude / Claude Code Review

Re-installing handlers per global object causes infinite recursion and loses JSC/ASAN chain 

`Bun__REPRL__registerFuzzilliFunctions()` is called from `Zig__GlobalObject__create` (ZigGlobalObject.cpp:518) for **every** global object — including Workers — so the first `new Worker(...)` re-runs `installFuzzilliSignalHandler()`, and `sigaction()` returns `fuzzilliSignalHandler` itself as the previous handler, overwriting `fuzzilliOldActions[sig]`. This permanently loses the saved JSC/ASAN chain (back to bare `TERMSIG: 11`) and makes any subsequent signal recurse infinitely into itself. Guar

@claude claude Bot May 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Bun__REPRL__registerFuzzilliFunctions() is called from Zig__GlobalObject__create (ZigGlobalObject.cpp:518) for every global object — including Workers — so the first new Worker(...) re-runs installFuzzilliSignalHandler(), and sigaction() returns fuzzilliSignalHandler itself as the previous handler, overwriting fuzzilliOldActions[sig]. This permanently loses the saved JSC/ASAN chain (back to bare TERMSIG: 11) and makes any subsequent signal recurse infinitely into itself. Guard installation with a static once-flag, or skip the save when the returned sa_sigaction == fuzzilliSignalHandler.

Extended reasoning...

What the bug is

installFuzzilliSignalHandler() saves the previous handler into the static fuzzilliOldActions[sig] so that fuzzilliSignalHandler can chain to it. This is correct the first time it runs. But the function is not idempotent: if it runs a second time, sigaction() returns the current handler — which is now fuzzilliSignalHandler itself, with SA_SIGINFO set — and overwrites the saved JSC/ASAN handler with a self-reference.

Code path that triggers it

Bun__REPRL__registerFuzzilliFunctions() is invoked unconditionally at ZigGlobalObject.cpp:518 inside Zig__GlobalObject__create, which per its own header comment (lines 427–429) runs for the main thread (executionContextId == -1), macros (maxInt32), and Workers (> -1). The Worker-init branch at line 534 confirms Worker globals flow through this same function and reach line 518 first. Signal dispositions are process-wide, and fuzzilliOldActions[] is a file-scope static, so the Worker thread's call mutates the same state the main thread relies on.

JSC's Config::finalize() / WTF signal-handler installation uses call_once and runs once per process, not per-VM, so the Worker's VM creation does not re-install jscSignalHandler between the two calls. The handler in place when the Worker calls sigaction() is therefore fuzzilliSignalHandler.

Why nothing prevents it

There is no once-guard around the installation, and installFuzzilliSignalHandler() does not check whether the returned old action is already its own handler. The old signal()-based code was harmlessly idempotent under repeated installation (it just wrote the same function pointer); the new chaining code is stateful and is actively corrupted by re-installation — a regression introduced by this PR.

Step-by-step proof

  1. Main global is created → installFuzzilliSignalHandler(SIGSEGV) runs. sigaction() installs fuzzilliSignalHandler and writes the previous handler (WTF::jscSignalHandler, SA_SIGINFO set) into fuzzilliOldActions[SIGSEGV]. ✅
  2. Fuzzilli input executes new Worker(...) → Zig__GlobalObject__create(executionContextId > -1) → line 518 → installFuzzilliSignalHandler(SIGSEGV) runs again.
  3. sigaction() returns the current handler — { .sa_sigaction = fuzzilliSignalHandler, .sa_flags = SA_SIGINFO } — into &fuzzilliOldActions[SIGSEGV], overwriting the saved jscSignalHandler.
  4. Later, a SIGSEGV fires. fuzzilliSignalHandler flushes, reads old = fuzzilliOldActions[SIGSEGV], sees old.sa_flags & SA_SIGINFO and old.sa_sigaction != nullptr, and calls old.sa_sigaction(sig, info, ucontext) — i.e. calls itself.
  5. This is a direct function call, not signal redelivery, so sa_mask does not block it. The handler recurses until the stack overflows.

Impact

After the first Worker is spawned in a REPRL session: (a) the JSC→ASAN chain is permanently lost, so crashes go back to bare TERMSIG: 11 with no ASAN report — defeating the purpose of this PR; and (b) JSC's signal-based VMTraps/WASM fault handling is broken again; and (c) any crash signal triggers infinite self-recursion. The PR description's "100 iterations interleaving new Worker(...).terminate()" test only verified that no spurious crash occurred — it didn't deliver a real crash signal after a Worker existed, so the corrupted chain was never exercised.

Fix

Either guard the installation block with a static once-flag (signal handlers are process-wide, so installing once is correct), e.g.:

static bool installed = false;
if (!std::exchange(installed, true)) {
    installFuzzilliSignalHandler(SIGABRT);
    installFuzzilliSignalHandler(SIGSEGV);
    installFuzzilliSignalHandler(SIGILL);
    installFuzzilliSignalHandler(SIGFPE);
}

or, inside installFuzzilliSignalHandler, capture the old action into a temporary and skip the save when tmp.sa_sigaction == fuzzilliSignalHandler.

@robobun robobun May 2, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — confirmed: before the fix, new Worker(...) followed by fuzzilli("FUZZILLI_CRASH", 5) died with bare SIGSEGV (exit 139, no ASAN output). Wrapped the install block in std::call_once in 696a1f4 and added a test that creates a Worker before triggering the null deref.


globalObject->putDirectNativeFunction(
vm,
Expand Down
Loading