mirrors/bun.sh
1
0
Fork 0
mirror of https://github.com/oven-sh/bun synced 2026-02-02 15:08:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: protect StringOrBuffer from GC in async operations (#25594)

Browse Source 
## Summary

- Fix use-after-free crash in async zstd compression, scrypt, and
JSTranspiler operations
- When `StringOrBuffer.fromJSMaybeAsync` is called with `is_async=true`,
the buffer's JSValue is now protected from garbage collection
- Previously, the buffer could be GC'd while a worker thread was still
accessing it, causing segfaults in zstd's `HIST_count_simple` and
similar functions

Fixes BUN-167Z

## Changes

- `fromJSMaybeAsync`: Call `protect()` on buffer when `is_async=true`
- `fromJSWithEncodingMaybeAsync`: Same protection for the early return
path
- `Scrypt`: Fix cleanup to use `deinitAndUnprotect()` for async path,
add missing `deinit()` in sync path
- `JSTranspiler`: Use new protection mechanism instead of manual
`protect()`/`unprotect()` calls
- Simplify `createOnJSThread` signatures to not return errors (OOM is
handled internally)
- Update all callers to use renamed/simplified APIs

## Test plan

- [x] Code review of all callsites to verify correct protect/unprotect
pairing
- [ ] Run existing zstd tests
- [ ] Run existing scrypt tests
- [ ] Run existing transpiler tests

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Dylan Conway
2025-12-19 17:30:26 -08:00
committed by GitHub
parent ce5c336ea5
commit 085e25d5d1
12 changed files with 65 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
src/bun.js/api/JSTranspiler.zig
View File

@@ -453,7 +453,7 @@ pub const TransformTask = struct {
pub const AsyncTransformTask = jsc.ConcurrentPromiseTask(TransformTask);
pub const AsyncTransformEventLoopTask = AsyncTransformTask.EventLoopTask;
pub fn create(transpiler: *JSTranspiler, input_code: bun.jsc.Node.StringOrBuffer, globalThis: *JSGlobalObject, loader: Loader) !*AsyncTransformTask {
pub fn create(transpiler: *JSTranspiler, input_code: bun.jsc.Node.StringOrBuffer, globalThis: *JSGlobalObject, loader: Loader) *AsyncTransformTask {
var transform_task = TransformTask.new(.{
.input_code = input_code,
.transpiler = transpiler.transpiler,
@@ -474,8 +474,7 @@ pub const TransformTask = struct {
transform_task.transpiler.setAllocator(bun.default_allocator);
transpiler.ref();
errdefer transform_task.deinit();
return try AsyncTransformTask.createOnJSThread(bun.default_allocator, globalThis, transform_task);
return AsyncTransformTask.createOnJSThread(bun.default_allocator, globalThis, transform_task);
}
pub fn run(this: *TransformTask) void {
@@ -847,7 +846,7 @@ pub fn transform(this: *JSTranspiler, globalThis: *jsc.JSGlobalObject, callframe
var code = try jsc.Node.StringOrBuffer.fromJSWithEncodingMaybeAsync(globalThis, bun.default_allocator, code_arg, .utf8, true, allow_string_object) orelse {
return globalThis.throwInvalidArgumentType("transform", "code", "string or Uint8Array");
};
errdefer code.deinit();
errdefer code.deinitAndUnprotect();
args.eat();
const loader: ?Loader = brk: {
@@ -859,21 +858,12 @@ pub fn transform(this: *JSTranspiler, globalThis: *jsc.JSGlobalObject, callframe
break :brk null;
};
if (code == .buffer) {
code_arg.protect();
}
var task = TransformTask.create(
this,
code,
globalThis,
loader orelse this.config.default_loader,
) catch {
if (code == .buffer) {
code_arg.unprotect();
}
globalThis.throwOutOfMemory();
return error.JSError;
};
);
task.schedule();
return task.promise.value();
}

2
src/bun.js/api/bun/dns.zig
View File

@@ -155,7 +155,7 @@ const LibC = struct {
) catch |err| bun.handleOom(err);
const promise_value = request.head.promise.value();
var io = bun.handleOom(GetAddrInfoRequest.Task.createOnJSThread(this.vm.allocator, globalThis, request));
var io = GetAddrInfoRequest.Task.createOnJSThread(this.vm.allocator, globalThis, request);
io.schedule();
this.requestSent(globalThis.bunVM());

4
src/bun.js/api/bun/socket.zig
View File

@@ -929,7 +929,7 @@ pub fn NewSocket(comptime ssl: bool) type {
const buffer: jsc.Node.StringOrBuffer = if (data_value.isUndefined())
jsc.Node.StringOrBuffer.empty
else
jsc.Node.StringOrBuffer.fromJSWithEncodingValueMaybeAsync(globalObject, stack_fallback.get(), data_value, encoding_value, false, allow_string_object) catch {
jsc.Node.StringOrBuffer.fromJSWithEncodingValueAllowStringObject(globalObject, stack_fallback.get(), data_value, encoding_value, allow_string_object) catch {
return .fail;
} orelse {
if (!globalObject.hasException()) {
@@ -1071,7 +1071,7 @@ pub fn NewSocket(comptime ssl: bool) type {
const buffer: jsc.Node.BlobOrStringOrBuffer = if (args[0].isUndefined())
jsc.Node.BlobOrStringOrBuffer{ .string_or_buffer = jsc.Node.StringOrBuffer.empty }
else
jsc.Node.BlobOrStringOrBuffer.fromJSWithEncodingValueMaybeAsyncAllowRequestResponse(globalObject, stack_fallback.get(), args[0], encoding_value, false, true) catch {
jsc.Node.BlobOrStringOrBuffer.fromJSWithEncodingValueAllowRequestResponse(globalObject, stack_fallback.get(), args[0], encoding_value, true) catch {
return .fail;
} orelse {
if (!globalObject.hasException()) {

2
src/bun.js/api/glob.zig
View File

@@ -158,7 +158,7 @@ pub const WalkTask = struct {
.alloc = alloc,
.has_pending_activity = has_pending_activity,
};
return try AsyncGlobWalkTask.createOnJSThread(alloc, globalThis, walkTask);
return AsyncGlobWalkTask.createOnJSThread(alloc, globalThis, walkTask);
}
pub fn run(this: *WalkTask) void {

2
src/bun.js/event_loop/ConcurrentPromiseTask.zig
View File

@@ -21,7 +21,7 @@ pub fn ConcurrentPromiseTask(comptime Context: type) type {
pub const new = bun.TrivialNew(@This());
pub fn createOnJSThread(allocator: std.mem.Allocator, globalThis: *jsc.JSGlobalObject, value: *Context) !*This {
pub fn createOnJSThread(allocator: std.mem.Allocator, globalThis: *jsc.JSGlobalObject, value: *Context) *This {
var this = This.new(.{
.event_loop = VirtualMachine.get().event_loop,
.ctx = value,

2
src/bun.js/event_loop/WorkTask.zig
View File

@@ -27,7 +27,7 @@ pub fn WorkTask(comptime Context: type) type {
// This is a poll because we want it to enter the uSockets loop
ref: Async.KeepAlive = .{},
pub fn createOnJSThread(allocator: std.mem.Allocator, globalThis: *jsc.JSGlobalObject, value: *Context) !*This {
pub fn createOnJSThread(allocator: std.mem.Allocator, globalThis: *jsc.JSGlobalObject, value: *Context) *This {
var vm = globalThis.bunVM();
var this = bun.new(This, .{
.event_loop = vm.eventLoop(),

27
src/bun.js/node/node_crypto_binding.zig
View File

@@ -536,12 +536,26 @@ const Scrypt = struct {
const password = try Node.StringOrBuffer.fromJSMaybeAsync(global, bun.default_allocator, password_value, is_async, true) orelse {
return global.throwInvalidArgumentTypeValue("password", "string, ArrayBuffer, Buffer, TypedArray, or DataView", password_value);
};
errdefer password.deinit();
errdefer {
if (is_async) {
password.deinitAndUnprotect();
} else {
password.deinit();
}
}
const salt = try Node.StringOrBuffer.fromJSMaybeAsync(global, bun.default_allocator, salt_value, is_async, true) orelse {
return global.throwInvalidArgumentTypeValue("salt", "string, ArrayBuffer, Buffer, TypedArray, or DataView", salt_value);
};
errdefer salt.deinit();
errdefer {
if (is_async) {
salt.deinitAndUnprotect();
} else {
salt.deinit();
}
}
const keylen = try validators.validateInt32(global, keylen_value, "keylen", .{}, 0, null);
@@ -719,6 +733,14 @@ const Scrypt = struct {
}
fn deinit(this: *Scrypt) void {
this.salt.deinitAndUnprotect();
this.password.deinitAndUnprotect();
this.buf.deinit();
}
fn deinitSync(this: *Scrypt) void {
this.salt.deinit();
this.password.deinit();
this.buf.deinit();
}
};
@@ -731,6 +753,7 @@ fn scrypt(global: *JSGlobalObject, callFrame: *jsc.CallFrame) JSError!JSValue {
fn scryptSync(global: *JSGlobalObject, callFrame: *jsc.CallFrame) JSError!JSValue {
var ctx = try Scrypt.fromJS(global, callFrame, false);
defer ctx.deinitSync();
const buf, const bytes = try jsc.ArrayBuffer.alloc(global, .ArrayBuffer, ctx.keylen);
ctx.runTask(bytes);
return buf;

4
src/bun.js/node/node_fs.zig
View File

@@ -2829,7 +2829,9 @@ pub const Arguments = struct {
// String objects not allowed (typeof new String("hi") === "object")
// https://github.com/nodejs/node/blob/6f946c95b9da75c70e868637de8161bc8d048379/lib/internal/fs/utils.js#L916
const allow_string_object = false;
const data = try StringOrBuffer.fromJSWithEncodingMaybeAsync(ctx, bun.default_allocator, data_value, encoding, arguments.will_be_async, allow_string_object) orelse {
// the pattern in node_fs.zig is to call toThreadSafe after Arguments.*.fromJS
const is_async = false;
const data = try StringOrBuffer.fromJSWithEncodingMaybeAsync(ctx, bun.default_allocator, data_value, encoding, is_async, allow_string_object) orelse {
return ctx.ERR(.INVALID_ARG_TYPE, "The \"data\" argument must be of type string or an instance of Buffer, TypedArray, or DataView", .{}).throw();
};

31
src/bun.js/node/types.zig
View File

@@ -68,14 +68,10 @@ pub const BlobOrStringOrBuffer = union(enum) {
}
pub fn fromJSWithEncodingValue(global: *jsc.JSGlobalObject, allocator: std.mem.Allocator, value: jsc.JSValue, encoding_value: jsc.JSValue) bun.JSError!?BlobOrStringOrBuffer {
return fromJSWithEncodingValueMaybeAsync(global, allocator, value, encoding_value, false);
return fromJSWithEncodingValueAllowRequestResponse(global, allocator, value, encoding_value, false);
}
pub fn fromJSWithEncodingValueMaybeAsync(global: *jsc.JSGlobalObject, allocator: std.mem.Allocator, value: jsc.JSValue, encoding_value: jsc.JSValue, is_async: bool) bun.JSError!?BlobOrStringOrBuffer {
return fromJSWithEncodingValueMaybeAsyncAllowRequestResponse(global, allocator, value, encoding_value, is_async, false);
}
pub fn fromJSWithEncodingValueMaybeAsyncAllowRequestResponse(global: *jsc.JSGlobalObject, allocator: std.mem.Allocator, value: jsc.JSValue, encoding_value: jsc.JSValue, is_async: bool, allow_request_response: bool) bun.JSError!?BlobOrStringOrBuffer {
pub fn fromJSWithEncodingValueAllowRequestResponse(global: *jsc.JSGlobalObject, allocator: std.mem.Allocator, value: jsc.JSValue, encoding_value: jsc.JSValue, allow_request_response: bool) bun.JSError!?BlobOrStringOrBuffer {
switch (value.jsType()) {
.DOMWrapper => {
if (value.as(jsc.WebCore.Blob)) |blob| {
@@ -116,7 +112,7 @@ pub const BlobOrStringOrBuffer = union(enum) {
}
const allow_string_object = true;
return .{ .string_or_buffer = try StringOrBuffer.fromJSWithEncodingValueMaybeAsync(global, allocator, value, encoding_value, is_async, allow_string_object) orelse return null };
return .{ .string_or_buffer = try StringOrBuffer.fromJSWithEncodingValueAllowStringObject(global, allocator, value, encoding_value, allow_string_object) orelse return null };
}
};
@@ -255,7 +251,15 @@ pub const StringOrBuffer = union(enum) {
.BigInt64Array,
.BigUint64Array,
.DataView,
=> .{ .buffer = Buffer.fromArrayBuffer(global, value) },
=> {
const buffer = Buffer.fromArrayBuffer(global, value);
if (is_async) {
buffer.buffer.value.protect();
}
return .{ .buffer = buffer };
},
else => null,
};
}
@@ -270,7 +274,11 @@ pub const StringOrBuffer = union(enum) {
pub fn fromJSWithEncodingMaybeAsync(global: *jsc.JSGlobalObject, allocator: std.mem.Allocator, value: jsc.JSValue, encoding: Encoding, is_async: bool, allow_string_object: bool) bun.JSError!?StringOrBuffer {
if (value.isCell() and value.jsType().isArrayBufferLike()) {
return .{ .buffer = Buffer.fromTypedArray(global, value) };
const buffer = Buffer.fromArrayBuffer(global, value);
if (is_async) {
buffer.buffer.value.protect();
}
return .{ .buffer = buffer };
}
if (encoding == .utf8) {
@@ -303,13 +311,14 @@ pub const StringOrBuffer = union(enum) {
return fromJSWithEncoding(global, allocator, value, encoding);
}
pub fn fromJSWithEncodingValueMaybeAsync(global: *jsc.JSGlobalObject, allocator: std.mem.Allocator, value: jsc.JSValue, encoding_value: jsc.JSValue, maybe_async: bool, allow_string_object: bool) bun.JSError!?StringOrBuffer {
pub fn fromJSWithEncodingValueAllowStringObject(global: *jsc.JSGlobalObject, allocator: std.mem.Allocator, value: jsc.JSValue, encoding_value: jsc.JSValue, allow_string_object: bool) bun.JSError!?StringOrBuffer {
const encoding: Encoding = brk: {
if (!encoding_value.isCell())
break :brk .utf8;
break :brk try Encoding.fromJS(encoding_value, global) orelse .utf8;
};
return fromJSWithEncodingMaybeAsync(global, allocator, value, encoding, maybe_async, allow_string_object);
const is_async = false;
return fromJSWithEncodingMaybeAsync(global, allocator, value, encoding, is_async, allow_string_object);
}
};

8
src/bun.js/webcore/Blob.zig
View File

@@ -152,7 +152,7 @@ pub fn doReadFile(this: *Blob, comptime Function: anytype, global: *JSGlobalObje
handler,
Handler.run,
) catch |err| bun.handleOom(err);
var read_file_task = bun.handleOom(read_file.ReadFileTask.createOnJSThread(bun.default_allocator, global, file_read));
var read_file_task = read_file.ReadFileTask.createOnJSThread(bun.default_allocator, global, file_read);
// Create the Promise only after the store has been ref()'d.
// The garbage collector runs on memory allocations
@@ -190,7 +190,7 @@ pub fn doReadFileInternal(this: *Blob, comptime Handler: type, ctx: Handler, com
this.offset,
this.size,
) catch |err| bun.handleOom(err);
var read_file_task = bun.handleOom(read_file.ReadFileTask.createOnJSThread(bun.default_allocator, global, file_read));
var read_file_task = read_file.ReadFileTask.createOnJSThread(bun.default_allocator, global, file_read);
read_file_task.schedule();
}
@@ -1036,7 +1036,7 @@ pub fn writeFileWithSourceDestination(ctx: *jsc.JSGlobalObject, source_blob: *Bl
WriteFilePromise.run,
options.mkdirp_if_not_exists orelse true,
) catch unreachable;
var task = bun.handleOom(write_file.WriteFileTask.createOnJSThread(bun.default_allocator, ctx, file_copier));
var task = write_file.WriteFileTask.createOnJSThread(bun.default_allocator, ctx, file_copier);
// Defer promise creation until we're just about to schedule the task
var promise = jsc.JSPromise.create(ctx);
const promise_value = promise.asValue(ctx);
@@ -1064,7 +1064,7 @@ pub fn writeFileWithSourceDestination(ctx: *jsc.JSGlobalObject, source_blob: *Bl
destination_blob.size,
ctx,
options.mkdirp_if_not_exists orelse true,
) catch unreachable;
);
file_copier.schedule();
return file_copier.promise.value();
} else if (destination_type == .file and source_type == .s3) {

4
src/bun.js/webcore/blob/copy_file.zig
View File

@@ -31,7 +31,7 @@ pub const CopyFile = struct {
max_len: SizeType,
globalThis: *JSGlobalObject,
mkdirp_if_not_exists: bool,
) !*CopyFilePromiseTask {
) *CopyFilePromiseTask {
const read_file = bun.new(CopyFile, CopyFile{
.store = store,
.source_store = source_store,
@@ -44,7 +44,7 @@ pub const CopyFile = struct {
});
store.ref();
source_store.ref();
return bun.handleOom(CopyFilePromiseTask.createOnJSThread(allocator, globalThis, read_file));
return CopyFilePromiseTask.createOnJSThread(allocator, globalThis, read_file);
}
const linux = std.os.linux;

2
src/deps/uws/us_socket_t.zig
View File

@@ -295,7 +295,7 @@ pub const c = struct {
const node_buffer: jsc.Node.BlobOrStringOrBuffer = if (data.isUndefined())
jsc.Node.BlobOrStringOrBuffer{ .string_or_buffer = jsc.Node.StringOrBuffer.empty }
else
jsc.Node.BlobOrStringOrBuffer.fromJSWithEncodingValueMaybeAsyncAllowRequestResponse(globalObject, stack_fallback.get(), data, encoding, false, true) catch {
jsc.Node.BlobOrStringOrBuffer.fromJSWithEncodingValueAllowRequestResponse(globalObject, stack_fallback.get(), data, encoding, true) catch {
return .zero;
} orelse {
if (!globalObject.hasException()) {