Skip to content

Commit 5cf2315

Browse files
committed
buffer: avoid wrapper hop in unsafe allocation path
Refs: #61967 Signed-off-by: jorge guerrero <[email protected]>
1 parent 488a854 commit 5cf2315

3 files changed

Lines changed: 108 additions & 20 deletions

File tree

lib/internal/buffer.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const {
3333
hexWrite,
3434
ucs2Write,
3535
utf8WriteStatic,
36-
createUnsafeArrayBuffer,
36+
createUnsafeBuffer: createUnsafeBufferSlow,
3737
setDetachKey,
3838
} = internalBinding('buffer');
3939

@@ -1101,7 +1101,7 @@ function createUnsafeBuffer(size) {
11011101
return new FastBuffer(size);
11021102
}
11031103

1104-
return new FastBuffer(createUnsafeArrayBuffer(size));
1104+
return createUnsafeBufferSlow(size);
11051105
}
11061106

11071107
module.exports = {

src/node_buffer.cc

Lines changed: 51 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1457,6 +1457,30 @@ inline size_t CheckNumberToSize(Local<Value> number) {
14571457
return size;
14581458
}
14591459

1460+
MaybeLocal<ArrayBuffer> CreateUnsafeArrayBufferFromSize(Environment* env,
1461+
size_t size) {
1462+
Isolate* isolate = env->isolate();
1463+
1464+
// 0-length, or zero-fill flag is set, or building snapshot
1465+
if (size == 0 || per_process::cli_options->zero_fill_all_buffers ||
1466+
env->isolate_data()->is_building_snapshot()) {
1467+
return ArrayBuffer::New(isolate, size);
1468+
}
1469+
1470+
std::unique_ptr<BackingStore> store = ArrayBuffer::NewBackingStore(
1471+
isolate,
1472+
size,
1473+
BackingStoreInitializationMode::kUninitialized,
1474+
BackingStoreOnFailureMode::kReturnNull);
1475+
1476+
if (!store) [[unlikely]] {
1477+
THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
1478+
return MaybeLocal<ArrayBuffer>();
1479+
}
1480+
1481+
return ArrayBuffer::New(isolate, std::move(store));
1482+
}
1483+
14601484
void CreateUnsafeArrayBuffer(const FunctionCallbackInfo<Value>& args) {
14611485
Environment* env = Environment::GetCurrent(args);
14621486
if (args.Length() != 1) {
@@ -1466,30 +1490,36 @@ void CreateUnsafeArrayBuffer(const FunctionCallbackInfo<Value>& args) {
14661490

14671491
size_t size = CheckNumberToSize(args[0]);
14681492

1469-
Isolate* isolate = env->isolate();
1470-
14711493
Local<ArrayBuffer> buf;
1494+
if (!CreateUnsafeArrayBufferFromSize(env, size).ToLocal(&buf))
1495+
return;
14721496

1473-
// 0-length, or zero-fill flag is set, or building snapshot
1474-
if (size == 0 || per_process::cli_options->zero_fill_all_buffers ||
1475-
env->isolate_data()->is_building_snapshot()) {
1476-
buf = ArrayBuffer::New(isolate, size);
1477-
} else {
1478-
std::unique_ptr<BackingStore> store = ArrayBuffer::NewBackingStore(
1479-
isolate,
1480-
size,
1481-
BackingStoreInitializationMode::kUninitialized,
1482-
v8::BackingStoreOnFailureMode::kReturnNull);
1497+
args.GetReturnValue().Set(buf);
1498+
}
14831499

1484-
if (!store) [[unlikely]] {
1485-
THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
1486-
return;
1487-
}
1500+
void CreateUnsafeBuffer(const FunctionCallbackInfo<Value>& args) {
1501+
Environment* env = Environment::GetCurrent(args);
1502+
if (args.Length() != 1) {
1503+
env->ThrowRangeError("Invalid array buffer length");
1504+
return;
1505+
}
1506+
1507+
size_t size = CheckNumberToSize(args[0]);
1508+
1509+
Local<ArrayBuffer> array_buffer;
1510+
if (!CreateUnsafeArrayBufferFromSize(env, size).ToLocal(&array_buffer))
1511+
return;
14881512

1489-
buf = ArrayBuffer::New(isolate, std::move(store));
1513+
if (env->buffer_prototype_object().IsEmpty()) {
1514+
args.GetReturnValue().Set(Uint8Array::New(array_buffer, 0, size));
1515+
return;
14901516
}
14911517

1492-
args.GetReturnValue().Set(buf);
1518+
Local<Uint8Array> buffer;
1519+
if (!Buffer::New(env, array_buffer, 0, size).ToLocal(&buffer))
1520+
return;
1521+
1522+
args.GetReturnValue().Set(buffer);
14931523
}
14941524

14951525
template <encoding encoding>
@@ -1621,6 +1651,8 @@ void Initialize(Local<Object> target,
16211651
SetMethod(context, target, "copyArrayBuffer", CopyArrayBuffer);
16221652
SetMethodNoSideEffect(
16231653
context, target, "createUnsafeArrayBuffer", CreateUnsafeArrayBuffer);
1654+
SetMethodNoSideEffect(
1655+
context, target, "createUnsafeBuffer", CreateUnsafeBuffer);
16241656

16251657
SetMethod(context, target, "swap16", Swap16);
16261658
SetMethod(context, target, "swap32", Swap32);
@@ -1723,6 +1755,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
17231755

17241756
registry->Register(CopyArrayBuffer);
17251757
registry->Register(CreateUnsafeArrayBuffer);
1758+
registry->Register(CreateUnsafeBuffer);
17261759

17271760
registry->Register(Atob);
17281761
registry->Register(Btoa);
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
4+
const assert = require('assert');
5+
const { internalBinding } = require('internal/test/binding');
6+
const { loadBuiltinModule } = require('internal/modules/helpers');
7+
8+
const binding = internalBinding('buffer');
9+
const internalBufferModule = loadBuiltinModule('internal/buffer');
10+
11+
function reloadInternalBuffer() {
12+
internalBufferModule.loaded = false;
13+
internalBufferModule.loading = false;
14+
internalBufferModule.exports = {};
15+
internalBufferModule.exportKeys = undefined;
16+
internalBufferModule.module = undefined;
17+
internalBufferModule.compileForPublicLoader();
18+
return internalBufferModule.exports;
19+
}
20+
21+
const originalCreateUnsafeArrayBuffer = binding.createUnsafeArrayBuffer;
22+
const originalCreateUnsafeBuffer = binding.createUnsafeBuffer;
23+
24+
// Regression: Large allocations should not route through createUnsafeArrayBuffer.
25+
binding.createUnsafeArrayBuffer = () => {
26+
throw new Error('createUnsafeArrayBuffer should not be called for large buffers');
27+
};
28+
29+
let internalBuffer = reloadInternalBuffer();
30+
const large = internalBuffer.createUnsafeBuffer(65);
31+
assert(large instanceof Uint8Array);
32+
assert.strictEqual(large.length, 65);
33+
34+
// Edge case: Small sizes should remain on the in-heap path.
35+
binding.createUnsafeBuffer = () => {
36+
throw new Error('createUnsafeBuffer should not be called for small buffers');
37+
};
38+
39+
internalBuffer = reloadInternalBuffer();
40+
const small = internalBuffer.createUnsafeBuffer(64);
41+
assert(small instanceof Uint8Array);
42+
assert.strictEqual(small.length, 64);
43+
44+
// Nearby-path safety check: Public API behavior is unchanged for large sizes.
45+
const userVisible = Buffer.allocUnsafeSlow(65);
46+
assert(Buffer.isBuffer(userVisible));
47+
assert.strictEqual(userVisible.length, 65);
48+
49+
binding.createUnsafeArrayBuffer = originalCreateUnsafeArrayBuffer;
50+
if (originalCreateUnsafeBuffer === undefined) {
51+
delete binding.createUnsafeBuffer;
52+
} else {
53+
binding.createUnsafeBuffer = originalCreateUnsafeBuffer;
54+
}
55+
reloadInternalBuffer();

0 commit comments

Comments
 (0)