Skip to content

Commit aa3f168

Browse files
trivikraduh95
authored andcommitted
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: #64551Fixes: #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
constkFastBuffer=Symbol('kFastBuffer');
31-
constkStringConversionBuffer=Symbol('kStringConversionBuffer');
3231

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

122-
functiongetStringConversionPointer(owner,value,index){
123-
constsize=value.length*3+1;
124-
letbuffers=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+
functionenterStringConversion(state){
122+
if(state.buffers[state.depth]===undefined){
123+
state.buffers[state.depth]=[];
134124
}
125+
state.depth++;
126+
}
127+
128+
functionexitStringConversion(state){
129+
state.depth--;
130+
}
131+
132+
functiongetStringConversionPointer(state,value,index){
133+
constsize=value.length*3+1;
134+
constbuffers=state.buffers[state.depth-1];
135135
letentry=buffers[index];
136136
if(entry!==undefined&&entry.string===value){
137137
returnentry.pointer;
@@ -157,13 +157,13 @@ function getStringConversionPointer(owner, value, index) {
157157
returnentry.pointer;
158158
}
159159

160-
functionconvertPointerArg(type,value,owner,index){
160+
functionconvertPointerArg(type,value,stringState,index){
161161
if(needsNullPointerConversion(type)&&
162162
(value===null||value===undefined)){
163163
return0n;
164164
}
165165
if(hasStringPointerArg(type,value)){
166-
returngetStringConversionPointer(owner,value,index);
166+
returngetStringConversionPointer(stringState,value,index);
167167
}
168168
if(hasPointerMemoryArg(type,value)){
169169
returngetRawPointer(value);
@@ -189,10 +189,10 @@ function getFastArgumentIndexes(argumentsTypes, rawFn) {
189189
returnindexes;
190190
}
191191

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

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

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

247+
conststringState={
248+
__proto__: null,
249+
buffers: [],
250+
depth: 0,
251+
};
252+
247253
constnargs=argumentTypes.length;
248254
letwrapper;
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
}elseif(string0&&typeofarg==='string'){
265-
arg=getStringConversionPointer(owner,arg,0);
271+
enterStringConversion(stringState);
272+
try{
273+
returnrawFn(getStringConversionPointer(stringState,arg,0));
274+
}finally{
275+
exitStringConversion(stringState);
276+
}
266277
}elseif(memory0&&(isArrayBufferView(arg)||isAnyArrayBuffer(arg))){
267278
if(fastBufferInvoke!==undefined){
268279
returnfastBufferInvoke(arg);
@@ -280,8 +291,16 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
280291
if(arguments.length!==2){
281292
throwFFIArgCountError(2,arguments.length);
282293
}
283-
returnrawFn(c0 ? convertFastArg(t0,a0,rawFn,owner,0) : a0,
284-
c1 ? convertFastArg(t1,a1,rawFn,owner,1) : a1);
294+
conststringCall=(c0&&hasStringPointerArg(t0,a0))||
295+
(c1&&hasStringPointerArg(t1,a1));
296+
if(stringCall)enterStringConversion(stringState);
297+
try{
298+
returnrawFn(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
}elseif(nargs===3){
287306
constc0=ArrayPrototypeIncludes(indexes,0);
@@ -294,21 +313,43 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
294313
if(arguments.length!==3){
295314
throwFFIArgCountError(3,arguments.length);
296315
}
297-
returnrawFn(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+
conststringCall=(c0&&hasStringPointerArg(t0,a0))||
317+
(c1&&hasStringPointerArg(t1,a1))||
318+
(c2&&hasStringPointerArg(t2,a2));
319+
if(stringCall)enterStringConversion(stringState);
320+
try{
321+
returnrawFn(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+
letstringCall=false;
306335
for(leti=0;i<indexes.length;i++){
307336
constindex=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(leti=0;i<indexes.length;i++){
345+
constindex=indexes[i];
346+
args[index]=convertFastArg(
347+
argumentTypes[index],args[index],rawFn,stringState,index);
348+
}
349+
returnReflectApply(rawFn,undefined,args);
350+
}finally{
351+
if(stringCall)exitStringConversion(stringState);
310352
}
311-
returnReflectApply(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_EXPORTint32_tstring_survives_callback(constchar*str,
337+
VoidCallbackcallback) {
338+
if (callback) {
339+
callback();
340+
}
341+
342+
returnstr&&strcmp(str, "outer string") ==0;
343+
}
344+
336345
FFI_EXPORTvoidcall_string_callback(StringCallbackcallback, constchar*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+
letnestedLength;
85+
constcallback=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)