Skip to content

Commit

Permalink
FFI: Treat cdata finalizer table as a GC root.
Browse files Browse the repository at this point in the history
Thanks to Sergey Bronnikov.

(cherry picked from commit dda1ac2)

The finalizers table is created on initialization of the `ffi` module by
calling the `ffi_finalizer()` routine in the `luaopen_ffi()`. `ffi.gc()`
is referenced by Lua stack via the `ffi` library, and the finalizer
table is anchored there as well. If there is no FFI module table
anywhere to anchor the `ffi.gc` itself and the `lua_State` object is
marked after the function `ffi.gc` is removed from it (since we stop the
GC before chunk loading and start after), then the finalizer table isn't
marked. Hence, after the atomic phase, the table is considered dead and
collected. Since the table is collected, the usage of its nodes in the
`lj_gc_finalize_cdata()` leads to heap-use-after-free.

The patch fixes the problem partially by marking the finalizer table at
the start of the GC cycle. The complete fix will be applied in the next
patch by turning the finalizer table into the proper GC root.

Sergey Bronnikov:
* added the description and the tests for the problem

Part of tarantool/tarantool#10199

Reviewed-by: Sergey Kaplun <[email protected]>
Reviewed-by: Maxim Kokryashkin <[email protected]>
Signed-off-by: Sergey Kaplun <[email protected]>
  • Loading branch information
Mike Pall authored and Buristan committed Aug 15, 2024
1 parent 01f4586 commit feaf8ba
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/lj_gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ static void gc_mark_start(global_State *g)
gc_markobj(g, tabref(mainthread(g)->env));
gc_marktv(g, &g->registrytv);
gc_mark_gcroot(g);
#if LJ_HASFFI
if (ctype_ctsG(g)) gc_markobj(g, ctype_ctsG(g)->finalizer);
#endif
g->gc.state = GCSpropagate;
}

Expand Down
76 changes: 76 additions & 0 deletions test/tarantool-c-tests/lj-1168-unmarked-finalizer-tab.test.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
#include "lua.h"
#include "lauxlib.h"

#include "test.h"

#define UNUSED(x) ((void)(x))

/*
* This test demonstrates LuaJIT's incorrect behaviour on
* loading Lua chunk with cdata numbers.
* See https://github.com/LuaJIT/LuaJIT/issues/1168 for details.
*
* The GC is driving forward during parsing of the Lua chunk
* (`test_chunk`). The chunk contains a single cdata object with
* a number. That leads to the opening of the FFI library
* on-demand during the parsing of this number. After the FFI
* library is open, `ffi.gc` has the finalizer table as its
* environment. But, there is no FFI module table anywhere to
* anchor the `ffi.gc` itself, and the `lua_State` object is
* marked after the function is removed from it. Hence, after the
* atomic phase, the table is considered dead and collected. Since
* the table is collected, the usage of its nodes in the
* `lj_gc_finalize_cdata` leads to heap-use-after-free.
*/

const char buff[] = "return 1LL";

/*
* lua_close is a part of testcase, so testcase creates
* its own Lua state and closes it at the end.
*/
static int unmarked_finalizer_tab_gcstart(void *test_state)
{
/* Shared Lua state is not needed. */
UNUSED(test_state);

/* Setup. */
lua_State *L = luaL_newstate();

/* Set GC at the start. */
lua_gc(L, LUA_GCCOLLECT, 0);

/* Not trigger GC during `lua_openffi()`. */
lua_gc(L, LUA_GCSTOP, 0);

/*
* The terminating '\0' is considered by parser as part of
* the input, so we must chomp it.
*/
int res = luaL_loadbufferx(L, buff, sizeof(buff) - 1,
"test_chunk", "t");
if (res != LUA_OK) {
test_comment("error loading Lua chunk: %s",
lua_tostring(L, -1));
bail_out("error loading Lua chunk");
}

/* Finish GC cycle to collect the finalizer table. */
while (!lua_gc(L, LUA_GCSTEP, -1));

/* Teardown. */
lua_settop(L, 0);
lua_close(L);

return TEST_EXIT_SUCCESS;
}

int main(void)
{
const struct test_unit tgroup[] = {
test_unit_def(unmarked_finalizer_tab_gcstart),
};
const int test_result = test_run_group(tgroup, NULL);

return test_result;
}
18 changes: 18 additions & 0 deletions test/tarantool-tests/lj-1168-unmarked-finalizer-tab.test.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
local tap = require('tap')

-- This test demonstrates LuaJIT's heap-use-after-free on
-- cleaning of resources during shutdown. The test simulates
-- "unloading" of the library, or removing some of its
-- functionality and then calls `collectgarbage`.
-- See https://github.com/LuaJIT/LuaJIT/issues/1168 for details.
local test = tap.test('lj-1168-unmarked-finalizer-tab')
test:plan(1)

local ffi = require('ffi')

ffi.gc = nil
collectgarbage()

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

test:done(true)

0 comments on commit feaf8ba

Please sign in to comment.