[Tarantool-patches] [PATCH luajit] Fix error generation in load*.

[Tarantool-patches] [PATCH luajit] Fix error generation in load*.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Reported by Sergey Kaplun.

(cherry picked from commit e76bb50d44702f601ec5dd167b03b475ed53860c)

The chunkname pointer to the "@filename" is put on the Lua stack
before the `lua_loadx()` and is removed right before the next
`lua_pushfstring()` in case of the error. If the GC takes the step
right at this moment inside `lua_pushfstring()` the string may be
collected, and the next read from this `chunkname + 1` is from the
deallocated memory.

This patch fixes this by using the source string (or the constant one)
instead.

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

Part of tarantool/tarantool#11278
---

Related issues:
* https://github.com/LuaJIT/LuaJIT/issues/1353
* https://github.com/tarantool/tarantool/issues/11278
Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1353-loadfile-err-use-after-free


 src/lj_load.c                                 |  3 +-
 ...-1353-loadfile-err-use-after-free.test.lua | 39 +++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/lj-1353-loadfile-err-use-after-free.test.lua

diff --git a/src/lj_load.c b/src/lj_load.c
index fdbc54cb..205686bb 100644
--- a/src/lj_load.c
+++ b/src/lj_load.c
@@ -108,8 +108,9 @@ LUALIB_API int luaL_loadfilex(lua_State *L, const char *filename,
     copyTV(L, L->top-1, L->top);
   }
   if (err) {
+    const char *fname = filename ? filename : "stdin";
     L->top--;
-    lua_pushfstring(L, "cannot read %s: %s", chunkname+1, strerror(err));
+    lua_pushfstring(L, "cannot read %s: %s", fname, strerror(err));
     return LUA_ERRFILE;
   }
   return status;
diff --git a/test/tarantool-tests/lj-1353-loadfile-err-use-after-free.test.lua b/test/tarantool-tests/lj-1353-loadfile-err-use-after-free.test.lua
new file mode 100644
index 00000000..3aa02f3b
--- /dev/null
+++ b/test/tarantool-tests/lj-1353-loadfile-err-use-after-free.test.lua
@@ -0,0 +1,39 @@
+local tap = require('tap')
+
+-- Test file to demonstrate LuaJIT use-after-free in case of the
+-- error in `loadfile()`.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1353.
+local test = tap.test('lj-1353-loadfile-err-use-after-free'):skipcond({
+  ['Too many GC objects on start'] = _TARANTOOL,
+})
+
+test:plan(1)
+
+-- Determine the GC step size to finish the GC cycle in one step.
+local full_step = 1
+while true do
+  collectgarbage('collect')
+  collectgarbage('setpause', 0)
+  collectgarbage('setstepmul', full_step)
+  if collectgarbage('step') then break end
+  full_step = full_step + 1
+end
+
+-- Check all possible GC step sizes.
+for i = 1, full_step do
+  collectgarbage('collect')
+  collectgarbage('setpause', 0)
+  collectgarbage('setstepmul', i)
+  repeat
+    -- On Linux-like systems this always returns `nil`, with the
+    -- error: "cannot read .: Is a directory"
+    -- The string for the filename "@." may be collected during
+    -- the call, and later the pointer to the "." from that string
+    -- is used after the string is free.
+    loadfile('.')
+  until collectgarbage('step')
+end
+
+test:ok(true, 'no use-after-free error')
+
+test:done(true)
-- 
2.49.0

Re: [Tarantool-patches] [PATCH luajit] Fix error generation in load*.

[Sergey Bronnikov via Tarantool-patches]

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

Hi, Sergey!

thanks for the patch! LGTM

Sergey

On 6/10/25 18:59, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by Sergey Kaplun.
>
> (cherry picked from commit e76bb50d44702f601ec5dd167b03b475ed53860c)
>
> The chunkname pointer to the "@filename" is put on the Lua stack
> before the `lua_loadx()` and is removed right before the next
> `lua_pushfstring()` in case of the error. If the GC takes the step
> right at this moment inside `lua_pushfstring()` the string may be
> collected, and the next read from this `chunkname + 1` is from the
> deallocated memory.
>
> This patch fixes this by using the source string (or the constant one)
> instead.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#11278
> ---
>
> Related issues:
> *https://github.com/LuaJIT/LuaJIT/issues/1353
> *https://github.com/tarantool/tarantool/issues/11278
> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1353-loadfile-err-use-after-free
>
>
>   src/lj_load.c                                 |  3 +-
>   ...-1353-loadfile-err-use-after-free.test.lua | 39 +++++++++++++++++++
>   2 files changed, 41 insertions(+), 1 deletion(-)
>   create mode 100644 test/tarantool-tests/lj-1353-loadfile-err-use-after-free.test.lua
>
> diff --git a/src/lj_load.c b/src/lj_load.c
> index fdbc54cb..205686bb 100644
> --- a/src/lj_load.c
> +++ b/src/lj_load.c
> @@ -108,8 +108,9 @@ LUALIB_API int luaL_loadfilex(lua_State *L, const char *filename,
>       copyTV(L, L->top-1, L->top);
>     }
>     if (err) {
> +    const char *fname = filename ? filename : "stdin";
>       L->top--;
> -    lua_pushfstring(L, "cannot read %s: %s", chunkname+1, strerror(err));
> +    lua_pushfstring(L, "cannot read %s: %s", fname, strerror(err));
>       return LUA_ERRFILE;
>     }
>     return status;
> diff --git a/test/tarantool-tests/lj-1353-loadfile-err-use-after-free.test.lua b/test/tarantool-tests/lj-1353-loadfile-err-use-after-free.test.lua
> new file mode 100644
> index 00000000..3aa02f3b
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1353-loadfile-err-use-after-free.test.lua
> @@ -0,0 +1,39 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate LuaJIT use-after-free in case of the
> +-- error in `loadfile()`.
> +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1353.
> +local test = tap.test('lj-1353-loadfile-err-use-after-free'):skipcond({
> +  ['Too many GC objects on start'] = _TARANTOOL,
> +})
> +
> +test:plan(1)
> +
> +-- Determine the GC step size to finish the GC cycle in one step.
> +local full_step = 1
> +while true do
> +  collectgarbage('collect')
> +  collectgarbage('setpause', 0)
> +  collectgarbage('setstepmul', full_step)
> +  if collectgarbage('step') then break end
> +  full_step = full_step + 1
> +end
> +
> +-- Check all possible GC step sizes.
> +for i = 1, full_step do
> +  collectgarbage('collect')
> +  collectgarbage('setpause', 0)
> +  collectgarbage('setstepmul', i)
> +  repeat
> +    -- On Linux-like systems this always returns `nil`, with the
> +    -- error: "cannot read .: Is a directory"
> +    -- The string for the filename "@." may be collected during
> +    -- the call, and later the pointer to the "." from that string
> +    -- is used after the string is free.
> +    loadfile('.')
> +  until collectgarbage('step')
> +end
> +
> +test:ok(true, 'no use-after-free error')
> +
> +test:done(true)

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