[Tarantool-patches] [PATCH luajit 1/2][v2] FFI: Treat cdata finalizer table as a GC root.

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

From: Mike Pall <mike>

Thanks to Sergey Bronnikov.

(cherry picked from commit dda1ac273ad946387088d91039a8ae319359903d)

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 on 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
---
 src/lj_gc.c                                   |  3 +
 .../lj-1168-unmarked-finalizer-tab.test.c     | 76 +++++++++++++++++++
 .../lj-1168-unmarked-finalizer-tab.test.lua   | 18 +++++
 3 files changed, 97 insertions(+)
 create mode 100644 test/tarantool-c-tests/lj-1168-unmarked-finalizer-tab.test.c
 create mode 100644 test/tarantool-tests/lj-1168-unmarked-finalizer-tab.test.lua

diff --git a/src/lj_gc.c b/src/lj_gc.c
index 591862b3..42348a34 100644
--- a/src/lj_gc.c
+++ b/src/lj_gc.c
@@ -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;
 }
 
diff --git a/test/tarantool-c-tests/lj-1168-unmarked-finalizer-tab.test.c b/test/tarantool-c-tests/lj-1168-unmarked-finalizer-tab.test.c
new file mode 100644
index 00000000..d577b551
--- /dev/null
+++ b/test/tarantool-c-tests/lj-1168-unmarked-finalizer-tab.test.c
@@ -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;
+}
diff --git a/test/tarantool-tests/lj-1168-unmarked-finalizer-tab.test.lua b/test/tarantool-tests/lj-1168-unmarked-finalizer-tab.test.lua
new file mode 100644
index 00000000..4b49e9a1
--- /dev/null
+++ b/test/tarantool-tests/lj-1168-unmarked-finalizer-tab.test.lua
@@ -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)
-- 
2.34.1