mirror of
https://github.com/oven-sh/bun
synced 2026-02-15 05:12:29 +00:00
fix(shell): prevent use-after-free in IOWriter when commands are freed
When shell commands (builtins, Cmd, Pipeline, etc.) complete and are freed, they may still have pending chunks in the IOWriter queue. If a write callback fires after the command is freed, it would try to call onIOWriterChunk on freed memory, causing a use-after-free crash (typically at address 0x0). This fixes the issue by canceling all pending IOWriter chunks that reference a command before freeing it. The fix adds cancelPendingChunks() calls to: - Builtin.deinit() - for all shell builtins (echo, cd, ls, etc.) - Cmd.deinit() - for external commands and builtin wrappers - Pipeline.deinit() - for pipeline commands - CondExpr.deinit() - for conditional expressions - Subshell.deinit() - for subshell commands Also adds cancelChunksWithRawPtr() to IOWriter to support canceling chunks by raw pointer address, needed for the Builtin case where we have a generic pointer to the impl union field. Closes #26660 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -668,6 +668,10 @@ pub fn deinit(this: *Builtin) void {
|
||||
// No need to free it because it belongs to the parent cmd
|
||||
// _ = Syscall.close(this.cwd);
|
||||
|
||||
// Cancel any pending IOWriter chunks that reference this builtin
|
||||
// to prevent use-after-free when the write completes.
|
||||
this.cancelPendingChunks();
|
||||
|
||||
this.stdout.deref();
|
||||
this.stderr.deref();
|
||||
this.stdin.deref();
|
||||
@@ -676,6 +680,49 @@ pub fn deinit(this: *Builtin) void {
|
||||
// this.arena.deinit();
|
||||
}
|
||||
|
||||
/// Cancel any pending IOWriter chunks that reference this builtin's impl.
|
||||
/// This prevents use-after-free crashes when a write completes after
|
||||
/// the builtin has been freed.
|
||||
fn cancelPendingChunks(this: *Builtin) void {
|
||||
const impl_ptr: usize = switch (this.kind) {
|
||||
.cat => @intFromPtr(&this.impl.cat),
|
||||
.touch => @intFromPtr(&this.impl.touch),
|
||||
.mkdir => @intFromPtr(&this.impl.mkdir),
|
||||
.@"export" => @intFromPtr(&this.impl.@"export"),
|
||||
.echo => @intFromPtr(&this.impl.echo),
|
||||
.cd => @intFromPtr(&this.impl.cd),
|
||||
.which => @intFromPtr(&this.impl.which),
|
||||
.rm => @intFromPtr(&this.impl.rm),
|
||||
.pwd => @intFromPtr(&this.impl.pwd),
|
||||
.mv => @intFromPtr(&this.impl.mv),
|
||||
.ls => @intFromPtr(&this.impl.ls),
|
||||
.exit => @intFromPtr(&this.impl.exit),
|
||||
.true => @intFromPtr(&this.impl.true),
|
||||
.false => @intFromPtr(&this.impl.false),
|
||||
.yes => @intFromPtr(&this.impl.yes),
|
||||
.seq => @intFromPtr(&this.impl.seq),
|
||||
.dirname => @intFromPtr(&this.impl.dirname),
|
||||
.basename => @intFromPtr(&this.impl.basename),
|
||||
.cp => @intFromPtr(&this.impl.cp),
|
||||
};
|
||||
|
||||
// Cancel chunks in stdout writer if it's an fd
|
||||
switch (this.stdout) {
|
||||
.fd => |fd| {
|
||||
fd.writer.cancelChunksWithRawPtr(impl_ptr);
|
||||
},
|
||||
else => {},
|
||||
}
|
||||
|
||||
// Cancel chunks in stderr writer if it's an fd
|
||||
switch (this.stderr) {
|
||||
.fd => |fd| {
|
||||
fd.writer.cancelChunksWithRawPtr(impl_ptr);
|
||||
},
|
||||
else => {},
|
||||
}
|
||||
}
|
||||
|
||||
/// If the stdout/stderr is supposed to be captured then get the bytelist associated with that
|
||||
pub fn stdBufferedBytelist(this: *Builtin, comptime io_kind: @Type(.enum_literal)) ?*bun.ByteList {
|
||||
if (comptime io_kind != .stdout and io_kind != .stderr) {
|
||||
|
||||
@@ -252,7 +252,12 @@ pub fn cancelChunks(this: *IOWriter, ptr_: anytype) void {
|
||||
ChildPtr => ptr_,
|
||||
else => ChildPtr.init(ptr_),
|
||||
};
|
||||
const actual_ptr = ptr.ptr.repr._ptr;
|
||||
this.cancelChunksWithRawPtr(ptr.ptr.repr._ptr);
|
||||
}
|
||||
|
||||
/// Cancel chunks by raw pointer address. Used when we have an opaque
|
||||
/// pointer and need to cancel chunks without type information.
|
||||
pub fn cancelChunksWithRawPtr(this: *IOWriter, actual_ptr: usize) void {
|
||||
if (this.writers.len() == 0) return;
|
||||
const idx = this.writer_idx;
|
||||
const slice: []Writer = this.writers.sliceMutable();
|
||||
|
||||
@@ -726,11 +726,27 @@ pub fn deinit(this: *Cmd) void {
|
||||
this.spawn_arena.deinit();
|
||||
}
|
||||
|
||||
// Cancel any pending IOWriter chunks that reference this Cmd
|
||||
// to prevent use-after-free when the write completes.
|
||||
this.cancelPendingChunks();
|
||||
|
||||
this.io.deref();
|
||||
this.base.endScope();
|
||||
this.parent.destroy(this);
|
||||
}
|
||||
|
||||
/// Cancel any pending IOWriter chunks that reference this Cmd.
|
||||
fn cancelPendingChunks(this: *Cmd) void {
|
||||
switch (this.io.stderr) {
|
||||
.fd => |fd| fd.writer.cancelChunks(this),
|
||||
else => {},
|
||||
}
|
||||
switch (this.io.stdout) {
|
||||
.fd => |fd| fd.writer.cancelChunks(this),
|
||||
else => {},
|
||||
}
|
||||
}
|
||||
|
||||
pub fn bufferedInputClose(this: *Cmd) void {
|
||||
this.exec.subproc.buffered_closed.close(this, .stdin);
|
||||
}
|
||||
|
||||
@@ -211,6 +211,10 @@ fn doStat(this: *CondExpr) Yield {
|
||||
}
|
||||
|
||||
pub fn deinit(this: *CondExpr) void {
|
||||
// Cancel any pending IOWriter chunks that reference this CondExpr
|
||||
// to prevent use-after-free when the write completes.
|
||||
this.cancelPendingChunks();
|
||||
|
||||
this.io.deinit();
|
||||
for (this.args.items) |item| {
|
||||
this.base.allocator().free(item);
|
||||
@@ -220,6 +224,18 @@ pub fn deinit(this: *CondExpr) void {
|
||||
this.parent.destroy(this);
|
||||
}
|
||||
|
||||
/// Cancel any pending IOWriter chunks that reference this CondExpr.
|
||||
fn cancelPendingChunks(this: *CondExpr) void {
|
||||
switch (this.io.stderr) {
|
||||
.fd => |fd| fd.writer.cancelChunks(this),
|
||||
else => {},
|
||||
}
|
||||
switch (this.io.stdout) {
|
||||
.fd => |fd| fd.writer.cancelChunks(this),
|
||||
else => {},
|
||||
}
|
||||
}
|
||||
|
||||
pub fn childDone(this: *CondExpr, child: ChildPtr, exit_code: ExitCode) Yield {
|
||||
if (child.ptr.is(Expansion)) {
|
||||
if (exit_code != 0) {
|
||||
|
||||
@@ -274,11 +274,28 @@ pub fn deinit(this: *Pipeline) void {
|
||||
if (this.cmds) |cmds| {
|
||||
this.base.allocator().free(cmds);
|
||||
}
|
||||
|
||||
// Cancel any pending IOWriter chunks that reference this Pipeline
|
||||
// to prevent use-after-free when the write completes.
|
||||
this.cancelPendingChunks();
|
||||
|
||||
this.io.deref();
|
||||
this.base.endScope();
|
||||
this.parent.destroy(this);
|
||||
}
|
||||
|
||||
/// Cancel any pending IOWriter chunks that reference this Pipeline.
|
||||
fn cancelPendingChunks(this: *Pipeline) void {
|
||||
switch (this.io.stderr) {
|
||||
.fd => |fd| fd.writer.cancelChunks(this),
|
||||
else => {},
|
||||
}
|
||||
switch (this.io.stdout) {
|
||||
.fd => |fd| fd.writer.cancelChunks(this),
|
||||
else => {},
|
||||
}
|
||||
}
|
||||
|
||||
fn initializePipes(pipes: []Pipe, set_count: *u32) Maybe(void) {
|
||||
for (pipes) |*pipe| {
|
||||
if (bun.Environment.isWindows) {
|
||||
|
||||
@@ -172,12 +172,29 @@ pub fn onIOWriterChunk(this: *Subshell, _: usize, err: ?jsc.SystemError) Yield {
|
||||
|
||||
pub fn deinit(this: *Subshell) void {
|
||||
this.base.shell.deinit();
|
||||
|
||||
// Cancel any pending IOWriter chunks that reference this Subshell
|
||||
// to prevent use-after-free when the write completes.
|
||||
this.cancelPendingChunks();
|
||||
|
||||
this.io.deref();
|
||||
this.redirection_file.deinit();
|
||||
this.base.endScope();
|
||||
this.parent.destroy(this);
|
||||
}
|
||||
|
||||
/// Cancel any pending IOWriter chunks that reference this Subshell.
|
||||
fn cancelPendingChunks(this: *Subshell) void {
|
||||
switch (this.io.stderr) {
|
||||
.fd => |fd| fd.writer.cancelChunks(this),
|
||||
else => {},
|
||||
}
|
||||
switch (this.io.stdout) {
|
||||
.fd => |fd| fd.writer.cancelChunks(this),
|
||||
else => {},
|
||||
}
|
||||
}
|
||||
|
||||
pub fn writeFailingError(this: *Subshell, comptime fmt: []const u8, args: anytype) Yield {
|
||||
const handler = struct {
|
||||
fn enqueueCb(ctx: *Subshell) void {
|
||||
|
||||
109
test/regression/issue/26660.test.ts
Normal file
109
test/regression/issue/26660.test.ts
Normal file
@@ -0,0 +1,109 @@
|
||||
import { expect, test } from "bun:test";
|
||||
import { bunEnv, bunExe, tempDir } from "harness";
|
||||
|
||||
// Test for issue #26660: Shell interpreter crash when spawning many concurrent shell commands
|
||||
// The bug was that IOWriter could have pending chunks referencing freed memory after
|
||||
// builtin/cmd cleanup, causing use-after-free crashes.
|
||||
test("many concurrent shell commands should not crash", async () => {
|
||||
// Create a test script that spawns many shell commands concurrently
|
||||
// This is similar to what OpenCode does which triggered the original crash
|
||||
using dir = tempDir("issue-26660", {
|
||||
"test.ts": `
|
||||
const promises: Promise<void>[] = [];
|
||||
|
||||
// Spawn 60 shell commands concurrently (similar to the crash report)
|
||||
for (let i = 0; i < 60; i++) {
|
||||
promises.push(
|
||||
Bun.$\`echo "Hello from shell \${i}"\`.text().then(() => {})
|
||||
);
|
||||
}
|
||||
|
||||
await Promise.all(promises);
|
||||
console.log("All shell commands completed successfully");
|
||||
`,
|
||||
});
|
||||
|
||||
await using proc = Bun.spawn({
|
||||
cmd: [bunExe(), "run", "test.ts"],
|
||||
cwd: String(dir),
|
||||
env: bunEnv,
|
||||
stderr: "pipe",
|
||||
stdout: "pipe",
|
||||
});
|
||||
|
||||
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
|
||||
|
||||
expect(stderr).not.toContain("Segmentation fault");
|
||||
expect(stderr).not.toContain("panic");
|
||||
expect(stdout).toContain("All shell commands completed successfully");
|
||||
expect(exitCode).toBe(0);
|
||||
});
|
||||
|
||||
test("concurrent shell commands with output should not crash", async () => {
|
||||
// Test with commands that produce output, which exercises IOWriter more
|
||||
using dir = tempDir("issue-26660-output", {
|
||||
"test.ts": `
|
||||
const promises: Promise<string>[] = [];
|
||||
|
||||
// Spawn many echo commands that all write to IOWriter
|
||||
for (let i = 0; i < 40; i++) {
|
||||
promises.push(
|
||||
Bun.$\`echo "Line \${i}: This is a test with some longer output to exercise the IOWriter buffer"\`.text()
|
||||
);
|
||||
}
|
||||
|
||||
const results = await Promise.all(promises);
|
||||
console.log("Collected", results.length, "results");
|
||||
console.log("All outputs received correctly");
|
||||
`,
|
||||
});
|
||||
|
||||
await using proc = Bun.spawn({
|
||||
cmd: [bunExe(), "run", "test.ts"],
|
||||
cwd: String(dir),
|
||||
env: bunEnv,
|
||||
stderr: "pipe",
|
||||
stdout: "pipe",
|
||||
});
|
||||
|
||||
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
|
||||
|
||||
expect(stderr).not.toContain("Segmentation fault");
|
||||
expect(stderr).not.toContain("panic");
|
||||
expect(stdout).toContain("All outputs received correctly");
|
||||
expect(exitCode).toBe(0);
|
||||
});
|
||||
|
||||
test("shell commands with mixed builtins should not crash", async () => {
|
||||
// Test various builtins that use IOWriter
|
||||
using dir = tempDir("issue-26660-builtins", {
|
||||
"test.ts": `
|
||||
const promises: Promise<void>[] = [];
|
||||
|
||||
// Spawn various builtin commands concurrently
|
||||
for (let i = 0; i < 20; i++) {
|
||||
promises.push(Bun.$\`echo "Echo \${i}"\`.text().then(() => {}));
|
||||
promises.push(Bun.$\`pwd\`.text().then(() => {}));
|
||||
promises.push(Bun.$\`true\`.text().then(() => {}));
|
||||
}
|
||||
|
||||
await Promise.all(promises);
|
||||
console.log("All builtin commands completed");
|
||||
`,
|
||||
});
|
||||
|
||||
await using proc = Bun.spawn({
|
||||
cmd: [bunExe(), "run", "test.ts"],
|
||||
cwd: String(dir),
|
||||
env: bunEnv,
|
||||
stderr: "pipe",
|
||||
stdout: "pipe",
|
||||
});
|
||||
|
||||
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
|
||||
|
||||
expect(stderr).not.toContain("Segmentation fault");
|
||||
expect(stderr).not.toContain("panic");
|
||||
expect(stdout).toContain("All builtin commands completed");
|
||||
expect(exitCode).toBe(0);
|
||||
});
|
||||
Reference in New Issue
Block a user