* [Tarantool-patches] [PATCH luajit 0/2] Fix dangling CType references @ 2025-08-22 6:36 Sergey Kaplun via Tarantool-patches 2025-08-22 6:36 ` [Tarantool-patches] [PATCH luajit 1/2] FFI: " Sergey Kaplun via Tarantool-patches ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Sergey Kaplun via Tarantool-patches @ 2025-08-22 6:36 UTC (permalink / raw) To: Sergey Bronnikov; +Cc: tarantool-patches These two patches fix the dangling ctype references for the vararg FFI call and its recording. Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1360-dangling-ctype-ref-on-ccall Issues: * https://github.com/tarantool/tarantool/issues/11691 * https://github.com/LuaJIT/LuaJIT/issues/1360 Mike Pall (2): FFI: Fix dangling CType references. FFI: Fix dangling CType references (again). src/lj_ccall.c | 19 +++-- src/lj_crecord.c | 32 +++++--- ...0-dangling-ctype-ref-on-ccall-jit.test.lua | 82 +++++++++++++++++++ ...-1360-dangling-ctype-ref-on-ccall.test.lua | 70 ++++++++++++++++ 4 files changed, 183 insertions(+), 20 deletions(-) create mode 100644 test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua create mode 100644 test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall.test.lua -- 2.50.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Tarantool-patches] [PATCH luajit 1/2] FFI: Fix dangling CType references. 2025-08-22 6:36 [Tarantool-patches] [PATCH luajit 0/2] Fix dangling CType references Sergey Kaplun via Tarantool-patches @ 2025-08-22 6:36 ` Sergey Kaplun via Tarantool-patches 2025-09-08 12:00 ` Sergey Bronnikov via Tarantool-patches 2025-08-22 6:36 ` [Tarantool-patches] [PATCH luajit 2/2] FFI: Fix dangling CType references (again) Sergey Kaplun via Tarantool-patches 2025-09-08 12:02 ` [Tarantool-patches] [PATCH luajit 0/2] Fix dangling CType references Sergey Bronnikov via Tarantool-patches 2 siblings, 1 reply; 6+ messages in thread From: Sergey Kaplun via Tarantool-patches @ 2025-08-22 6:36 UTC (permalink / raw) To: Sergey Bronnikov; +Cc: tarantool-patches From: Mike Pall <mike> Reported by Sergey Kaplun. (cherry picked from commit 9c8eb7cfe10ef5939d9b358a0bd805a610818ba5) In case when the `ccall_set_args()` (and `crec_call_args()` in JIT) reallocates the ctype table, the references of the `ct` become invalid, and `ct->info` dereferences the invalid pointer. This patch fixes this by preserving the ctype info and ctype id to avoid dereferencing invalid pointer. This patch doesn't properly fix the issue for the JIT recording, since the child ctype reference is still in use. It will be fixed in the next patch. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#11691 --- src/lj_ccall.c | 19 +++-- src/lj_crecord.c | 21 +++--- ...-1360-dangling-ctype-ref-on-ccall.test.lua | 70 +++++++++++++++++++ 3 files changed, 95 insertions(+), 15 deletions(-) create mode 100644 test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall.test.lua diff --git a/src/lj_ccall.c b/src/lj_ccall.c index a989f657..c3b27572 100644 --- a/src/lj_ccall.c +++ b/src/lj_ccall.c @@ -890,7 +890,9 @@ void ccall_copy_struct(CCallState *cc, CType *ctr, void *dp, void *sp, int ft) /* -- Common C call handling ---------------------------------------------- */ -/* Infer the destination CTypeID for a vararg argument. */ +/* Infer the destination CTypeID for a vararg argument. +** Note: may reallocate cts->tab and invalidate CType pointers. +*/ CTypeID lj_ccall_ctid_vararg(CTState *cts, cTValue *o) { if (tvisnumber(o)) { @@ -918,13 +920,16 @@ CTypeID lj_ccall_ctid_vararg(CTState *cts, cTValue *o) } } -/* Setup arguments for C call. */ +/* Setup arguments for C call. +** Note: may reallocate cts->tab and invalidate CType pointers. +*/ static int ccall_set_args(lua_State *L, CTState *cts, CType *ct, CCallState *cc) { int gcsteps = 0; TValue *o, *top = L->top; CTypeID fid; + CTInfo info = ct->info; /* lj_ccall_ctid_vararg may invalidate ct pointer. */ CType *ctr; MSize maxgpr, ngpr = 0, nsp = 0, narg; #if CCALL_NARG_FPR @@ -943,7 +948,7 @@ static int ccall_set_args(lua_State *L, CTState *cts, CType *ct, #if LJ_TARGET_X86 /* x86 has several different calling conventions. */ cc->resx87 = 0; - switch (ctype_cconv(ct->info)) { + switch (ctype_cconv(info)) { case CTCC_FASTCALL: maxgpr = 2; break; case CTCC_THISCALL: maxgpr = 1; break; default: maxgpr = 0; break; @@ -960,7 +965,7 @@ static int ccall_set_args(lua_State *L, CTState *cts, CType *ct, } else if (ctype_iscomplex(ctr->info) || ctype_isstruct(ctr->info)) { /* Preallocate cdata object and anchor it after arguments. */ CTSize sz = ctr->size; - GCcdata *cd = lj_cdata_new(cts, ctype_cid(ct->info), sz); + GCcdata *cd = lj_cdata_new(cts, ctype_cid(info), sz); void *dp = cdataptr(cd); setcdataV(L, L->top++, cd); if (ctype_isstruct(ctr->info)) { @@ -996,7 +1001,7 @@ static int ccall_set_args(lua_State *L, CTState *cts, CType *ct, lj_assertL(ctype_isfield(ctf->info), "field expected"); did = ctype_cid(ctf->info); } else { - if (!(ct->info & CTF_VARARG)) + if (!(info & CTF_VARARG)) lj_err_caller(L, LJ_ERR_FFI_NUMARG); /* Too many arguments. */ did = lj_ccall_ctid_vararg(cts, o); /* Infer vararg type. */ isva = 1; @@ -1157,11 +1162,11 @@ int lj_ccall_func(lua_State *L, GCcdata *cd) ct = ctype_rawchild(cts, ct); } if (ctype_isfunc(ct->info)) { + CTypeID id = ctype_typeid(cts, ct); CCallState cc; int gcsteps, ret; cc.func = (void (*)(void))cdata_getptr(cdataptr(cd), sz); gcsteps = ccall_set_args(L, cts, ct, &cc); - ct = (CType *)((intptr_t)ct-(intptr_t)cts->tab); cts->cb.slot = ~0u; lj_vm_ffi_call(&cc); if (cts->cb.slot != ~0u) { /* Blacklist function that called a callback. */ @@ -1169,7 +1174,7 @@ int lj_ccall_func(lua_State *L, GCcdata *cd) tv.u64 = ((uintptr_t)(void *)cc.func >> 2) | U64x(800000000, 00000000); setboolV(lj_tab_set(L, cts->miscmap, &tv), 1); } - ct = (CType *)((intptr_t)ct+(intptr_t)cts->tab); /* May be reallocated. */ + ct = ctype_get(cts, id); /* Table may have been reallocated. */ gcsteps += ccall_get_results(L, cts, ct, &cc, &ret); #if LJ_TARGET_X86 && LJ_ABI_WIN /* Automatically detect __stdcall and fix up C function declaration. */ diff --git a/src/lj_crecord.c b/src/lj_crecord.c index dcb90216..935106dc 100644 --- a/src/lj_crecord.c +++ b/src/lj_crecord.c @@ -1099,12 +1099,15 @@ static void crec_alloc(jit_State *J, RecordFFData *rd, CTypeID id) crec_finalizer(J, trcd, 0, fin); } -/* Record argument conversions. */ +/* Record argument conversions. +** Note: may reallocate cts->tab and invalidate CType pointers. +*/ static TRef crec_call_args(jit_State *J, RecordFFData *rd, CTState *cts, CType *ct) { TRef args[CCI_NARGS_MAX]; CTypeID fid; + CTInfo info = ct->info; /* lj_ccall_ctid_vararg may invalidate ct pointer. */ MSize i, n; TRef tr, *base; cTValue *o; @@ -1113,9 +1116,9 @@ static TRef crec_call_args(jit_State *J, RecordFFData *rd, TRef *arg0 = NULL, *arg1 = NULL; #endif int ngpr = 0; - if (ctype_cconv(ct->info) == CTCC_THISCALL) + if (ctype_cconv(info) == CTCC_THISCALL) ngpr = 1; - else if (ctype_cconv(ct->info) == CTCC_FASTCALL) + else if (ctype_cconv(info) == CTCC_FASTCALL) ngpr = 2; #endif @@ -1140,7 +1143,7 @@ static TRef crec_call_args(jit_State *J, RecordFFData *rd, lj_assertJ(ctype_isfield(ctf->info), "field expected"); did = ctype_cid(ctf->info); } else { - if (!(ct->info & CTF_VARARG)) + if (!(info & CTF_VARARG)) lj_trace_err(J, LJ_TRERR_NYICALL); /* Too many arguments. */ did = lj_ccall_ctid_vararg(cts, o); /* Infer vararg type. */ } @@ -1223,12 +1226,14 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) { CTState *cts = ctype_ctsG(J2G(J)); CType *ct = ctype_raw(cts, cd->ctypeid); + CTInfo info; IRType tp = IRT_PTR; if (ctype_isptr(ct->info)) { tp = (LJ_64 && ct->size == 8) ? IRT_P64 : IRT_P32; ct = ctype_rawchild(cts, ct); } - if (ctype_isfunc(ct->info)) { + info = ct->info; /* crec_call_args may invalidate ct pointer. */ + if (ctype_isfunc(info)) { TRef func = emitir(IRT(IR_FLOAD, tp), J->base[0], IRFL_CDATA_PTR); CType *ctr = ctype_rawchild(cts, ct); IRType t = crec_ct2irt(cts, ctr); @@ -1245,9 +1250,9 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) ctype_isenum(ctr->info)) || t == IRT_CDATA) { lj_trace_err(J, LJ_TRERR_NYICALL); } - if ((ct->info & CTF_VARARG) + if ((info & CTF_VARARG) #if LJ_TARGET_X86 - || ctype_cconv(ct->info) != CTCC_CDECL + || ctype_cconv(info) != CTCC_CDECL #endif ) func = emitir(IRT(IR_CARG, IRT_NIL), func, @@ -1270,7 +1275,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) } } else if (t == IRT_PTR || (LJ_64 && t == IRT_P32) || t == IRT_I64 || t == IRT_U64 || ctype_isenum(ctr->info)) { - TRef trid = lj_ir_kint(J, ctype_cid(ct->info)); + TRef trid = lj_ir_kint(J, ctype_cid(info)); tr = emitir(IRTG(IR_CNEWI, IRT_CDATA), trid, tr); if (t == IRT_I64 || t == IRT_U64) lj_needsplit(J); } else if (t == IRT_FLOAT || t == IRT_U32) { diff --git a/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall.test.lua b/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall.test.lua new file mode 100644 index 00000000..2d790e43 --- /dev/null +++ b/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall.test.lua @@ -0,0 +1,70 @@ +local tap = require('tap') +local ffi = require('ffi') + +-- This test demonstrates LuaJIT's incorrect behaviour when the +-- reallocation of `cts->tab` strikes during the setup arguments +-- for the FFI call. +-- Before the patch, the test failed only under ASAN. +-- See also https://github.com/LuaJIT/LuaJIT/issues/1360. +local test = tap.test('lj-1360-dangling-ctype-ref-on-ccall'):skipcond({ + ['Impossible to predict the value of cts->top'] = _TARANTOOL, +}) + +test:plan(1) + +-- XXX: Declare the structure to increase `cts->top` up to 128 +-- slots. The setting up of function's arguments to the next call +-- will reallocate the `cts->tab` during `lj_ccall_ctid_vararg()` +-- and `ccall_set_args()` will use a dangling reference. +ffi.cdef[[ + struct test { + int a; + int b; + int c; + int d; + int e; + int f; + int g; + int h; + int i; + int j; + int k; + int l; + int m; + int n; + int o; + int p; + int q; + int r; + int s; + int t; + int u; + int v; + int w; + int x; + int y; + int z; + int aa; + int ab; + int ac; + int ad; + }; + // Use an existing function that actually takes no arguments. + // We can declare it however we want. + // Need a vararg function for this issue. + int getppid(...); +]] + +local arg_call = ffi.new('struct test') + +-- Assertions to check the `cts->top` value. +assert(ffi.typeinfo(127), 'cts->top >= 127') +assert(not ffi.typeinfo(128), 'cts->top < 128') + +-- Don't check the result, just check that there is no invalid +-- memory access. +ffi.C.getppid(arg_call) + +test:ok(true, 'no heap-use-after-free in C call') + +test:done(true) -- 2.50.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit 1/2] FFI: Fix dangling CType references. 2025-08-22 6:36 ` [Tarantool-patches] [PATCH luajit 1/2] FFI: " Sergey Kaplun via Tarantool-patches @ 2025-09-08 12:00 ` Sergey Bronnikov via Tarantool-patches 0 siblings, 0 replies; 6+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2025-09-08 12:00 UTC (permalink / raw) To: Sergey Kaplun; +Cc: tarantool-patches [-- Attachment #1: Type: text/plain, Size: 10150 bytes --] Hi, Sergey, thanks for the patch! LGTM Sergey On 8/22/25 09:36, Sergey Kaplun wrote: > From: Mike Pall <mike> > > Reported by Sergey Kaplun. > > (cherry picked from commit 9c8eb7cfe10ef5939d9b358a0bd805a610818ba5) > > In case when the `ccall_set_args()` (and `crec_call_args()` in JIT) > reallocates the ctype table, the references of the `ct` become invalid, > and `ct->info` dereferences the invalid pointer. > > This patch fixes this by preserving the ctype info and ctype id to avoid > dereferencing invalid pointer. > > This patch doesn't properly fix the issue for the JIT recording, since > the child ctype reference is still in use. It will be fixed in the next > patch. > > Sergey Kaplun: > * added the description and the test for the problem > > Part of tarantool/tarantool#11691 > --- > src/lj_ccall.c | 19 +++-- > src/lj_crecord.c | 21 +++--- > ...-1360-dangling-ctype-ref-on-ccall.test.lua | 70 +++++++++++++++++++ > 3 files changed, 95 insertions(+), 15 deletions(-) > create mode 100644 test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall.test.lua > > diff --git a/src/lj_ccall.c b/src/lj_ccall.c > index a989f657..c3b27572 100644 > --- a/src/lj_ccall.c > +++ b/src/lj_ccall.c > @@ -890,7 +890,9 @@ void ccall_copy_struct(CCallState *cc, CType *ctr, void *dp, void *sp, int ft) > > /* -- Common C call handling ---------------------------------------------- */ > > -/* Infer the destination CTypeID for a vararg argument. */ > +/* Infer the destination CTypeID for a vararg argument. > +** Note: may reallocate cts->tab and invalidate CType pointers. > +*/ > CTypeID lj_ccall_ctid_vararg(CTState *cts, cTValue *o) > { > if (tvisnumber(o)) { > @@ -918,13 +920,16 @@ CTypeID lj_ccall_ctid_vararg(CTState *cts, cTValue *o) > } > } > > -/* Setup arguments for C call. */ > +/* Setup arguments for C call. > +** Note: may reallocate cts->tab and invalidate CType pointers. > +*/ > static int ccall_set_args(lua_State *L, CTState *cts, CType *ct, > CCallState *cc) > { > int gcsteps = 0; > TValue *o, *top = L->top; > CTypeID fid; > + CTInfo info = ct->info; /* lj_ccall_ctid_vararg may invalidate ct pointer. */ > CType *ctr; > MSize maxgpr, ngpr = 0, nsp = 0, narg; > #if CCALL_NARG_FPR > @@ -943,7 +948,7 @@ static int ccall_set_args(lua_State *L, CTState *cts, CType *ct, > #if LJ_TARGET_X86 > /* x86 has several different calling conventions. */ > cc->resx87 = 0; > - switch (ctype_cconv(ct->info)) { > + switch (ctype_cconv(info)) { > case CTCC_FASTCALL: maxgpr = 2; break; > case CTCC_THISCALL: maxgpr = 1; break; > default: maxgpr = 0; break; > @@ -960,7 +965,7 @@ static int ccall_set_args(lua_State *L, CTState *cts, CType *ct, > } else if (ctype_iscomplex(ctr->info) || ctype_isstruct(ctr->info)) { > /* Preallocate cdata object and anchor it after arguments. */ > CTSize sz = ctr->size; > - GCcdata *cd = lj_cdata_new(cts, ctype_cid(ct->info), sz); > + GCcdata *cd = lj_cdata_new(cts, ctype_cid(info), sz); > void *dp = cdataptr(cd); > setcdataV(L, L->top++, cd); > if (ctype_isstruct(ctr->info)) { > @@ -996,7 +1001,7 @@ static int ccall_set_args(lua_State *L, CTState *cts, CType *ct, > lj_assertL(ctype_isfield(ctf->info), "field expected"); > did = ctype_cid(ctf->info); > } else { > - if (!(ct->info & CTF_VARARG)) > + if (!(info & CTF_VARARG)) > lj_err_caller(L, LJ_ERR_FFI_NUMARG); /* Too many arguments. */ > did = lj_ccall_ctid_vararg(cts, o); /* Infer vararg type. */ > isva = 1; > @@ -1157,11 +1162,11 @@ int lj_ccall_func(lua_State *L, GCcdata *cd) > ct = ctype_rawchild(cts, ct); > } > if (ctype_isfunc(ct->info)) { > + CTypeID id = ctype_typeid(cts, ct); > CCallState cc; > int gcsteps, ret; > cc.func = (void (*)(void))cdata_getptr(cdataptr(cd), sz); > gcsteps = ccall_set_args(L, cts, ct, &cc); > - ct = (CType *)((intptr_t)ct-(intptr_t)cts->tab); > cts->cb.slot = ~0u; > lj_vm_ffi_call(&cc); > if (cts->cb.slot != ~0u) { /* Blacklist function that called a callback. */ > @@ -1169,7 +1174,7 @@ int lj_ccall_func(lua_State *L, GCcdata *cd) > tv.u64 = ((uintptr_t)(void *)cc.func >> 2) | U64x(800000000, 00000000); > setboolV(lj_tab_set(L, cts->miscmap, &tv), 1); > } > - ct = (CType *)((intptr_t)ct+(intptr_t)cts->tab); /* May be reallocated. */ > + ct = ctype_get(cts, id); /* Table may have been reallocated. */ > gcsteps += ccall_get_results(L, cts, ct, &cc, &ret); > #if LJ_TARGET_X86 && LJ_ABI_WIN > /* Automatically detect __stdcall and fix up C function declaration. */ > diff --git a/src/lj_crecord.c b/src/lj_crecord.c > index dcb90216..935106dc 100644 > --- a/src/lj_crecord.c > +++ b/src/lj_crecord.c > @@ -1099,12 +1099,15 @@ static void crec_alloc(jit_State *J, RecordFFData *rd, CTypeID id) > crec_finalizer(J, trcd, 0, fin); > } > > -/* Record argument conversions. */ > +/* Record argument conversions. > +** Note: may reallocate cts->tab and invalidate CType pointers. > +*/ > static TRef crec_call_args(jit_State *J, RecordFFData *rd, > CTState *cts, CType *ct) > { > TRef args[CCI_NARGS_MAX]; > CTypeID fid; > + CTInfo info = ct->info; /* lj_ccall_ctid_vararg may invalidate ct pointer. */ > MSize i, n; > TRef tr, *base; > cTValue *o; > @@ -1113,9 +1116,9 @@ static TRef crec_call_args(jit_State *J, RecordFFData *rd, > TRef *arg0 = NULL, *arg1 = NULL; > #endif > int ngpr = 0; > - if (ctype_cconv(ct->info) == CTCC_THISCALL) > + if (ctype_cconv(info) == CTCC_THISCALL) > ngpr = 1; > - else if (ctype_cconv(ct->info) == CTCC_FASTCALL) > + else if (ctype_cconv(info) == CTCC_FASTCALL) > ngpr = 2; > #endif > > @@ -1140,7 +1143,7 @@ static TRef crec_call_args(jit_State *J, RecordFFData *rd, > lj_assertJ(ctype_isfield(ctf->info), "field expected"); > did = ctype_cid(ctf->info); > } else { > - if (!(ct->info & CTF_VARARG)) > + if (!(info & CTF_VARARG)) > lj_trace_err(J, LJ_TRERR_NYICALL); /* Too many arguments. */ > did = lj_ccall_ctid_vararg(cts, o); /* Infer vararg type. */ > } > @@ -1223,12 +1226,14 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) > { > CTState *cts = ctype_ctsG(J2G(J)); > CType *ct = ctype_raw(cts, cd->ctypeid); > + CTInfo info; > IRType tp = IRT_PTR; > if (ctype_isptr(ct->info)) { > tp = (LJ_64 && ct->size == 8) ? IRT_P64 : IRT_P32; > ct = ctype_rawchild(cts, ct); > } > - if (ctype_isfunc(ct->info)) { > + info = ct->info; /* crec_call_args may invalidate ct pointer. */ > + if (ctype_isfunc(info)) { > TRef func = emitir(IRT(IR_FLOAD, tp), J->base[0], IRFL_CDATA_PTR); > CType *ctr = ctype_rawchild(cts, ct); > IRType t = crec_ct2irt(cts, ctr); > @@ -1245,9 +1250,9 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) > ctype_isenum(ctr->info)) || t == IRT_CDATA) { > lj_trace_err(J, LJ_TRERR_NYICALL); > } > - if ((ct->info & CTF_VARARG) > + if ((info & CTF_VARARG) > #if LJ_TARGET_X86 > - || ctype_cconv(ct->info) != CTCC_CDECL > + || ctype_cconv(info) != CTCC_CDECL > #endif > ) > func = emitir(IRT(IR_CARG, IRT_NIL), func, > @@ -1270,7 +1275,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) > } > } else if (t == IRT_PTR || (LJ_64 && t == IRT_P32) || > t == IRT_I64 || t == IRT_U64 || ctype_isenum(ctr->info)) { > - TRef trid = lj_ir_kint(J, ctype_cid(ct->info)); > + TRef trid = lj_ir_kint(J, ctype_cid(info)); > tr = emitir(IRTG(IR_CNEWI, IRT_CDATA), trid, tr); > if (t == IRT_I64 || t == IRT_U64) lj_needsplit(J); > } else if (t == IRT_FLOAT || t == IRT_U32) { > diff --git a/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall.test.lua b/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall.test.lua > new file mode 100644 > index 00000000..2d790e43 > --- /dev/null > +++ b/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall.test.lua > @@ -0,0 +1,70 @@ > +local tap = require('tap') > +local ffi = require('ffi') > + > +-- This test demonstrates LuaJIT's incorrect behaviour when the > +-- reallocation of `cts->tab` strikes during the setup arguments > +-- for the FFI call. > +-- Before the patch, the test failed only under ASAN. > +-- See alsohttps://github.com/LuaJIT/LuaJIT/issues/1360. > +local test = tap.test('lj-1360-dangling-ctype-ref-on-ccall'):skipcond({ > + ['Impossible to predict the value of cts->top'] = _TARANTOOL, > +}) > + > +test:plan(1) > + > +-- XXX: Declare the structure to increase `cts->top` up to 128 > +-- slots. The setting up of function's arguments to the next call > +-- will reallocate the `cts->tab` during `lj_ccall_ctid_vararg()` > +-- and `ccall_set_args()` will use a dangling reference. > +ffi.cdef[[ > + struct test { > + int a; > + int b; > + int c; > + int d; > + int e; > + int f; > + int g; > + int h; > + int i; > + int j; > + int k; > + int l; > + int m; > + int n; > + int o; > + int p; > + int q; > + int r; > + int s; > + int t; > + int u; > + int v; > + int w; > + int x; > + int y; > + int z; > + int aa; > + int ab; > + int ac; > + int ad; > + }; > + // Use an existing function that actually takes no arguments. > + // We can declare it however we want. > + // Need a vararg function for this issue. > + int getppid(...); > +]] > + > +local arg_call = ffi.new('struct test') > + > +-- Assertions to check the `cts->top` value. > +assert(ffi.typeinfo(127), 'cts->top >= 127') > +assert(not ffi.typeinfo(128), 'cts->top < 128') > + > +-- Don't check the result, just check that there is no invalid > +-- memory access. > +ffi.C.getppid(arg_call) > + > +test:ok(true, 'no heap-use-after-free in C call') > + > +test:done(true) [-- Attachment #2: Type: text/html, Size: 10358 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Tarantool-patches] [PATCH luajit 2/2] FFI: Fix dangling CType references (again). 2025-08-22 6:36 [Tarantool-patches] [PATCH luajit 0/2] Fix dangling CType references Sergey Kaplun via Tarantool-patches 2025-08-22 6:36 ` [Tarantool-patches] [PATCH luajit 1/2] FFI: " Sergey Kaplun via Tarantool-patches @ 2025-08-22 6:36 ` Sergey Kaplun via Tarantool-patches 2025-09-08 12:01 ` Sergey Bronnikov via Tarantool-patches 2025-09-08 12:02 ` [Tarantool-patches] [PATCH luajit 0/2] Fix dangling CType references Sergey Bronnikov via Tarantool-patches 2 siblings, 1 reply; 6+ messages in thread From: Sergey Kaplun via Tarantool-patches @ 2025-08-22 6:36 UTC (permalink / raw) To: Sergey Bronnikov; +Cc: tarantool-patches From: Mike Pall <mike> Reported by Sergey Kaplun. (cherry picked from commit c64020f3c6d124503213147f2fb47c20335a395b) This patch fixes JIT recording for the vararg calls, that was not fixed in the previous patch. The `ctr` pointer to the child cdata value may become invalid after the ctype state table reallocation in the `crec_call_args()`. This patch preserves `ctr->info` in the separate variable to avoid dereferencing an invalid pointer. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#11691 --- src/lj_crecord.c | 11 +-- ...0-dangling-ctype-ref-on-ccall-jit.test.lua | 82 +++++++++++++++++++ 2 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua diff --git a/src/lj_crecord.c b/src/lj_crecord.c index 935106dc..b016eaec 100644 --- a/src/lj_crecord.c +++ b/src/lj_crecord.c @@ -1236,6 +1236,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) if (ctype_isfunc(info)) { TRef func = emitir(IRT(IR_FLOAD, tp), J->base[0], IRFL_CDATA_PTR); CType *ctr = ctype_rawchild(cts, ct); + CTInfo ctr_info = ctr->info; /* crec_call_args may invalidate ctr. */ IRType t = crec_ct2irt(cts, ctr); TRef tr; TValue tv; @@ -1243,11 +1244,11 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) tv.u64 = ((uintptr_t)cdata_getptr(cdataptr(cd), (LJ_64 && tp == IRT_P64) ? 8 : 4) >> 2) | U64x(800000000, 00000000); if (tvistrue(lj_tab_get(J->L, cts->miscmap, &tv))) lj_trace_err(J, LJ_TRERR_BLACKL); - if (ctype_isvoid(ctr->info)) { + if (ctype_isvoid(ctr_info)) { t = IRT_NIL; rd->nres = 0; - } else if (!(ctype_isnum(ctr->info) || ctype_isptr(ctr->info) || - ctype_isenum(ctr->info)) || t == IRT_CDATA) { + } else if (!(ctype_isnum(ctr_info) || ctype_isptr(ctr_info) || + ctype_isenum(ctr_info)) || t == IRT_CDATA) { lj_trace_err(J, LJ_TRERR_NYICALL); } if ((info & CTF_VARARG) @@ -1258,7 +1259,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) func = emitir(IRT(IR_CARG, IRT_NIL), func, lj_ir_kint(J, ctype_typeid(cts, ct))); tr = emitir(IRT(IR_CALLXS, t), crec_call_args(J, rd, cts, ct), func); - if (ctype_isbool(ctr->info)) { + if (ctype_isbool(ctr_info)) { if (frame_islua(J->L->base-1) && bc_b(frame_pc(J->L->base-1)[-1]) == 1) { /* Don't check result if ignored. */ tr = TREF_NIL; @@ -1274,7 +1275,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) tr = TREF_TRUE; } } else if (t == IRT_PTR || (LJ_64 && t == IRT_P32) || - t == IRT_I64 || t == IRT_U64 || ctype_isenum(ctr->info)) { + t == IRT_I64 || t == IRT_U64 || ctype_isenum(ctr_info)) { TRef trid = lj_ir_kint(J, ctype_cid(info)); tr = emitir(IRTG(IR_CNEWI, IRT_CDATA), trid, tr); if (t == IRT_I64 || t == IRT_U64) lj_needsplit(J); diff --git a/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua b/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua new file mode 100644 index 00000000..388bec6e --- /dev/null +++ b/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua @@ -0,0 +1,82 @@ +local tap = require('tap') +local ffi = require('ffi') + +-- This test demonstrates LuaJIT's incorrect behaviour when the +-- reallocation of `cts->tab` strikes during the recording of the +-- setup arguments for the FFI call. +-- Before the patch, the test failed only under ASAN. +-- This file tests the case similar to the +-- <lj-1360-dangling-ctype-ref-on-ccall.test.lua>, but for the +-- compiler part only. +-- See also https://github.com/LuaJIT/LuaJIT/issues/1360. +local test = tap.test('lj-1360-dangling-ctype-ref-on-ccall'):skipcond({ + ['Impossible to predict the value of cts->top'] = _TARANTOOL, + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(1) + +-- XXX: Declare the structure to increase `cts->top` up to 128 +-- slots. The setting up of function's arguments to the next call +-- will reallocate the `cts->tab` during `lj_ccall_ctid_vararg()` +-- and `ccall_set_args()` will use a dangling reference. +ffi.cdef[[ + struct test { + int a; + int b; + int c; + int d; + int e; + int f; + int g; + int h; + int i; + int j; + int k; + int l; + int m; + int n; + int o; + int p; + int q; + int r; + int s; + int t; + int u; + int v; + int w; + int x; + int y; + int z; + int aa; + int ab; + int ac; + int ad; + }; + // Use an existing function that actually takes no arguments. + // We can declare it however we want. + // Need a vararg function for this issue. + int getppid(...); +]] + +local arg_call = ffi.new('struct test') + +-- Place `getppid` symbol in cache to permit recording. +ffi.C.getppid(0) + +-- Assertions to check the `cts->top` value. +assert(ffi.typeinfo(127), 'cts->top >= 127') +assert(not ffi.typeinfo(128), 'cts->top < 128') + +jit.opt.start('hotloop=1') +local counter = 0 +while counter < 4 do + -- Don't check the result, just check that there is no invalid + -- memory access. + ffi.C.getppid(arg_call) + counter = counter + 1 +end + +test:ok(true, 'no heap-use-after-free in C call') + +test:done(true) -- 2.50.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit 2/2] FFI: Fix dangling CType references (again). 2025-08-22 6:36 ` [Tarantool-patches] [PATCH luajit 2/2] FFI: Fix dangling CType references (again) Sergey Kaplun via Tarantool-patches @ 2025-09-08 12:01 ` Sergey Bronnikov via Tarantool-patches 0 siblings, 0 replies; 6+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2025-09-08 12:01 UTC (permalink / raw) To: Sergey Kaplun; +Cc: tarantool-patches [-- Attachment #1: Type: text/plain, Size: 5868 bytes --] Hi, Sergey, thanks for the patch! LGTM Sergey On 8/22/25 09:36, Sergey Kaplun wrote: > From: Mike Pall <mike> > > Reported by Sergey Kaplun. > > (cherry picked from commit c64020f3c6d124503213147f2fb47c20335a395b) > > This patch fixes JIT recording for the vararg calls, that was not fixed > in the previous patch. The `ctr` pointer to the child cdata value may > become invalid after the ctype state table reallocation in the > `crec_call_args()`. > > This patch preserves `ctr->info` in the separate variable to avoid > dereferencing an invalid pointer. > > Sergey Kaplun: > * added the description and the test for the problem > > Part of tarantool/tarantool#11691 > --- > src/lj_crecord.c | 11 +-- > ...0-dangling-ctype-ref-on-ccall-jit.test.lua | 82 +++++++++++++++++++ > 2 files changed, 88 insertions(+), 5 deletions(-) > create mode 100644 test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua > > diff --git a/src/lj_crecord.c b/src/lj_crecord.c > index 935106dc..b016eaec 100644 > --- a/src/lj_crecord.c > +++ b/src/lj_crecord.c > @@ -1236,6 +1236,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) > if (ctype_isfunc(info)) { > TRef func = emitir(IRT(IR_FLOAD, tp), J->base[0], IRFL_CDATA_PTR); > CType *ctr = ctype_rawchild(cts, ct); > + CTInfo ctr_info = ctr->info; /* crec_call_args may invalidate ctr. */ > IRType t = crec_ct2irt(cts, ctr); > TRef tr; > TValue tv; > @@ -1243,11 +1244,11 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) > tv.u64 = ((uintptr_t)cdata_getptr(cdataptr(cd), (LJ_64 && tp == IRT_P64) ? 8 : 4) >> 2) | U64x(800000000, 00000000); > if (tvistrue(lj_tab_get(J->L, cts->miscmap, &tv))) > lj_trace_err(J, LJ_TRERR_BLACKL); > - if (ctype_isvoid(ctr->info)) { > + if (ctype_isvoid(ctr_info)) { > t = IRT_NIL; > rd->nres = 0; > - } else if (!(ctype_isnum(ctr->info) || ctype_isptr(ctr->info) || > - ctype_isenum(ctr->info)) || t == IRT_CDATA) { > + } else if (!(ctype_isnum(ctr_info) || ctype_isptr(ctr_info) || > + ctype_isenum(ctr_info)) || t == IRT_CDATA) { > lj_trace_err(J, LJ_TRERR_NYICALL); > } > if ((info & CTF_VARARG) > @@ -1258,7 +1259,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) > func = emitir(IRT(IR_CARG, IRT_NIL), func, > lj_ir_kint(J, ctype_typeid(cts, ct))); > tr = emitir(IRT(IR_CALLXS, t), crec_call_args(J, rd, cts, ct), func); > - if (ctype_isbool(ctr->info)) { > + if (ctype_isbool(ctr_info)) { > if (frame_islua(J->L->base-1) && bc_b(frame_pc(J->L->base-1)[-1]) == 1) { > /* Don't check result if ignored. */ > tr = TREF_NIL; > @@ -1274,7 +1275,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) > tr = TREF_TRUE; > } > } else if (t == IRT_PTR || (LJ_64 && t == IRT_P32) || > - t == IRT_I64 || t == IRT_U64 || ctype_isenum(ctr->info)) { > + t == IRT_I64 || t == IRT_U64 || ctype_isenum(ctr_info)) { > TRef trid = lj_ir_kint(J, ctype_cid(info)); > tr = emitir(IRTG(IR_CNEWI, IRT_CDATA), trid, tr); > if (t == IRT_I64 || t == IRT_U64) lj_needsplit(J); > diff --git a/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua b/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua > new file mode 100644 > index 00000000..388bec6e > --- /dev/null > +++ b/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua > @@ -0,0 +1,82 @@ > +local tap = require('tap') > +local ffi = require('ffi') > + > +-- This test demonstrates LuaJIT's incorrect behaviour when the > +-- reallocation of `cts->tab` strikes during the recording of the > +-- setup arguments for the FFI call. > +-- Before the patch, the test failed only under ASAN. > +-- This file tests the case similar to the > +-- <lj-1360-dangling-ctype-ref-on-ccall.test.lua>, but for the > +-- compiler part only. > +-- See alsohttps://github.com/LuaJIT/LuaJIT/issues/1360. > +local test = tap.test('lj-1360-dangling-ctype-ref-on-ccall'):skipcond({ > + ['Impossible to predict the value of cts->top'] = _TARANTOOL, > + ['Test requires JIT enabled'] = not jit.status(), > +}) > + > +test:plan(1) > + > +-- XXX: Declare the structure to increase `cts->top` up to 128 > +-- slots. The setting up of function's arguments to the next call > +-- will reallocate the `cts->tab` during `lj_ccall_ctid_vararg()` > +-- and `ccall_set_args()` will use a dangling reference. > +ffi.cdef[[ > + struct test { > + int a; > + int b; > + int c; > + int d; > + int e; > + int f; > + int g; > + int h; > + int i; > + int j; > + int k; > + int l; > + int m; > + int n; > + int o; > + int p; > + int q; > + int r; > + int s; > + int t; > + int u; > + int v; > + int w; > + int x; > + int y; > + int z; > + int aa; > + int ab; > + int ac; > + int ad; > + }; > + // Use an existing function that actually takes no arguments. > + // We can declare it however we want. > + // Need a vararg function for this issue. > + int getppid(...); > +]] > + > +local arg_call = ffi.new('struct test') > + > +-- Place `getppid` symbol in cache to permit recording. > +ffi.C.getppid(0) > + > +-- Assertions to check the `cts->top` value. > +assert(ffi.typeinfo(127), 'cts->top >= 127') > +assert(not ffi.typeinfo(128), 'cts->top < 128') > + > +jit.opt.start('hotloop=1') > +local counter = 0 > +while counter < 4 do > + -- Don't check the result, just check that there is no invalid > + -- memory access. > + ffi.C.getppid(arg_call) > + counter = counter + 1 > +end > + > +test:ok(true, 'no heap-use-after-free in C call') > + > +test:done(true) [-- Attachment #2: Type: text/html, Size: 6248 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit 0/2] Fix dangling CType references 2025-08-22 6:36 [Tarantool-patches] [PATCH luajit 0/2] Fix dangling CType references Sergey Kaplun via Tarantool-patches 2025-08-22 6:36 ` [Tarantool-patches] [PATCH luajit 1/2] FFI: " Sergey Kaplun via Tarantool-patches 2025-08-22 6:36 ` [Tarantool-patches] [PATCH luajit 2/2] FFI: Fix dangling CType references (again) Sergey Kaplun via Tarantool-patches @ 2025-09-08 12:02 ` Sergey Bronnikov via Tarantool-patches 2 siblings, 0 replies; 6+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2025-09-08 12:02 UTC (permalink / raw) To: Sergey Kaplun; +Cc: tarantool-patches [-- Attachment #1: Type: text/plain, Size: 991 bytes --] Patch series LGTM. On 8/22/25 09:36, Sergey Kaplun wrote: > These two patches fix the dangling ctype references for the vararg FFI > call and its recording. > > Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1360-dangling-ctype-ref-on-ccall > > Issues: > *https://github.com/tarantool/tarantool/issues/11691 > *https://github.com/LuaJIT/LuaJIT/issues/1360 > > Mike Pall (2): > FFI: Fix dangling CType references. > FFI: Fix dangling CType references (again). > > src/lj_ccall.c | 19 +++-- > src/lj_crecord.c | 32 +++++--- > ...0-dangling-ctype-ref-on-ccall-jit.test.lua | 82 +++++++++++++++++++ > ...-1360-dangling-ctype-ref-on-ccall.test.lua | 70 ++++++++++++++++ > 4 files changed, 183 insertions(+), 20 deletions(-) > create mode 100644 test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua > create mode 100644 test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall.test.lua > [-- Attachment #2: Type: text/html, Size: 1712 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-09-08 12:02 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-08-22 6:36 [Tarantool-patches] [PATCH luajit 0/2] Fix dangling CType references Sergey Kaplun via Tarantool-patches 2025-08-22 6:36 ` [Tarantool-patches] [PATCH luajit 1/2] FFI: " Sergey Kaplun via Tarantool-patches 2025-09-08 12:00 ` Sergey Bronnikov via Tarantool-patches 2025-08-22 6:36 ` [Tarantool-patches] [PATCH luajit 2/2] FFI: Fix dangling CType references (again) Sergey Kaplun via Tarantool-patches 2025-09-08 12:01 ` Sergey Bronnikov via Tarantool-patches 2025-09-08 12:02 ` [Tarantool-patches] [PATCH luajit 0/2] Fix dangling CType references Sergey Bronnikov via Tarantool-patches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox