Skip to content

Commit

Permalink
FFI: Fix dangling reference to CType in carith_checkarg().
Browse files Browse the repository at this point in the history
Reported by Sergey Kaplun.

(cherry-picked from commit db944b2)

During of an arithmetic operation with a cdata function object and some
cdata value in `carith_checkarg()`, reallocation of `cts->tab` in
`lj_ctype_intern()` may occur. In that case, the reference to the first
`CType` object (`ca->ct[0]`) becomes invalid. This patch saves the
`CTypeID` of this object and gets its `CType` again after possible
reallocation.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#9145
  • Loading branch information
Mike Pall authored and Buristan committed Oct 23, 2023
1 parent a963238 commit 33a9a2f
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/lj_carith.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,13 @@ static int carith_checkarg(lua_State *L, CTState *cts, CDArith *ca)
p = (uint8_t *)cdata_getptr(p, ct->size);
if (ctype_isref(ct->info)) ct = ctype_rawchild(cts, ct);
} else if (ctype_isfunc(ct->info)) {
CTypeID id0 = i ? ctype_typeid(cts, ca->ct[0]) : 0;
p = (uint8_t *)*(void **)p;
ct = ctype_get(cts,
lj_ctype_intern(cts, CTINFO(CT_PTR, CTALIGN_PTR|id), CTSIZE_PTR));
if (i) { /* cts->tab may have been reallocated. */
ca->ct[0] = ctype_get(cts, id0);
}
}
if (ctype_isenum(ct->info)) ct = ctype_child(cts, ct);
ca->ct[i] = ct;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
local tap = require('tap')
local ffi = require('ffi')
local test = tap.test('lj-1108-fix-dangling-reference-to-ctype'):skipcond({
-- luacheck: no global
['Impossible to predict the value of cts->top'] = _TARANTOOL,
})

test:plan(1)

-- This test demonstrates LuaJIT's incorrect behaviour when the
-- reallocation of `cts->tab` strikes during the arithmetic cdata
-- metamethod.
-- The test fails under ASAN.

-- XXX: Just some C functions to be casted. There is no need to
-- declare their prototypes correctly.
ffi.cdef[[
int malloc(void);
int fprintf(void);
int printf(void);
int memset(void);
int memcpy(void);
int memmove(void);
int getppid(void);
]]

local cfunc_type = ffi.metatype(ffi.typeof('struct {int a;}'), {
-- Just some metatable with reloaded arithmetic operator.
__add = function(o1, _) return o1 end
})
-- Just some cdata with metamethod.
local test_value = cfunc_type(1)

-- XXX: structure to set `cts->top` to 112.
local _ = ffi.new('struct {int a; long b; float c; double d;}', 0)

-- Anchor table to prevent cdata objects from being collected.
local anchor = {}
-- Each call to this function grows `cts->top` by 3.
local function save_new_func(func)
anchor[#anchor + 1] = ffi.cast('void (*)(void)', func)
end

save_new_func(ffi.C.fprintf) -- `cts->top` = 112
save_new_func(ffi.C.printf) -- `cts->top` = 115
save_new_func(ffi.C.memset) -- `cts->top` = 118
save_new_func(ffi.C.memcpy) -- `cts->top` = 121
save_new_func(ffi.C.malloc) -- `cts->top` = 124

-- Assertions to check the `cts->top` value and step between
-- calls.
assert(ffi.typeinfo(124), 'cts->top >= 124')
assert(not ffi.typeinfo(125), 'cts->top < 125')

save_new_func(ffi.C.memmove) -- `cts->top` = 127

assert(ffi.typeinfo(127), 'cts->top >= 127')
assert(not ffi.typeinfo(128), 'cts->top < 128')

-- Just check cdata arith metamethod. The function argument should
-- be the second because dangling reference is the CType of the
-- first argumnent.
_ = test_value + ffi.C.getppid

test:ok(true, 'no heap-use-after-free in carith_checkarg')

test:done(true)

0 comments on commit 33a9a2f

Please sign in to comment.