Re: [Tarantool-patches] [PATCH luajit 2/2] FFI: Fix dangling CType references (again).

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

[-- 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 --]