Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit 0/2] Fix cdata finalizer table
@ 2024-07-08 12:26 Sergey Bronnikov via Tarantool-patches
  2024-07-08 12:26 ` [Tarantool-patches] [PATCH luajit 1/2] FFI: Treat cdata finalizer table as a GC root Sergey Bronnikov via Tarantool-patches
  2024-07-08 12:26 ` [Tarantool-patches] [PATCH luajit 2/2] FFI: Turn FFI finalizer table into a proper " Sergey Bronnikov via Tarantool-patches
  0 siblings, 2 replies; 3+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-07-08 12:26 UTC (permalink / raw)
  To: tarantool-patches, Sergey Kaplun, Maxim Kokryashkin

From: Sergey Bronnikov <sergeyb@tarantool.org>

Branch: https://github.com/tarantool/luajit/tree/ligurio/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer-nointegration
NOTE: Jobs with Tarantool regression tests has failed because
patch "FFI: Turn FFI finalizer table into a proper GC root."
broke Tarantool build and fix (see below) must be applied before
a bump to LuaJIT version with proposed patches.

Branch with fix in Tarantool: https://github.com/ligurio/tarantool/tree/ligurio/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer
Issues:
- https://github.com/luaJIT/luaJIT/issues/1168
- https://github.com/tarantool/tarantool/issues/10199

Mike Pall (2):
  FFI: Treat cdata finalizer table as a GC root.
  FFI: Turn FFI finalizer table into a proper GC root.

 src/lib_ffi.c                                 |  20 +--
 src/lj_cdata.c                                |   2 +-
 src/lj_ctype.c                                |  12 ++
 src/lj_ctype.h                                |   2 +-
 src/lj_gc.c                                   |  38 ++---
 src/lj_obj.h                                  |   3 +
 src/lj_state.c                                |   3 +
 ...free-on-access-to-CTState-finalizer.test.c | 147 ++++++++++++++++++
 ...ee-on-access-to-CTState-finalizer.test.lua |  18 +++
 9 files changed, 204 insertions(+), 41 deletions(-)
 create mode 100644 test/tarantool-c-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.c
 create mode 100644 test/tarantool-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.lua

-- 
2.34.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [Tarantool-patches] [PATCH luajit 1/2] FFI: Treat cdata finalizer table as a GC root.
  2024-07-08 12:26 [Tarantool-patches] [PATCH luajit 0/2] Fix cdata finalizer table Sergey Bronnikov via Tarantool-patches
@ 2024-07-08 12:26 ` Sergey Bronnikov via Tarantool-patches
  2024-07-08 12:26 ` [Tarantool-patches] [PATCH luajit 2/2] FFI: Turn FFI finalizer table into a proper " Sergey Bronnikov via Tarantool-patches
  1 sibling, 0 replies; 3+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-07-08 12:26 UTC (permalink / raw)
  To: tarantool-patches, Sergey Kaplun, Maxim Kokryashkin

From: Mike Pall <mike>

Thanks to Sergey Bronnikov.

(cherry picked from commit dda1ac273ad946387088d91039a8ae319359903d)

There is a table `CTState->finalizer` that contains cdata finalizers.
This table is created on initialization of the `ffi` module
by calling the functions `luaopen_ffi` and `ffi_finalizer`. In some
circumstances, this table could be collected by GC and then accessed by
the function `lj_gc_finalize_cdata`. This leads to a heap-use-after-free
problem. The patch fixes the problem.

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

Part of tarantool/tarantool#10199
---
 src/lj_gc.c                                   |   3 +
 ...free-on-access-to-CTState-finalizer.test.c | 105 ++++++++++++++++++
 ...ee-on-access-to-CTState-finalizer.test.lua |  18 +++
 3 files changed, 126 insertions(+)
 create mode 100644 test/tarantool-c-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.c
 create mode 100644 test/tarantool-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.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-heap-use-after-free-on-access-to-CTState-finalizer.test.c b/test/tarantool-c-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.c
new file mode 100644
index 00000000..ad2d8e62
--- /dev/null
+++ b/test/tarantool-c-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.c
@@ -0,0 +1,105 @@
+#include <string.h>
+
+#include "lua.h"
+#include "lauxlib.h"
+
+#include "test.h"
+
+/*
+ * This test demonstrates LuaJIT's incorrect behaviour on
+ * loading Lua chunk.
+ * See https://github.com/LuaJIT/LuaJIT/issues/1168 for details.
+ *
+ * The GC is driving forward during parsing of the Lua chunk (`chunk`).
+ * The chunk contains plenty of global objects, and the parsing
+ * of this creates a lot of strings to be used as keys in `_G`.
+ * Also, it contains several imaginary numbers (1i, 4i, 8i).
+ * That leads to the opening of the FFI library on-demand during the
+ * parsing of these numbers. 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 was marked before the function is
+ * placed on 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.
+ *
+ * The second reason when the loaded function/chunk isn't loaded
+ * on the `lua_State`, so the cdata (or the FFI module table)
+ * isn't marked is when the chunk contains an error (the chunk
+ * below is valid grammatically but not semantically).
+ *
+ * The repro itself is very fragile, since if we add any of the
+ * `luaopen_*()` or `luaL_openlibs()` calls, it may cause
+ * the marking of the `lua_State *` at the moment when the
+ * FFI module table is on it, so the ffi.gc function and
+ * finalizer table are marked.
+ */
+
+char chunk[] =
+"a=b\n"
+"Y{d^v,vy^y{\n"
+" K/false*#4,o,v,oo,v,Lv8,v,L,v,o5,o,v,oo,v,Lv8,v,Lv8,v,oee,vL,8,vv8,v,o,v,oo,v,Lv4,v,L8,o,v,oo,v,Lv4,v,Lv8,v,o9,o,v,oo,v,Lv8,v,Lv8,v,o,Io,v1,v,oo,v,vL,8,vv8,v;N3%0,yzx{\n"
+"  0,x{0,3,D,3;0,yzx{3;uyyyyzx{0,3;3,yzx{\n"
+"   0,0,y9,o,v,oo,v,Lv4,v,Lv8,v,oeeeey8,v,oeeeeyv,o,v,oo,v,Lv8,v,Lv8,v,oeeeeyv,o,v,oo,v,Lv8,v,Lv4,v,Lv8,v,oeeeeyv,o,v,oo,v,Lv8,v,Lv8,v,o9,eee,oo,voyyy,v,oo,v,oo,voyyy,v,oo,v,vL,8,v,v^I{\n"
+"    K,v,oo,v,vL,8,vv8,v8,v,Lv8,v,o,oo,vy,eeee,oo,v,v,oo,v,vL,8,vv8,vy^yI{\n"
+"     K,v,oo,v,vL,5/4>4*#4/4/4>4*#8i*#4/4/4>4*#4/4/4>6*#4.4>4/4>4*#1i*#4/4/4>46*#4/6/444446*#4i*#45/46/4>44444*#44444454444/49/4>410*#444484445/48,v,oe,ee,oo,vy,v,oo,v,e1,oo,vy,v,oo,v,vL,8,vv8,v7^y8,o,v,oo,v,Lv4,v,Le{\n"
+"      K,v,o0,o,v,oo,v,Lv8,v,Lv8,vK=A{44;4,4,oy^y{Ky,3,y{z{3,y{3;z{3%0,y{0,3;z{\n"
+"       3%0,y3;3%0,yz<{3;zx{3;3%0,yzx{3;zx{0,3,DOJv0,3%0,yzx{y8{\n"
+"        3%0,y{y3;3,y{0,3;z{0,3;3,y{3;z{0,3,0,3;3,y{\n"
+"         zx{0,3;3,yzx{3;zx{0,3;3,yzx{0;N3%0,yzx{0,x{0,3,D,3;3%0,yzx{\n"
+"          3;ux{3,yzx{0,y{3;3%0,yzx{y,Io,v,v,oo,v,vL,8,vv8,v;N3%0,yzx{0,x{D,3;3%0,yzx{\n"
+"           3;u{3;1%0,yzx{\n"
+"             y,o,v,oo,v,Lv4,v,Lv8,v,o,v,o,o,v,oo,v,L,v,L,v,o,o,v,o,v,L,v,L,v,L,v,o,o,v,o,v,L,v,L,v,o,e,o,v,v,o,v,e,oo,v,v,oo,v,vL,8,v,v7y^y{\n"
+"              K8,v,oo,v,vL,vv8,v8,v,Lv8,v,o0,oo,v,v,oo,v,vL,v,o,v,o,v,L,v,Lv8,v,o,v,o,o,v,oo,v,Lv8,v,L1,v,ov,o,v,oo,v,Lv8,v,Lv4,v,Lv8,v,oeeeeyv,o,v,oo,v,Lv8,v,Lv8,v,o,e,oo,vy,v,oo,v,e,o,v,v,o,v,vL,8,v8,vy^y{\n"
+"               K4,v,oo,v,vL,vv8,vee8,v,Lv8,v,o9,oo,voyyy,v,oo,v,vL,8,vv8,v9y^yI{\n"
+"                K7,v,oo,v,vL,8,vvv,v3,v,oe1,o,v\n"
+"               }\n"
+"              }\n"
+"             }\n"
+"            }}\n"
+"           }}}\n"
+"          }}}}\n"
+"         }}}}}}}\n"
+"        }}}}}}\n"
+"       }}}}}}\n"
+"      }}}}}}}}\n"
+"     }\n"
+"    }\n"
+"   }\n"
+"  }}}}\n"
+" }\n"
+"}}\n"
+"\n"
+"}\n";
+
+/*
+ * lua_close is a part of testcase, so testcase creates
+ * it's own Lua state and closes it at the end.
+ */
+static int lua_cdata_finalizers_testcase_part_1(void *test_state)
+{
+	/* Shared Lua state is not needed. */
+	(void)test_state;
+
+	/* Setup. */
+	lua_State *L = luaL_newstate();
+
+	luaL_loadbufferx(L, chunk, sizeof(chunk), "test", "t");
+
+	/* Teardown. */
+	lua_settop(L, 0);
+	lua_close(L);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+int main(void)
+{
+	const struct test_unit tgroup[] = {
+		test_unit_def(lua_cdata_finalizers_testcase_part_1),
+	};
+	const int test_result = test_run_group(tgroup, NULL);
+
+	return test_result;
+}
diff --git a/test/tarantool-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.lua b/test/tarantool-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.lua
new file mode 100644
index 00000000..fca5ec76
--- /dev/null
+++ b/test/tarantool-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.lua
@@ -0,0 +1,18 @@
+local tap = require('tap')
+
+-- This test demonstrates LuaJIT's heap-use-after-free
+-- on collecting garbage. Test simulates "unloading" of the library,
+-- or removing some of the functionality of it and then calls
+-- `collectgarbage`.
+-- See https://github.com/LuaJIT/LuaJIT/issues/1168 for details.
+local test = tap.test('lj-1168-heap-use-after-free-on-access-to-CTState-finalizer')
+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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [Tarantool-patches] [PATCH luajit 2/2] FFI: Turn FFI finalizer table into a proper GC root.
  2024-07-08 12:26 [Tarantool-patches] [PATCH luajit 0/2] Fix cdata finalizer table Sergey Bronnikov via Tarantool-patches
  2024-07-08 12:26 ` [Tarantool-patches] [PATCH luajit 1/2] FFI: Treat cdata finalizer table as a GC root Sergey Bronnikov via Tarantool-patches
@ 2024-07-08 12:26 ` Sergey Bronnikov via Tarantool-patches
  1 sibling, 0 replies; 3+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-07-08 12:26 UTC (permalink / raw)
  To: tarantool-patches, Sergey Kaplun, Maxim Kokryashkin

From: Mike Pall <mike>

Reported by Sergey Bronnikov.

(cherry picked from commit f5affaa6c4e7524e661484f22f24255f9a83eb47)

Previous patch fixes the problem partially because the introduced
GC root may not exist at the start phase of the GC cycle. In that
case, the cdata finalizer table will be collected at the end of
the cycle. Access to the cdata finalizer table exhibits heap use
after free. The patch is turned the finalizer table into a proper
GC root.

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

Part of tarantool/tarantool#10199
---
 src/lib_ffi.c                                 | 20 +--------
 src/lj_cdata.c                                |  2 +-
 src/lj_ctype.c                                | 12 ++++++
 src/lj_ctype.h                                |  2 +-
 src/lj_gc.c                                   | 41 ++++++++----------
 src/lj_obj.h                                  |  3 ++
 src/lj_state.c                                |  3 ++
 ...free-on-access-to-CTState-finalizer.test.c | 42 +++++++++++++++++++
 8 files changed, 81 insertions(+), 44 deletions(-)

diff --git a/src/lib_ffi.c b/src/lib_ffi.c
index 7ed6fc78..3c8dd77f 100644
--- a/src/lib_ffi.c
+++ b/src/lib_ffi.c
@@ -513,7 +513,7 @@ LJLIB_CF(ffi_new)	LJLIB_REC(.)
     /* Handle ctype __gc metamethod. Use the fast lookup here. */
     cTValue *tv = lj_tab_getinth(cts->miscmap, -(int32_t)id);
     if (tv && tvistab(tv) && (tv = lj_meta_fast(L, tabV(tv), MM_gc))) {
-      GCtab *t = cts->finalizer;
+      GCtab *t = tabref(G(L)->gcroot[GCROOT_FFI_FIN]);
       if (gcref(t->metatable)) {
 	/* Add to finalizer table, if still enabled. */
 	copyTV(L, lj_tab_set(L, t, o-1), tv);
@@ -762,7 +762,7 @@ LJLIB_CF(ffi_abi)	LJLIB_REC(.)
   return 1;
 }
 
-LJLIB_PUSH(top-8) LJLIB_SET(!)  /* Store reference to miscmap table. */
+LJLIB_PUSH(top-7) LJLIB_SET(!)  /* Store reference to miscmap table. */
 
 LJLIB_CF(ffi_metatype)
 {
@@ -788,8 +788,6 @@ LJLIB_CF(ffi_metatype)
   return 1;
 }
 
-LJLIB_PUSH(top-7) LJLIB_SET(!)  /* Store reference to finalizer table. */
-
 LJLIB_CF(ffi_gc)	LJLIB_REC(.)
 {
   GCcdata *cd = ffi_checkcdata(L, 1);
@@ -822,19 +820,6 @@ LJLIB_PUSH(top-2) LJLIB_SET(arch)
 
 /* ------------------------------------------------------------------------ */
 
-/* Create special weak-keyed finalizer table. */
-static GCtab *ffi_finalizer(lua_State *L)
-{
-  /* NOBARRIER: The table is new (marked white). */
-  GCtab *t = lj_tab_new(L, 0, 1);
-  settabV(L, L->top++, t);
-  setgcref(t->metatable, obj2gco(t));
-  setstrV(L, lj_tab_setstr(L, t, lj_str_newlit(L, "__mode")),
-	  lj_str_newlit(L, "k"));
-  t->nomm = (uint8_t)(~(1u<<MM_mode));
-  return t;
-}
-
 /* Register FFI module as loaded. */
 static void ffi_register_module(lua_State *L)
 {
@@ -850,7 +835,6 @@ LUALIB_API int luaopen_ffi(lua_State *L)
 {
   CTState *cts = lj_ctype_init(L);
   settabV(L, L->top++, (cts->miscmap = lj_tab_new(L, 0, 1)));
-  cts->finalizer = ffi_finalizer(L);
   LJ_LIB_REG(L, NULL, ffi_meta);
   /* NOBARRIER: basemt is a GC root. */
   setgcref(basemt_it(G(L), LJ_TCDATA), obj2gco(tabV(L->top-1)));
diff --git a/src/lj_cdata.c b/src/lj_cdata.c
index 35d0e76a..3d6ff1cc 100644
--- a/src/lj_cdata.c
+++ b/src/lj_cdata.c
@@ -89,7 +89,7 @@ void LJ_FASTCALL lj_cdata_free(global_State *g, GCcdata *cd)
 
 void lj_cdata_setfin(lua_State *L, GCcdata *cd, GCobj *obj, uint32_t it)
 {
-  GCtab *t = ctype_ctsG(G(L))->finalizer;
+  GCtab *t = tabref(G(L)->gcroot[GCROOT_FFI_FIN]);
   if (gcref(t->metatable)) {
     /* Add cdata to finalizer table, if still enabled. */
     TValue *tv, tmp;
diff --git a/src/lj_ctype.c b/src/lj_ctype.c
index 53b83031..c0213629 100644
--- a/src/lj_ctype.c
+++ b/src/lj_ctype.c
@@ -643,6 +643,18 @@ CTState *lj_ctype_init(lua_State *L)
   return cts;
 }
 
+/* Create special weak-keyed finalizer table. */
+void lj_ctype_initfin(lua_State *L)
+{
+  /* NOBARRIER: The table is new (marked white). */
+  GCtab *t = lj_tab_new(L, 0, 1);
+  setgcref(t->metatable, obj2gco(t));
+  setstrV(L, lj_tab_setstr(L, t, lj_str_newlit(L, "__mode")),
+	  lj_str_newlit(L, "k"));
+  t->nomm = (uint8_t)(~(1u<<MM_mode));
+  setgcref(G(L)->gcroot[GCROOT_FFI_FIN], obj2gco(t));
+}
+
 /* Free C type table and state. */
 void lj_ctype_freestate(global_State *g)
 {
diff --git a/src/lj_ctype.h b/src/lj_ctype.h
index 8edbd561..2d393eb9 100644
--- a/src/lj_ctype.h
+++ b/src/lj_ctype.h
@@ -177,7 +177,6 @@ typedef struct CTState {
   MSize sizetab;	/* Size of C type table. */
   lua_State *L;		/* Lua state (needed for errors and allocations). */
   global_State *g;	/* Global state. */
-  GCtab *finalizer;	/* Map of cdata to finalizer. */
   GCtab *miscmap;	/* Map of -CTypeID to metatable and cb slot to func. */
   CCallback cb;		/* Temporary callback state. */
   CTypeID1 hash[CTHASH_SIZE];  /* Hash anchors for C type table. */
@@ -473,6 +472,7 @@ LJ_FUNC GCstr *lj_ctype_repr(lua_State *L, CTypeID id, GCstr *name);
 LJ_FUNC GCstr *lj_ctype_repr_int64(lua_State *L, uint64_t n, int isunsigned);
 LJ_FUNC GCstr *lj_ctype_repr_complex(lua_State *L, void *sp, CTSize size);
 LJ_FUNC CTState *lj_ctype_init(lua_State *L);
+LJ_FUNC void lj_ctype_initfin(lua_State *L);
 LJ_FUNC void lj_ctype_freestate(global_State *g);
 
 #endif
diff --git a/src/lj_gc.c b/src/lj_gc.c
index 42348a34..4c222f21 100644
--- a/src/lj_gc.c
+++ b/src/lj_gc.c
@@ -99,9 +99,6 @@ 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;
 }
 
@@ -181,8 +178,7 @@ static int gc_traverse_tab(global_State *g, GCtab *t)
     }
     if (weak) {  /* Weak tables are cleared in the atomic phase. */
 #if LJ_HASFFI
-      CTState *cts = ctype_ctsG(g);
-      if (cts && cts->finalizer == t) {
+      if (gcref(g->gcroot[GCROOT_FFI_FIN]) == obj2gco(t)) {
 	weak = (int)(~0u & ~LJ_GC_WEAKVAL);
       } else
 #endif
@@ -550,7 +546,7 @@ static void gc_finalize(lua_State *L)
     o->gch.marked &= (uint8_t)~LJ_GC_CDATA_FIN;
     /* Resolve finalizer. */
     setcdataV(L, &tmp, gco2cd(o));
-    tv = lj_tab_set(L, ctype_ctsG(g)->finalizer, &tmp);
+    tv = lj_tab_set(L, tabref(g->gcroot[GCROOT_FFI_FIN]), &tmp);
     if (!tvisnil(tv)) {
       g->gc.nocdatafin = 0;
       copyTV(L, &tmp, tv);
@@ -582,23 +578,20 @@ void lj_gc_finalize_udata(lua_State *L)
 void lj_gc_finalize_cdata(lua_State *L)
 {
   global_State *g = G(L);
-  CTState *cts = ctype_ctsG(g);
-  if (cts) {
-    GCtab *t = cts->finalizer;
-    Node *node = noderef(t->node);
-    ptrdiff_t i;
-    setgcrefnull(t->metatable);  /* Mark finalizer table as disabled. */
-    for (i = (ptrdiff_t)t->hmask; i >= 0; i--)
-      if (!tvisnil(&node[i].val) && tviscdata(&node[i].key)) {
-	GCobj *o = gcV(&node[i].key);
-	TValue tmp;
-	makewhite(g, o);
-	o->gch.marked &= (uint8_t)~LJ_GC_CDATA_FIN;
-	copyTV(L, &tmp, &node[i].val);
-	setnilV(&node[i].val);
-	gc_call_finalizer(g, L, &tmp, o);
-      }
-  }
+  GCtab *t = tabref(g->gcroot[GCROOT_FFI_FIN]);
+  Node *node = noderef(t->node);
+  ptrdiff_t i;
+  setgcrefnull(t->metatable);  /* Mark finalizer table as disabled. */
+  for (i = (ptrdiff_t)t->hmask; i >= 0; i--)
+    if (!tvisnil(&node[i].val) && tviscdata(&node[i].key)) {
+      GCobj *o = gcV(&node[i].key);
+      TValue tmp;
+      makewhite(g, o);
+      o->gch.marked &= (uint8_t)~LJ_GC_CDATA_FIN;
+      copyTV(L, &tmp, &node[i].val);
+      setnilV(&node[i].val);
+      gc_call_finalizer(g, L, &tmp, o);
+    }
 }
 #endif
 
@@ -721,7 +714,7 @@ static size_t gc_onestep(lua_State *L)
       return GCFINALIZECOST;
     }
 #if LJ_HASFFI
-    if (!g->gc.nocdatafin) lj_tab_rehash(L, ctype_ctsG(g)->finalizer);
+    if (!g->gc.nocdatafin) lj_tab_rehash(L, tabref(g->gcroot[GCROOT_FFI_FIN]));
 #endif
     g->gc.state = GCSpause;  /* End of GC cycle. */
     g->gc.debt = 0;
diff --git a/src/lj_obj.h b/src/lj_obj.h
index 69e94ff2..06ea0cd0 100644
--- a/src/lj_obj.h
+++ b/src/lj_obj.h
@@ -580,6 +580,9 @@ typedef enum {
   GCROOT_BASEMT_NUM = GCROOT_BASEMT + ~LJ_TNUMX,
   GCROOT_IO_INPUT,	/* Userdata for default I/O input file. */
   GCROOT_IO_OUTPUT,	/* Userdata for default I/O output file. */
+#if LJ_HASFFI
+  GCROOT_FFI_FIN,	/* FFI finalizer table. */
+#endif
   GCROOT_MAX
 } GCRootID;
 
diff --git a/src/lj_state.c b/src/lj_state.c
index 01d4901a..5a920102 100644
--- a/src/lj_state.c
+++ b/src/lj_state.c
@@ -180,6 +180,9 @@ static TValue *cpluaopen(lua_State *L, lua_CFunction dummy, void *ud)
   lj_lex_init(L);
   fixstring(lj_err_str(L, LJ_ERR_ERRMEM));  /* Preallocate memory error msg. */
   g->gc.threshold = 4*g->gc.total;
+#if LJ_HASFFI
+  lj_ctype_initfin(L);
+#endif
   lj_trace_initstate(g);
   lj_err_verify();
   return NULL;
diff --git a/test/tarantool-c-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.c b/test/tarantool-c-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.c
index ad2d8e62..50359a98 100644
--- a/test/tarantool-c-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.c
+++ b/test/tarantool-c-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.c
@@ -94,10 +94,52 @@ static int lua_cdata_finalizers_testcase_part_1(void *test_state)
 	return TEST_EXIT_SUCCESS;
 }
 
+static int
+lua_cdata_finalizers_testcase_part_2(void *test_state)
+{
+	const char buff[] = "return 1LL";
+
+	/* Shared Lua state is not needed. */
+	(void)test_state;
+
+	/* Setup. */
+	lua_State *L = luaL_newstate();
+
+	/* Set GC at the start. */
+	lua_gc(L, LUA_GCCOLLECT, 0);
+
+	/*
+	 * Default step is too big -- one step ends after the
+	 * atomic phase.
+	 */
+	lua_gc(L, LUA_GCSETSTEPMUL, 1);
+
+	/* Skip marking roots. */
+	lua_gc(L, LUA_GCSTEP, 1);
+
+	/* Not trigger GC during `lua_openffi()`. */
+	lua_gc(L, LUA_GCSTOP, 0);
+
+	int res = luaL_loadbufferx(L, buff, sizeof(buff) - 1, "chunk", "t");
+	assert_true(res == LUA_OK);
+
+	/* Finish GC cycle. */
+	while (!lua_gc(L, LUA_GCSTEP, -1));
+
+	assert_true(lua_gettop(L) == 1);
+
+	/* Teardown. */
+	lua_settop(L, 0);
+	lua_close(L);
+
+	return TEST_EXIT_SUCCESS;
+}
+
 int main(void)
 {
 	const struct test_unit tgroup[] = {
 		test_unit_def(lua_cdata_finalizers_testcase_part_1),
+		test_unit_def(lua_cdata_finalizers_testcase_part_2),
 	};
 	const int test_result = test_run_group(tgroup, NULL);
 
-- 
2.34.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-07-08 12:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-08 12:26 [Tarantool-patches] [PATCH luajit 0/2] Fix cdata finalizer table Sergey Bronnikov via Tarantool-patches
2024-07-08 12:26 ` [Tarantool-patches] [PATCH luajit 1/2] FFI: Treat cdata finalizer table as a GC root Sergey Bronnikov via Tarantool-patches
2024-07-08 12:26 ` [Tarantool-patches] [PATCH luajit 2/2] FFI: Turn FFI finalizer table into a proper " Sergey Bronnikov via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox