Fix SIGBUS when freeing memfd-backed Blob store#29155
Conversation
On Linux, large Blobs (>8MB) are backed by a memfd via mmap. When the Blob is finalized, Bytes.deinit calls allocator.free() which in safe builds first does @Memset(ptr, undefined) before calling the vtable free. For a memfd mapping whose backing file has been truncated (e.g. by user code calling fs.ftruncate on the fd, or by a raw fd passed to Bun.file being reused for the memfd and then operated on by an in-flight CopyFile task), writing past the file's EOF raises SIGBUS. Use rawFree() to skip the poisoning memset. The memory is about to be munmap'd so poisoning it buys nothing and is expensive for large Blobs regardless.
|
Updated 4:32 PM PT - Apr 12th, 2026
❌ @robobun, your commit 3f4e3ba has 1 failures in
🧪 To try this PR locally: bunx bun-pr 29155That installs a local version of the PR into your bun-29155 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughClears Bytes ownership on handoff and adjusts Blob store deallocation to avoid allocator memset on non-default allocators; adds Linux-only memfd-truncate test and two new copy-on-write Blob tests that validate data remains after GC. Changes
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/js/web/fetch/blob-memfd-truncate.test.ts`:
- Line 44: Replace the combined object assertion with separate assertions so
stdout is checked first: call expect(stdout).toBe("ok\n") before asserting
exitCode and signal; then assert expect(exitCode).toBe(0) and
expect(proc.signalCode).toBeNull() (or the existing signal expectation). Update
the assertion around the variables stdout, exitCode, and proc.signalCode instead
of using expect({ stdout, exitCode, signal: proc.signalCode }).toEqual(...).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b089eca9-f1a4-4c41-bdef-82bf3f553629
📒 Files selected for processing (2)
src/bun.js/webcore/blob/Store.zigtest/js/web/fetch/blob-memfd-truncate.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
- Null out ptr in toInternalBlob() after transferring ownership; previously it relied on Allocator.free()'s zero-length early return, which rawFree() lacks. - Guard cap > 0 in Bytes.deinit() to match the previous Allocator.free() contract. - Only use rawFree() for non-default allocators (memfd/mmap); keep free() with its debug-mode poisoning for ordinary heap-backed blobs. - Split test assertions so stdout is checked before exit code.
| pub fn deinit(this: *Bytes) void { | ||
| bun.default_allocator.free(this.stored_name.slice()); | ||
| if (this.ptr) |ptr| { | ||
| this.allocator.free(ptr[0..this.cap]); | ||
| if (this.cap > 0) { | ||
| if (this.allocator.vtable == bun.default_allocator.vtable) { | ||
| this.allocator.free(ptr[0..this.cap]); | ||
| } else { | ||
| // rawFree skips Allocator.free's @memset(ptr, undefined). | ||
| // For memfd/mmap-backed stores, writing to the mapping can | ||
| // SIGBUS if the backing file was truncated from under us. | ||
| this.allocator.rawFree(ptr[0..this.cap], .fromByteUnits(1), @returnAddress()); | ||
| } | ||
| } | ||
| } | ||
| this.ptr = null; | ||
| this.len = 0; |
There was a problem hiding this comment.
🔴 The fix correctly patches Store.Bytes.deinit() but leaves Blob.Internal.deinit() (Blob.zig:4585) unpatched. When a large blob is consumed via blob.arrayBuffer()/blob.text()/blob.json(), toInternalBlobIfPossible() converts it to a Blob.Internal whose bytes.allocator is still LinuxMemFdAllocator; Blob.Internal.deinit() calls bytes.clearAndFree() -> Allocator.free(), which in Debug/ReleaseSafe builds performs @Memset(mmap_ptr, 0xaa, capacity) before munmap. If the memfd was truncated between toInternalBlob() and the InternalBlob being freed, this @Memset writes past the new EOF and raises SIGBUS — the exact crash the PR claims to fix. The test only exercises the direct GC path (blob = null; Bun.gc(true)) and misses this consumption path entirely.
Extended reasoning...
Incomplete Fix: Blob.Internal.deinit() Still SIGBUS-Vulnerable
What the bug is
The PR fixes Store.Bytes.deinit() to use rawFree() for non-default allocators, correctly skipping the @Memset(ptr, undefined) poisoning that triggers SIGBUS when the memfd backing file is truncated. However, there is a second deallocation path for memfd-backed blobs that the PR does not address: Blob.Internal.deinit() in Blob.zig at line 4585.
The specific code path that triggers it
When a large (>=8MB) blob is consumed via blob.arrayBuffer(), blob.text(), or blob.json(), the engine calls toActionValue() -> toInternalBlobIfPossible(). Because hasOneRef() is true, this calls this.data.bytes.toInternalBlob(), which transfers ownership of the mmap buffer (including its LinuxMemFdAllocator) to a Blob.Internal struct, then derefs the store. The resulting Blob.Internal holds bytes.allocator = LinuxMemFdAllocator. When this Blob.Internal is later freed, Blob.Internal.deinit() calls this.bytes.clearAndFree() -> std.ArrayList.Managed.clearAndFree() -> self.allocator.free(self.allocatedSlice()).
Why existing code does not prevent it
The PR only modified Store.zig. Blob.Internal.deinit() in Blob.zig was not changed. Zig high-level Allocator.free() performs @Memset(buf, undefined) in Debug/ReleaseSafe builds before dispatching to the vtable rawFree. This is identical to the pattern the PR fixed in Store.Bytes.deinit(), but this call site remains. If the memfd was truncated between toInternalBlob() and clearAndFree(), the @Memset writes past the truncated EOF and raises SIGBUS.
Impact
Any consumption of a large memfd-backed blob via arrayBuffer(), text(), or json() in a Debug or ReleaseSafe build can SIGBUS if the memfd is truncated concurrently. ReleaseFast production builds are not affected because config.safety is false and Zig skips the @Memset. The PR test exercises only the direct GC path, so this path is not covered.
Step-by-step proof
- let blob = new Blob(new SharedArrayBuffer(16 * 1024 * 1024)); — creates a Store.Bytes backed by a memfd/mmap via LinuxMemFdAllocator.
- Externally truncate the memfd (e.g. via /proc/self/fd as in the test).
- Call await blob.json() — this calls toActionValue() -> toInternalBlobIfPossible() -> Store.Bytes.toInternalBlob(), producing a Blob.Internal with bytes.allocator = LinuxMemFdAllocator and bytes.capacity = 1610241024.
- Blob.Internal.deinit() is called after the JSON parse: this.bytes.clearAndFree() -> LinuxMemFdAllocator.free(mmap_ptr[0..16MB]).
- In safe builds, Zig Allocator.free() first does @Memset(mmap_ptr, 0xaa, 16MB) — this write accesses memory past the truncated EOF — SIGBUS.
How to fix
Apply the same rawFree-vs-free dispatch to Blob.Internal.deinit() that was applied to Store.Bytes.deinit(). Either check the allocator vtable against bun.default_allocator.vtable (mirroring the pattern in this PR), or use LinuxMemFdAllocator.isInstance() which already exists for this purpose. The fix should also be tested with a variant that calls blob.text() after truncating the memfd, not just blob = null; Bun.gc(true).
There was a problem hiding this comment.
This is a valid observation about Blob.Internal.deinit(), but I'm going to leave it out of scope for this PR:
-
Blob.prototype.text()/.json()/.arrayBuffer()go throughgetText/getTextClone, nottoActionValue→toInternalBlobIfPossible(). ThetoInternalBlobIfPossible()path is only reached viaAny.toActionValuefor Response/Request body consumption. -
Even on that path, consuming the blob reads the mapping first. If the memfd is already truncated, the read SIGBUSes (verified —
simdutf::validate_ascii_with_errorsfaults on the read). ForInternal.deinit()to SIGBUS, the truncation would have to land in the microsecond window between the read completing andclearAndFree()running, both sequential on the main thread. -
There are many places that read from memfd-backed mappings. Hardening all of them against concurrent truncation is a broader effort (SIGBUS handler → JS error) and not what the fuzzer found.
This PR fixes the specific crash the fuzzer reproduced: Store.Bytes.deinit() during GC finalization, where the Blob is being freed without any intent to access its contents. That is both the only path the fuzzer hit and the only path where the @memset serves no purpose (the data is never read again).
|
CI failures are pre-existing flakes unrelated to this change. This PR only touches Build #45056 (bd655c5):
Build #45375 (3f4e3ba): all of the above passed on retry. Only remaining failure:
|
Same root cause as the SIGBUS (Allocator.free's @Memset(undefined) on the MAP_SHARED store mapping), different symptom: outstanding MAP_PRIVATE views from blob.arrayBuffer()/blob.bytes() read 0xAA after the blob store is collected. Fuzzer fingerprint d0cddd0241b6b110.
|
Pushed 3f4e3ba: additional regression tests for another symptom of the same |
There was a problem hiding this comment.
LGTM — all prior feedback addressed: ownership-transfer fix ( in ), guard, vtable-based -vs- total used free shared buff/cache available
Mem: 32956368 2616112 29676900 3324 1054524 30340256
Swap: 0 0 0 dispatch, and additional MAP_PRIVATE regression tests in the final commit.
Extended reasoning...
Overview
The PR fixes a SIGBUS crash in on Linux when a memfd-backed Blob's backing file is truncated before GC finalization. Three files were modified: (core fix), and two test files (, ).
Prior Review Iterations
My first review identified two bugs introduced by the initial fix: (1) did not null after transferring ownership, leaving with a non-null ptr + zero cap — with having no zero-length guard this caused UB/double-free; and (2) was applied unconditionally to all allocators, bypassing debug poisoning even for mimalloc-backed blobs. Both were fixed in bd655c5: was added to , a guard was added in , and the -vs- total used free shared buff/cache available
Mem: 32956368 2616112 29676900 3324 1054524 30340256
Swap: 0 0 0 dispatch is now gated on a vtable comparison against .
My second review raised a potential residual path via . The author explained convincingly that (1) // do not go through ; (2) even on the path that does, a truncated memfd would SIGBUS during the read, not the free; and (3) hardening all memfd read paths is a separate effort. This is a reasonable scoping decision.
Security Risks
No security risks — this is a crash-safety fix in memory deallocation for a Linux-specific large-blob optimization path. The vtable comparison is a standard Zig pattern and does not introduce type confusion.
Level of Scrutiny
The change is small and focused: three logical changes to plus one line in . The regression tests (including the ASAN-passing subprocess test and the new MAP_PRIVATE COW tests) provide concrete coverage of the fix. CI failures are documented as pre-existing flakes on unrelated test files.
Other Factors
The fix is well-understood, bounded in scope, and backed by a fuzzer fingerprint + deterministic repro. All reviewer feedback was addressed across two iterations. No bugs were found by the automated bug hunting system.
1 participant
What does this PR do?
On Linux, large Blobs (>8MB) are backed by a
memfdviammap. When the Blob is finalized,Store.Bytes.deinitcallsallocator.free()which in safe builds first does@memset(ptr, undefined)before calling the vtable'sfree. For a memfd mapping whose backing file has been truncated, writing past the file's EOF raisesSIGBUS.This was found by Fuzzilli at commit 1cc8376 with fingerprint
Address:BUS:libc.so.6+0x173593. The fuzzer script did roughly:The async
CopyFiletask captured fd 14.memfd_createfromnew Blob(...)reused fd 14. TheCopyFiletask then ran on the thread pool,fstat'd /copy_file_range'd /ftruncate'd fd 14, shrinking the memfd. Later, GC finalized the Blob and@memset(mapping, 0xaa, cap)hitSIGBUSpast the new EOF.A simpler deterministic repro:
Fix
Call
rawFree()directly instead offree()inBytes.deinit, skipping the poisoning@memset. The memory is about to bemunmap'd so poisoning buys nothing, and for large Blobs the memset is expensive anyway.How did you verify your code works?
/proc/self/fd, drops the Blob and forces GC. Before this change the subprocess dies withSIGBUS; after, it exits cleanly.blob.test.tsandblob-cow.test.tsstill pass.