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

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

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 also https://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 (see https://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)
-- 
2.55.0