Skip to content

Commit 97c2af2

Browse files
Mike PallBuristan
authored andcommitted
FFI: Fix dangling CType references (again).
Reported by Sergey Kaplun. (cherry picked from commit c64020f) 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 Reviewed-by: Sergey Bronnikov <[email protected]> Signed-off-by: Sergey Kaplun <[email protected]> (cherry picked from commit 37d822e)
1 parent 29f92ed commit 97c2af2

File tree

2 files changed

+88
-5
lines changed

2 files changed

+88
-5
lines changed

src/lj_crecord.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1236,18 +1236,19 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd)
12361236
if (ctype_isfunc(info)) {
12371237
TRef func = emitir(IRT(IR_FLOAD, tp), J->base[0], IRFL_CDATA_PTR);
12381238
CType *ctr = ctype_rawchild(cts, ct);
1239+
CTInfo ctr_info = ctr->info; /* crec_call_args may invalidate ctr. */
12391240
IRType t = crec_ct2irt(cts, ctr);
12401241
TRef tr;
12411242
TValue tv;
12421243
/* Check for blacklisted C functions that might call a callback. */
12431244
tv.u64 = ((uintptr_t)cdata_getptr(cdataptr(cd), (LJ_64 && tp == IRT_P64) ? 8 : 4) >> 2) | U64x(800000000, 00000000);
12441245
if (tvistrue(lj_tab_get(J->L, cts->miscmap, &tv)))
12451246
lj_trace_err(J, LJ_TRERR_BLACKL);
1246-
if (ctype_isvoid(ctr->info)) {
1247+
if (ctype_isvoid(ctr_info)) {
12471248
t = IRT_NIL;
12481249
rd->nres = 0;
1249-
} else if (!(ctype_isnum(ctr->info) || ctype_isptr(ctr->info) ||
1250-
ctype_isenum(ctr->info)) || t == IRT_CDATA) {
1250+
} else if (!(ctype_isnum(ctr_info) || ctype_isptr(ctr_info) ||
1251+
ctype_isenum(ctr_info)) || t == IRT_CDATA) {
12511252
lj_trace_err(J, LJ_TRERR_NYICALL);
12521253
}
12531254
if ((info & CTF_VARARG)
@@ -1258,7 +1259,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd)
12581259
func = emitir(IRT(IR_CARG, IRT_NIL), func,
12591260
lj_ir_kint(J, ctype_typeid(cts, ct)));
12601261
tr = emitir(IRT(IR_CALLXS, t), crec_call_args(J, rd, cts, ct), func);
1261-
if (ctype_isbool(ctr->info)) {
1262+
if (ctype_isbool(ctr_info)) {
12621263
if (frame_islua(J->L->base-1) && bc_b(frame_pc(J->L->base-1)[-1]) == 1) {
12631264
/* Don't check result if ignored. */
12641265
tr = TREF_NIL;
@@ -1274,7 +1275,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd)
12741275
tr = TREF_TRUE;
12751276
}
12761277
} else if (t == IRT_PTR || (LJ_64 && t == IRT_P32) ||
1277-
t == IRT_I64 || t == IRT_U64 || ctype_isenum(ctr->info)) {
1278+
t == IRT_I64 || t == IRT_U64 || ctype_isenum(ctr_info)) {
12781279
TRef trid = lj_ir_kint(J, ctype_cid(info));
12791280
tr = emitir(IRTG(IR_CNEWI, IRT_CDATA), trid, tr);
12801281
if (t == IRT_I64 || t == IRT_U64) lj_needsplit(J);
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
local tap = require('tap')
2+
local ffi = require('ffi')
3+
4+
-- This test demonstrates LuaJIT's incorrect behaviour when the
5+
-- reallocation of `cts->tab` strikes during the recording of the
6+
-- setup arguments for the FFI call.
7+
-- Before the patch, the test failed only under ASAN.
8+
-- This file tests the case similar to the
9+
-- <lj-1360-dangling-ctype-ref-on-ccall.test.lua>, but for the
10+
-- compiler part only.
11+
-- See also https://github.com/LuaJIT/LuaJIT/issues/1360.
12+
local test = tap.test('lj-1360-dangling-ctype-ref-on-ccall'):skipcond({
13+
['Impossible to predict the value of cts->top'] = _TARANTOOL,
14+
['Test requires JIT enabled'] = not jit.status(),
15+
})
16+
17+
test:plan(1)
18+
19+
-- XXX: Declare the structure to increase `cts->top` up to 128
20+
-- slots. The setting up of function's arguments to the next call
21+
-- will reallocate the `cts->tab` during `lj_ccall_ctid_vararg()`
22+
-- and `ccall_set_args()` will use a dangling reference.
23+
ffi.cdef[[
24+
struct test {
25+
int a;
26+
int b;
27+
int c;
28+
int d;
29+
int e;
30+
int f;
31+
int g;
32+
int h;
33+
int i;
34+
int j;
35+
int k;
36+
int l;
37+
int m;
38+
int n;
39+
int o;
40+
int p;
41+
int q;
42+
int r;
43+
int s;
44+
int t;
45+
int u;
46+
int v;
47+
int w;
48+
int x;
49+
int y;
50+
int z;
51+
int aa;
52+
int ab;
53+
int ac;
54+
int ad;
55+
};
56+
// Use an existing function that actually takes no arguments.
57+
// We can declare it however we want.
58+
// Need a vararg function for this issue.
59+
int getppid(...);
60+
]]
61+
62+
local arg_call = ffi.new('struct test')
63+
64+
-- Place `getppid` symbol in cache to permit recording.
65+
ffi.C.getppid(0)
66+
67+
-- Assertions to check the `cts->top` value.
68+
assert(ffi.typeinfo(127), 'cts->top >= 127')
69+
assert(not ffi.typeinfo(128), 'cts->top < 128')
70+
71+
jit.opt.start('hotloop=1')
72+
local counter = 0
73+
while counter < 4 do
74+
-- Don't check the result, just check that there is no invalid
75+
-- memory access.
76+
ffi.C.getppid(arg_call)
77+
counter = counter + 1
78+
end
79+
80+
test:ok(true, 'no heap-use-after-free in C call')
81+
82+
test:done(true)

0 commit comments

Comments
 (0)