[Tarantool-patches] [PATCH luajit] FFI: Avoid dangling cts->L.

[Tarantool-patches] [PATCH luajit] FFI: Avoid dangling cts->L.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Reported by ZumiKua.

(cherry picked from commit c94312d348e3530b369b4e517fce4c65df6cd270)

When executing cdata finalizer on some lua_State, this state is set to
`cts->L`. If the state will be GC-ed and freed later, this reference
becomes dangling, so any FFI callback will use this invalid reference.

This patch fixes it by setting `cts->L` to the `mainthread` on the
destruction of the referenced one.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#12134
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1405-dangling-cts-L
Related issues:
* https://github.com/LuaJIT/LuaJIT/issues/1405
* https://github.com/tarantool/tarantool/issues/12134

 src/lj_state.c                                |  4 ++
 test/tarantool-c-tests/CMakeLists.txt         |  2 +
 .../lj-1405-dangling-cts-L.test.c             | 72 +++++++++++++++++++
 3 files changed, 78 insertions(+)
 create mode 100644 test/tarantool-c-tests/lj-1405-dangling-cts-L.test.c

diff --git a/src/lj_state.c b/src/lj_state.c
index 053e5ec9..2eec5857 100644
--- a/src/lj_state.c
+++ b/src/lj_state.c
@@ -360,6 +360,10 @@ void LJ_FASTCALL lj_state_free(global_State *g, lua_State *L)
   lj_assertG(L != mainthread(g), "free of main thread");
   if (obj2gco(L) == gcref(g->cur_L))
     setgcrefnull(g->cur_L);
+#if LJ_HASFFI
+  if (ctype_ctsG(g) && ctype_ctsG(g)->L == L)  /* Avoid dangling cts->L. */
+    ctype_ctsG(g)->L = mainthread(g);
+#endif
   lj_func_closeuv(L, tvref(L->stack));
   lj_assertG(gcref(L->openupval) == NULL, "stale open upvalues");
   lj_mem_freevec(g, tvref(L->stack), L->stacksize, TValue);
diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt
index 32a8add0..3bb20bff 100644
--- a/test/tarantool-c-tests/CMakeLists.txt
+++ b/test/tarantool-c-tests/CMakeLists.txt
@@ -59,6 +59,8 @@ foreach(test_source ${tests})
     OUTPUT_NAME "${exe}${C_TEST_SUFFIX}"
     RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
   )
+  # Allow to call non-static functions via FFI.
+  target_link_options(${exe} PRIVATE "-rdynamic")
   target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY})
   add_dependencies(tarantool-c-tests-build ${exe})
 
diff --git a/test/tarantool-c-tests/lj-1405-dangling-cts-L.test.c b/test/tarantool-c-tests/lj-1405-dangling-cts-L.test.c
new file mode 100644
index 00000000..b4ed4970
--- /dev/null
+++ b/test/tarantool-c-tests/lj-1405-dangling-cts-L.test.c
@@ -0,0 +1,72 @@
+#include "lua.h"
+
+#include "test.h"
+#include "utils.h"
+
+/* XXX: Still need normal assert inside `call_callback()`. */
+#undef NDEBUG
+#include <assert.h>
+
+typedef void (*callback_t)(void);
+static callback_t callback = NULL;
+
+/* Function to be called via FFI. */
+extern void add_callback(callback_t cb)
+{
+	callback = cb;
+}
+
+static void call_callback(void)
+{
+	assert(callback != NULL);
+	callback();
+}
+
+static int dangling_cts_L(void *test_state)
+{
+	lua_State *L = utils_lua_init();
+	luaopen_ffi(L);
+	const char code[] = {
+		" local ffi = require('ffi')                               \n" \
+		" ffi.cdef [[                                              \n" \
+		"   struct test { int a; };                                \n" \
+		"   void add_callback(void (*cb)(void a));                 \n" \
+		    /* Simple finalizer, nop. */
+		"   int getpid(void);                                      \n" \
+		" ]]                                                       \n" \
+		"                                                          \n" \
+		" local C = ffi.C                                          \n" \
+		"                                                          \n" \
+		" local function nop() end                                 \n" \
+		/* Collected later. Set `cts->L` in the finalizer. */
+		" ffi.gc(ffi.new('struct test'), C.getpid);                \n" \
+		/* Callback to be called on the old `cts->L`. */
+		" C.add_callback(ffi.cast('void (*)(void)', nop))          \n"
+	};
+	if (luaL_dostring(L, code) != LUA_OK) {
+		test_comment("error running Lua chunk: %s",
+			     lua_tostring(L, -1));
+		bail_out("error running Lua chunk");
+	}
+	lua_State* newL = lua_newthread(L);
+	/* Remove `newL` from `L`. */
+	lua_pop(L, 1);
+	/* Set `cts->L = newL` in the finalizer. */
+	lua_gc(newL, LUA_GCCOLLECT, 0);
+	/* Just to be sure we don't use it anymore. */
+	newL = NULL;
+	/* Collect `newL`. */
+	lua_gc(L, LUA_GCCOLLECT, 0);
+	/* Use after free before the patch. */
+	call_callback();
+	utils_lua_close(L);
+	return TEST_EXIT_SUCCESS;
+}
+
+int main(void)
+{
+	const struct test_unit tgroup[] = {
+		test_unit_def(dangling_cts_L),
+	};
+	return test_run_group(tgroup, NULL);
+}
-- 
2.53.0