Re: [Tarantool-patches] [PATCH luajit] Fix stack overflow relimit handling.

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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

Hello,

Thanks for the patch! LGTM

Sergey

On 7/20/26 11:52, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Thanks to Sergey Kaplun.
>
> (cherry picked from commit d2c1327f57dc96f6ed8b11474f3bc5e09a9d227a)
>
> If the error handler can't fit to the top of the Lua stack, the stack is
> resized up to `LJ_STACK_MAXEX + 1 + 2 * LUA_MINSTACK` hoping that
> `lj_state_relimitstack()` shrinks it back. In case when another error is
> (gently) handled in the error handler (`pcall(error)` or failed
> `load()`) the stack is shrunk to `LJ_STACK_MAX` leaving the rest of the
> handler to be executed on the clipped stack, which leads to the
> heap-overflow or a crash.
>
> This patch fixes the issue by hardening the relimiting border to match
> the extra space allocated before the stack overflow. Also, it fixes some
> typos in the comments.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#12880
> ---
>
> The test is leading to the core dump on GC64 build.
>
> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1471-fix-stackov-relimit
> Related issues:
> *https://github.com/LuaJIT/LuaJIT/issues/1471
> *https://github.com/tarantool/tarantool/issues/12880
>
>   src/lj_state.c                                | 17 ++++---
>   .../lj-1471-fix-stackov-relimit.test.lua      | 48 +++++++++++++++++++
>   2 files changed, 59 insertions(+), 6 deletions(-)
>   create mode 100644 test/tarantool-tests/lj-1471-fix-stackov-relimit.test.lua
>
> diff --git a/src/lj_state.c b/src/lj_state.c
> index 2fa6a2f6..ea0f24f8 100644
> --- a/src/lj_state.c
> +++ b/src/lj_state.c
> @@ -44,8 +44,9 @@
>   #define LJ_STACK_MAX	LUAI_MAXSTACK	/* Max. stack size. */
>   #define LJ_STACK_START	(2*LJ_STACK_MIN)	/* Starting stack size. */
>   #define LJ_STACK_MAXEX	(LJ_STACK_MAX + 1 + LJ_STACK_EXTRA)
> +#define LJ_STACK_ERREX	(1 + 2*LJ_STACK_MIN)	/* Extra for error handling. */
>   
> -/* Explanation of LJ_STACK_EXTRA:
> +/* Explanation for LJ_STACK_EXTRA:
>   **
>   ** Calls to metamethods store their arguments beyond the current top
>   ** without checking for the stack limit. This avoids stack resizes which
> @@ -58,6 +59,11 @@
>   ** slots above top, but then mobj is always a function. So we can get by
>   ** with 5 extra slots.
>   ** LJ_FR2: We need 2 more slots for the frame PC and the continuation PC.
> +**
> +** Explanation for LJ_STACK_ERREX:
> +**
> +** The 1 is space for the error message, and 2 * LJ_STACK_MIN is for
> +** the lj_state_checkstack() call in lj_err_run().
>   */
>   
>   /* Resize stack slots and adjust pointers in state. */
> @@ -101,7 +107,8 @@ static void resizestack(lua_State *L, MSize n)
>   /* Relimit stack after error, in case the limit was overdrawn. */
>   void lj_state_relimitstack(lua_State *L)
>   {
> -  if (L->stacksize > LJ_STACK_MAXEX && L->top-tvref(L->stack) < LJ_STACK_MAX-1)
> +  if (L->stacksize > LJ_STACK_MAXEX &&
> +      L->top-tvref(L->stack) < LJ_STACK_MAX - 1 - LJ_STACK_ERREX)
>       resizestack(L, LJ_STACK_MAX);
>   }
>   
> @@ -147,11 +154,9 @@ void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need)
>         /* An error handler might want to inspect the stack overflow error, but
>         ** will need some stack space to run in. We give it a stack size beyond
>         ** the normal limit in order to do so, then rely on lj_state_relimitstack
> -      ** calls during unwinding to bring us back to a convential stack size.
> -      ** The + 1 is space for the error message, and 2 * LUA_MINSTACK is for
> -      ** the lj_state_checkstack() call in lj_err_run().
> +      ** calls during unwinding to bring us back to a conventional stack size.
>         */
> -      resizestack(L, LJ_STACK_MAX + 1 + 2 * LUA_MINSTACK);
> +      resizestack(L, LJ_STACK_MAX + LJ_STACK_ERREX);
>         lj_err_stkov(L);  /* May invoke an error handler. */
>       } else {
>         /* If we're here, then the stack overflow error handler is requesting
> diff --git a/test/tarantool-tests/lj-1471-fix-stackov-relimit.test.lua b/test/tarantool-tests/lj-1471-fix-stackov-relimit.test.lua
> new file mode 100644
> index 00000000..6a10cc20
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1471-fix-stackov-relimit.test.lua
> @@ -0,0 +1,48 @@
> +local tap = require('tap')
> +
> +-- The test file demonstrates a heap-overflow due to Lua stack
> +-- out-of-bounds access after an incorrect stack shrinking after
> +-- unwinding.
> +-- See alsohttps://github.com/LuaJIT/LuaJIT/issues/1471.
> +
> +local test = tap.test('lj-1471-fix-stackov-relimit')
> +
> +test:plan(1)
> +
> +local function recursive_add(v1, v2)
> +  -- Slot to eat the stack.
> +  -- luacheck: no unused
> +  local _
> +  recursive_add(v1, v2)
> +end
> +
> +local table_mt = {
> +  __add = recursive_add,
> +}
> +
> +local t1 = setmetatable({}, table_mt)
> +local t2 = setmetatable({}, table_mt)
> +
> +coroutine.wrap(function()
> +  xpcall(error, function()
> +    pcall(error) -- Shrink stack back after unwinding.
> +    -- XXX: Empirical amount of stack slots to observe the issue.
> +    -- After the stack overflow, the `xpcall()` handler is invoked
> +    -- again (seehttps://github.com/LuaJIT/LuaJIT/issues/1382).
> +    -- The stack is overallocated beyond its normal limit to
> +    -- handle the error. After the `pcall(error)`, stack is
> +    -- shrinking back to its normal size, leaving no space for
> +    -- metamethod invocation. It is leaving to heap-overflow and a
> +    -- crash.
> +    -- luacheck: no unused
> +    local _, _, _, _, _, _, _, _, _, _
> +    local _, _, _, _, _, _, _, _, _, _
> +    local _, _, _, _, _, _, _, _, _, _
> +    local _
> +    local _ = t1 + t2
> +  end)
> +end)()
> +
> +test:ok(true, 'no heap overflow after stack relimiting')
> +
> +test:done(true)

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