From 375d8da8e6d2aec53ee57bdefb281a5f1d285fa9 Mon Sep 17 00:00:00 2001 From: Dylan Conway <35280289+dylan-conway@users.noreply.github.com> Date: Thu, 25 Jul 2024 04:01:53 -0700 Subject: [PATCH] fix(brotli): protect buffer jsvalues (#12800) Co-authored-by: Jarred Sumner --- src/bun.js/api/js_brotli.zig | 85 +++++++++++++++++++++--------------- src/bun.js/node/types.zig | 29 ++++++++++++ 2 files changed, 79 insertions(+), 35 deletions(-) diff --git a/src/bun.js/api/js_brotli.zig b/src/bun.js/api/js_brotli.zig index b6748deab7..925579d26e 100644 --- a/src/bun.js/api/js_brotli.zig +++ b/src/bun.js/api/js_brotli.zig @@ -5,14 +5,40 @@ const brotli = bun.brotli; const Queue = std.fifo.LinearFifo(JSC.Node.BlobOrStringOrBuffer, .Dynamic); +// We cannot free outside the JavaScript thread. +const FreeList = struct { + write_lock: bun.Lock = bun.Lock.init(), + list: std.ArrayListUnmanaged(JSC.Node.BlobOrStringOrBuffer) = .{}, + + pub fn append(this: *FreeList, slice: []const JSC.Node.BlobOrStringOrBuffer) void { + this.write_lock.lock(); + defer this.write_lock.unlock(); + this.list.appendSlice(bun.default_allocator, slice) catch bun.outOfMemory(); + } + + pub fn drain(this: *FreeList) void { + this.write_lock.lock(); + defer this.write_lock.unlock(); + const out = this.list.items; + for (out) |*item| { + item.deinitAndUnprotect(); + } + this.list.clearRetainingCapacity(); + } + + pub fn deinit(this: *FreeList) void { + this.drain(); + this.list.deinit(bun.default_allocator); + } +}; + pub const BrotliEncoder = struct { pub usingnamespace bun.New(@This()); pub usingnamespace JSC.Codegen.JSBrotliEncoder; stream: brotli.BrotliCompressionStream, - freelist: Queue = Queue.init(bun.default_allocator), - freelist_write_lock: bun.Lock = bun.Lock.init(), + freelist: FreeList = .{}, globalThis: *JSC.JSGlobalObject, @@ -85,20 +111,14 @@ pub const BrotliEncoder = struct { pub fn deinit(this: *BrotliEncoder) void { this.callback_value.deinit(); - this.drainFreelist(); + this.freelist.deinit(); this.stream.deinit(); this.input.deinit(); this.destroy(); } fn drainFreelist(this: *BrotliEncoder) void { - this.freelist_write_lock.lock(); - defer this.freelist_write_lock.unlock(); - const to_free = this.freelist.readableSlice(0); - for (to_free) |*input| { - input.deinit(); - } - this.freelist.discard(to_free.len); + this.freelist.drain(); } fn collectOutputValue(this: *BrotliEncoder) JSC.JSValue { @@ -175,11 +195,9 @@ pub const BrotliEncoder = struct { }; defer { - this.encoder.freelist_write_lock.lock(); - this.encoder.freelist.write(pending) catch unreachable; - this.encoder.freelist_write_lock.unlock(); + this.encoder.freelist.append(pending); } - for (pending) |input| { + for (pending) |*input| { var writer = this.encoder.stream.writer(Writer{ .encoder = this.encoder }); writer.writeAll(input.slice()) catch { _ = this.encoder.pending_encode_job_count.fetchSub(1, .monotonic); @@ -254,8 +272,7 @@ pub const BrotliEncoder = struct { { this.input_lock.lock(); defer this.input_lock.unlock(); - - this.input.writeItem(input_to_queue) catch unreachable; + this.input.writeItem(input_to_queue) catch bun.outOfMemory(); } JSC.WorkPool.schedule(&task.task); @@ -297,7 +314,7 @@ pub const BrotliEncoder = struct { this.input_lock.lock(); defer this.input_lock.unlock(); - this.input.writeItem(input_to_queue) catch unreachable; + this.input.writeItem(input_to_queue) catch bun.outOfMemory(); } task.run(); return if (!is_last and this.output.items.len == 0) .undefined else this.collectOutputValue(); @@ -341,8 +358,7 @@ pub const BrotliDecoder = struct { output: std.ArrayListUnmanaged(u8) = .{}, output_lock: bun.Lock = bun.Lock.init(), - freelist: Queue = Queue.init(bun.default_allocator), - freelist_write_lock: bun.Lock = bun.Lock.init(), + freelist: FreeList = .{}, pub fn hasPendingActivity(this: *BrotliDecoder) callconv(.C) bool { return this.has_pending_activity.load(.monotonic) > 0; @@ -350,7 +366,7 @@ pub const BrotliDecoder = struct { pub fn deinit(this: *BrotliDecoder) void { this.callback_value.deinit(); - this.drainFreelist(); + this.freelist.deinit(); this.output.deinit(bun.default_allocator); this.stream.brotli.destroyInstance(); this.input.deinit(); @@ -435,13 +451,7 @@ pub const BrotliDecoder = struct { } fn drainFreelist(this: *BrotliDecoder) void { - this.freelist_write_lock.lock(); - defer this.freelist_write_lock.unlock(); - const to_free = this.freelist.readableSlice(0); - for (to_free) |*input| { - input.deinit(); - } - this.freelist.discard(to_free.len); + this.freelist.drain(); } pub fn decode(this: *BrotliDecoder, globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) JSC.JSValue { @@ -479,7 +489,7 @@ pub const BrotliDecoder = struct { this.input_lock.lock(); defer this.input_lock.unlock(); - this.input.writeItem(input_to_queue) catch unreachable; + this.input.writeItem(input_to_queue) catch bun.outOfMemory(); } JSC.WorkPool.schedule(&task.task); @@ -521,7 +531,7 @@ pub const BrotliDecoder = struct { this.input_lock.lock(); defer this.input_lock.unlock(); - this.input.writeItem(input_to_queue) catch unreachable; + this.input.writeItem(input_to_queue) catch bun.outOfMemory(); } task.run(); return if (!is_last) .undefined else this.collectOutputValue(); @@ -557,20 +567,25 @@ pub const BrotliDecoder = struct { this.decoder.input_lock.lock(); defer this.decoder.input_lock.unlock(); if (!is_last) break; - const readable = this.decoder.input.readableSlice(0); - const pending = readable; + const pending = this.decoder.input.readableSlice(0); defer { - this.decoder.freelist_write_lock.lock(); - this.decoder.freelist.write(pending) catch unreachable; - this.decoder.freelist_write_lock.unlock(); + this.decoder.freelist.append(pending); } var input_list = std.ArrayListUnmanaged(u8){}; defer input_list.deinit(bun.default_allocator); + if (pending.len > 1) { + var count: usize = 0; for (pending) |input| { - input_list.appendSlice(bun.default_allocator, input.slice()) catch bun.outOfMemory(); + count += input.slice().len; + } + + input_list.ensureTotalCapacityPrecise(bun.default_allocator, count) catch bun.outOfMemory(); + + for (pending) |*input| { + input_list.appendSliceAssumeCapacity(input.slice()); } } diff --git a/src/bun.js/node/types.zig b/src/bun.js/node/types.zig index adf5bb2184..c0be183c45 100644 --- a/src/bun.js/node/types.zig +++ b/src/bun.js/node/types.zig @@ -269,6 +269,26 @@ pub const BlobOrStringOrBuffer = union(enum) { }; } + pub fn protect(this: *const BlobOrStringOrBuffer) void { + switch (this.*) { + .string_or_buffer => |sob| { + sob.protect(); + }, + else => {}, + } + } + + pub fn deinitAndUnprotect(this: *BlobOrStringOrBuffer) void { + switch (this.*) { + .string_or_buffer => |sob| { + sob.deinitAndUnprotect(); + }, + .blob => |*blob| { + blob.deinit(); + }, + } + } + pub fn fromJS(global: *JSC.JSGlobalObject, allocator: std.mem.Allocator, value: JSC.JSValue) ?BlobOrStringOrBuffer { if (value.as(JSC.WebCore.Blob)) |blob| { if (blob.store) |store| { @@ -316,6 +336,15 @@ pub const StringOrBuffer = union(enum) { } } + pub fn protect(this: *const StringOrBuffer) void { + switch (this.*) { + .buffer => |buf| { + buf.buffer.value.protect(); + }, + else => {}, + } + } + pub fn fromJSToOwnedSlice(globalObject: *JSC.JSGlobalObject, value: JSC.JSValue, allocator: std.mem.Allocator) ![]u8 { if (value.asArrayBuffer(globalObject)) |array_buffer| { defer globalObject.vm().reportExtraMemory(array_buffer.len);