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

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

[Sergey Bronnikov via Tarantool-patches]

From: sergeyb@tarantool.org

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#9595
---
Branch: https://github.com/tarantool/luajit/tree/ligurio/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer
PR: https://github.com/tarantool/tarantool/pull/9796
Issues:
- https://github.com/LuaJIT/LuaJIT/issues/1168
- https://github.com/tarantool/tarantool/issues/9595

 src/lj_gc.c                                   |   3 +
 ...free-on-access-to-CTState-finalizer.test.c | 106 ++++++++++++++++++
 ...ee-on-access-to-CTState-finalizer.test.lua |  18 +++
 3 files changed, 127 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..cdc688c3
--- /dev/null
+++ b/test/tarantool-c-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.c
@@ -0,0 +1,106 @@
+#include <string.h>
+
+#include "lua.h"
+#include "lauxlib.h"
+
+#include "test.h"
+#include "utils.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(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),
+	};
+	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