FFI: Turn FFI finalizer table into a proper GC root.

Reported by Sergey Bronnikov.

(cherry picked from commit f5affaa)

Previous patch fixes the problem partially because the introduced
GC root may not exist at the start phase of the GC cycle (since it
isn't marked because it is not accessible from any GC root).
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 turns the finalizer table into
a proper GC root. Note, that finalizer table is created on the
initialization of the main Lua State instead of loading the FFI
library.

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

Part of tarantool/tarantool#10199

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)));

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;

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)
 {

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

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;

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;
 

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;

test/tarantool-c-tests/lj-1168-unmarked-finalizer-tab.test.c

@@ -65,10 +65,57 @@ static int unmarked_finalizer_tab_gcstart(void *test_state)
 	return TEST_EXIT_SUCCESS;
 }
 
+static int
+unmarked_finalizer_tab_gcmark(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);
+
+	/*
+	 * 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);
+
+	/*
+	 * 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),
+		test_unit_def(unmarked_finalizer_tab_gcmark),
 	};
 	const int test_result = test_run_group(tgroup, NULL);