FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

ffi: preserve strings during reentrant calls · nodejs/node@aa3f168 · GitHub

/ node Public

Commit aa3f168

Browse files
trivikr
authored and
aduh95
committed
ffi: preserve strings during reentrant calls
Cache temporary string conversion buffers by wrapper and active call depth. This prevents nested FFI calls from overwriting or replacing buffers still in use by an outer native call. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.6-sol PR-URL: #64551 Fixes: #64550 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 870f499 commit aa3f168

3 files changed

Lines changed: 103 additions & 27 deletions

File tree

‎lib/internal/ffi/fast-api.js‎

Lines changed: 68 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ const {
2828
} = internalBinding('ffi');
2929

3030
const kFastBuffer = Symbol('kFastBuffer');
31-
const kStringConversionBuffer = Symbol('kStringConversionBuffer');
3231

3332
const U64_MAX = 0xFFFFFFFFFFFFFFFFn;
3433
const I64_MAX = 0x7FFFFFFFFFFFFFFFn;
@@ -119,19 +118,20 @@ function hasPointerMemoryArg(type, value) {
119118
(isArrayBufferView(value) || isAnyArrayBuffer(value));
120119
}
121120

122-
function getStringConversionPointer(owner, value, index) {
123-
const size = value.length * 3 + 1;
124-
let buffers = owner[kStringConversionBuffer];
125-
if (buffers === undefined) {
126-
buffers = [];
127-
ObjectDefineProperty(owner, kStringConversionBuffer, {
128-
__proto__: null,
129-
configurable: false,
130-
enumerable: false,
131-
writable: false,
132-
value: buffers,
133-
});
121+
function enterStringConversion(state) {
122+
if (state.buffers[state.depth] === undefined) {
123+
state.buffers[state.depth] = [];
134124
}
125+
state.depth++;
126+
}
127+
128+
function exitStringConversion(state) {
129+
state.depth--;
130+
}
131+
132+
function getStringConversionPointer(state, value, index) {
133+
const size = value.length * 3 + 1;
134+
const buffers = state.buffers[state.depth - 1];
135135
let entry = buffers[index];
136136
if (entry !== undefined && entry.string === value) {
137137
return entry.pointer;
@@ -157,13 +157,13 @@ function getStringConversionPointer(owner, value, index) {
157157
return entry.pointer;
158158
}
159159

160-
function convertPointerArg(type, value, owner, index) {
160+
function convertPointerArg(type, value, stringState, index) {
161161
if (needsNullPointerConversion(type) &&
162162
(value === null || value === undefined)) {
163163
return 0n;
164164
}
165165
if (hasStringPointerArg(type, value)) {
166-
return getStringConversionPointer(owner, value, index);
166+
return getStringConversionPointer(stringState, value, index);
167167
}
168168
if (hasPointerMemoryArg(type, value)) {
169169
return getRawPointer(value);
@@ -189,10 +189,10 @@ function getFastArgumentIndexes(argumentsTypes, rawFn) {
189189
return indexes;
190190
}
191191

192-
function convertFastArg(type, value, rawFn, owner, index) {
192+
function convertFastArg(type, value, rawFn, stringState, index) {
193193
validateFastIntegerArg(type, value, index);
194194
return needsPointerConversion(type, rawFn) ?
195-
convertPointerArg(type, value, owner, index) : value;
195+
convertPointerArg(type, value, stringState, index) : value;
196196
}
197197

198198
function initializeFastBufferMetadata(rawFn, argumentTypes) {
@@ -228,7 +228,7 @@ function inheritMetadata(wrapper, rawFn, nargs) {
228228
return wrapper;
229229
}
230230

231-
function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
231+
function wrapWithRawPointerConversions(rawFn, argumentTypes, _owner) {
232232
if (rawFn === undefined || rawFn === null) {
233233
return rawFn;
234234
}
@@ -244,6 +244,12 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
244244
return rawFn;
245245
}
246246

247+
const stringState = {
248+
__proto__: null,
249+
buffers: [],
250+
depth: 0,
251+
};
252+
247253
const nargs = argumentTypes.length;
248254
let wrapper;
249255
if (nargs === 1 && indexes.length === 1 && indexes[0] === 0) {
@@ -262,7 +268,12 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
262268
(arg === null || arg === undefined)) {
263269
arg = 0n;
264270
} else if (string0 && typeof arg === 'string') {
265-
arg = getStringConversionPointer(owner, arg, 0);
271+
enterStringConversion(stringState);
272+
try {
273+
return rawFn(getStringConversionPointer(stringState, arg, 0));
274+
} finally {
275+
exitStringConversion(stringState);
276+
}
266277
} else if (memory0 && (isArrayBufferView(arg) || isAnyArrayBuffer(arg))) {
267278
if (fastBufferInvoke !== undefined) {
268279
return fastBufferInvoke(arg);
@@ -280,8 +291,16 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
280291
if (arguments.length !== 2) {
281292
throwFFIArgCountError(2, arguments.length);
282293
}
283-
return rawFn(c0 ? convertFastArg(t0, a0, rawFn, owner, 0) : a0,
284-
c1 ? convertFastArg(t1, a1, rawFn, owner, 1) : a1);
294+
const stringCall = (c0 && hasStringPointerArg(t0, a0)) ||
295+
(c1 && hasStringPointerArg(t1, a1));
296+
if (stringCall) enterStringConversion(stringState);
297+
try {
298+
return rawFn(c0 ?
299+
convertFastArg(t0, a0, rawFn, stringState, 0) : a0,
300+
c1 ? convertFastArg(t1, a1, rawFn, stringState, 1) : a1);
301+
} finally {
302+
if (stringCall) exitStringConversion(stringState);
303+
}
285304
};
286305
} else if (nargs === 3) {
287306
const c0 = ArrayPrototypeIncludes(indexes, 0);
@@ -294,21 +313,43 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
294313
if (arguments.length !== 3) {
295314
throwFFIArgCountError(3, arguments.length);
296315
}
297-
return rawFn(c0 ? convertFastArg(t0, a0, rawFn, owner, 0) : a0,
298-
c1 ? convertFastArg(t1, a1, rawFn, owner, 1) : a1,
299-
c2 ? convertFastArg(t2, a2, rawFn, owner, 2) : a2);
316+
const stringCall = (c0 && hasStringPointerArg(t0, a0)) ||
317+
(c1 && hasStringPointerArg(t1, a1)) ||
318+
(c2 && hasStringPointerArg(t2, a2));
319+
if (stringCall) enterStringConversion(stringState);
320+
try {
321+
return rawFn(c0 ?
322+
convertFastArg(t0, a0, rawFn, stringState, 0) : a0,
323+
c1 ? convertFastArg(t1, a1, rawFn, stringState, 1) : a1,
324+
c2 ? convertFastArg(t2, a2, rawFn, stringState, 2) : a2);
325+
} finally {
326+
if (stringCall) exitStringConversion(stringState);
327+
}
300328
};
301329
} else {
302330
wrapper = function(...args) {
303331
if (args.length !== nargs) {
304332
throwFFIArgCountError(nargs, args.length);
305333
}
334+
let stringCall = false;
306335
for (let i = 0; i < indexes.length; i++) {
307336
const index = indexes[i];
308-
args[index] = convertFastArg(
309-
argumentTypes[index], args[index], rawFn, owner, index);
337+
if (hasStringPointerArg(argumentTypes[index], args[index])) {
338+
stringCall = true;
339+
break;
340+
}
341+
}
342+
if (stringCall) enterStringConversion(stringState);
343+
try {
344+
for (let i = 0; i < indexes.length; i++) {
345+
const index = indexes[i];
346+
args[index] = convertFastArg(
347+
argumentTypes[index], args[index], rawFn, stringState, index);
348+
}
349+
return ReflectApply(rawFn, undefined, args);
350+
} finally {
351+
if (stringCall) exitStringConversion(stringState);
310352
}
311-
return ReflectApply(rawFn, undefined, args);
312353
};
313354
}
314355

‎test/ffi/fixture_library/ffi_test_library.c‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,15 @@ FFI_EXPORT void call_void_callback(VoidCallback callback) {
333333
}
334334
}
335335

336+
FFI_EXPORT int32_t string_survives_callback(const char* str,
337+
VoidCallback callback) {
338+
if (callback) {
339+
callback();
340+
}
341+
342+
return str && strcmp(str, "outer string") == 0;
343+
}
344+
336345
FFI_EXPORT void call_string_callback(StringCallback callback, const char* str) {
337346
if (callback) {
338347
callback(str);

‎test/ffi/test-ffi-fast-buffer.js‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,29 @@ test('fast FFI buffer arguments reject invalid values', () => {
6969
lib.close();
7070
}
7171
});
72+
73+
test('fast FFI string buffers survive reentrant callbacks', {
74+
// Bundled libffi callbacks crash on SmartOS.
75+
skip: common.isSunOS,
76+
}, () => {
77+
const { lib, functions } = ffi.dlopen(libraryPath, {
78+
safe_strlen: { arguments: ['string'], return: 'i32' },
79+
string_survives_callback: {
80+
arguments: ['string', 'pointer'],
81+
return: 'i32',
82+
},
83+
});
84+
let nestedLength;
85+
const callback = lib.registerCallback(() => {
86+
nestedLength = functions.safe_strlen('inner string');
87+
});
88+
89+
try {
90+
assert.strictEqual(
91+
functions.string_survives_callback('outer string', callback), 1);
92+
assert.strictEqual(nestedLength, 12);
93+
} finally {
94+
lib.unregisterCallback(callback);
95+
lib.close();
96+
}
97+
});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL