[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

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

[Sergey Bronnikov via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 5179 bytes --]

Hi, Sergey,

thanks for the patch!

The bug is not reproduced without patch.

CMake configuration: cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug

Sergey

On 2/25/26 14:57, Sergey Kaplun wrote:
> 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);
> +}

[-- Attachment #2: Type: text/html, Size: 5748 bytes --]

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

[Sergey Kaplun via Tarantool-patches]

Hi, Sergey!
Thanks for the review!

On 03.03.26, Sergey Bronnikov wrote:
> Hi, Sergey,
> 
> thanks for the patch!
> 
> The bug is not reproduced without patch.
> 
> CMake configuration: cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug

Hi, Sergey!
I suppose you need to build LuaJIT with enabled ASAN check to catch
dangling pointer.

> 
> Sergey
> 

<snipped>

-- 
Best regards,
Sergey Kaplun