[Tarantool-patches] [PATCH luajit 2/3][v3] LJ_FR2: Fix stack checks in vararg calls.

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

From: Mike Pall <mike>

Thanks to Peter Cawley.

(cherry picked from commit d1a2fef8a8f53b0055ee041f7f63d83a27444ffa)

Stack overflow can cause a segmentation fault in a vararg
function on ARM64 and MIPS64 in LJ_FR2 mode. This happens
because the stack check in BC_IFUNCV is off by one on these
platforms without the patch. The original stack check
for ARM64 and MIPS64 was incorrect:

| RA == BASE + (RD=NARGS)*8 + framesize * 8 >= maxstack

while the stack check on x86_64 is correct and therefore is
not affected by the problem:

| RA == BASE + (RD=NARGS+1)*8 + framesize * 8 +8 > maxstack

The patch partially fixes the aforementioned issue by bumping
LJ_STACK_EXTRA by 1 to give a space to the entire frame link for a
vararg function as the __newindex metamethod.

A fixup for a number of required slots in `call_init()` was added
for consistency with non-GC64 flavor. The check is too strict, so
this can't lead to any crash.

This patch also corrects the number of redzone slots in
luajit-gdb.py to match the updated LJ_STACK_EXTRA and adds the test
<gh-1402-call_init-regression.test.lua> that will help to avoid
a regression in the future, see details in [1].

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

Part of tarantool/tarantool#12134

1. https://github.com/LuaJIT/LuaJIT/issues/1402
---
 src/lj_def.h                                  |  2 +-
 src/lj_dispatch.c                             |  2 +-
 src/luajit-gdb.py                             |  2 +-
 src/vm_arm64.dasc                             |  1 +
 src/vm_mips64.dasc                            |  1 +
 .../gh-1402-call_init-regression.test.lua     | 36 +++++++++++++
 ...048-fix-stack-checks-vararg-calls.test.lua | 53 +++++++++++++++++++
 7 files changed, 94 insertions(+), 3 deletions(-)
 create mode 100644 test/tarantool-tests/gh-1402-call_init-regression.test.lua
 create mode 100644 test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua

diff --git a/src/lj_def.h b/src/lj_def.h
index a5bca6b0..7e4f251e 100644
--- a/src/lj_def.h
+++ b/src/lj_def.h
@@ -69,7 +69,7 @@ typedef unsigned int uintptr_t;
 #define LJ_MAX_UPVAL	60		/* Max. # of upvalues. */
 
 #define LJ_MAX_IDXCHAIN	100		/* __index/__newindex chain limit. */
-#define LJ_STACK_EXTRA	(5+2*LJ_FR2)	/* Extra stack space (metamethods). */
+#define LJ_STACK_EXTRA	(5+3*LJ_FR2)	/* Extra stack space (metamethods). */
 
 #define LJ_NUM_CBPAGE	1		/* Number of FFI callback pages. */
 
diff --git a/src/lj_dispatch.c b/src/lj_dispatch.c
index a44a5adf..431cb3c2 100644
--- a/src/lj_dispatch.c
+++ b/src/lj_dispatch.c
@@ -453,7 +453,7 @@ static int call_init(lua_State *L, GCfunc *fn)
     int numparams = pt->numparams;
     int gotparams = (int)(L->top - L->base);
     int need = pt->framesize;
-    if ((pt->flags & PROTO_VARARG)) need += 1+gotparams;
+    if ((pt->flags & PROTO_VARARG)) need += 1+LJ_FR2+gotparams;
     lj_state_checkstack(L, (MSize)need);
     numparams -= gotparams;
     return numparams >= 0 ? numparams : 0;
diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
index 0ae2a6e0..dab07b35 100644
--- a/src/luajit-gdb.py
+++ b/src/luajit-gdb.py
@@ -542,7 +542,7 @@ def dump_stack(L, base=None, top=None):
     top = top or L['top']
     stack = mref('TValue *', L['stack'])
     maxstack = mref('TValue *', L['maxstack'])
-    red = 5 + 2 * LJ_FR2
+    red = 5 + 3 * LJ_FR2
 
     dump = [
         '{padding} Red zone: {nredslots: >2} slots {padding}'.format(
diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
index 6600e226..5ef37243 100644
--- a/src/vm_arm64.dasc
+++ b/src/vm_arm64.dasc
@@ -3779,6 +3779,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop)
     |   add TMP2, BASE, RC
     |   add LFUNC:CARG3, CARG3, TMP0, lsl #47
     |  add RA, RA, RC
+    |  sub CARG1, CARG1, #8
     |   add TMP0, RC, #16+FRAME_VARG
     |   str LFUNC:CARG3, [TMP2], #8	// Store (tagged) copy of LFUNC.
     |    ldr KBASE, [PC, #-4+PC2PROTO(k)]
diff --git a/src/vm_mips64.dasc b/src/vm_mips64.dasc
index da187a7a..6c2975b4 100644
--- a/src/vm_mips64.dasc
+++ b/src/vm_mips64.dasc
@@ -5268,6 +5268,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop)
     |   settp LFUNC:RB, TMP0
     |  daddu TMP0, RA, RC
     |   sd LFUNC:RB, 0(TMP1)		// Store (tagged) copy of LFUNC.
+    |  daddiu TMP2, TMP2, -8
     |   daddiu TMP3, RC, 16+FRAME_VARG
     |  sltu AT, TMP0, TMP2
     |    ld KBASE, -4+PC2PROTO(k)(PC)
diff --git a/test/tarantool-tests/gh-1402-call_init-regression.test.lua b/test/tarantool-tests/gh-1402-call_init-regression.test.lua
new file mode 100644
index 00000000..b20f9e39
--- /dev/null
+++ b/test/tarantool-tests/gh-1402-call_init-regression.test.lua
@@ -0,0 +1,36 @@
+local tap = require('tap')
+
+-- A test file to demonstrate a probably quite strict stack
+-- check for vararg functions in call_init.
+-- See also https://github.com/LuaJIT/LuaJIT/issues/1402
+local test = tap.test('gh-1402-call_init-regression.test.lua'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(1)
+
+local function vararg(...) -- luacheck: no unused
+  -- None.
+end
+
+-- Make compilation aggressive.
+jit.opt.start("hotloop=1")
+
+local function caller()
+  -- luacheck: push no unused
+  local _, _, _, _, _, _, _, _, _, _
+  local _, _, _, _, _, _, _, _, _, _
+  local _, _, _, _, _, _, _, _, _, _
+  -- luacheck: pop
+  local n = 1
+  while n < 3 do
+    vararg()
+    n = n + 1
+  end
+end
+
+pcall(coroutine.wrap(caller))
+
+test:ok(true, 'no assertion for vararg functions in call_init')
+
+test:done(true)
diff --git a/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua
new file mode 100644
index 00000000..3a8ad63d
--- /dev/null
+++ b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua
@@ -0,0 +1,53 @@
+local tap = require('tap')
+
+-- A test file to demonstrate a crash due to Lua stack
+-- out-of-bounds access, see below testcase descriptions.
+-- See also https://github.com/LuaJIT/LuaJIT/issues/1048.
+local test = tap.test('lj-1048-fix-stack-checks-vararg-calls')
+
+test:plan(2)
+
+-- The test case demonstrates a segmentation fault due to stack
+-- overflow by recursive calling `pcall()`. The functions are
+-- vararg because the stack check in BC_IFUNCV is off by one on
+-- ARM64 and MIPS64 without the patch.
+local function prober_1(...) -- luacheck: no unused
+  -- Any fast function can be used as metamethod, but `type` is
+  -- convenient here because it works fast and can be used with
+  -- any data type. Lua function cannot be used since it
+  -- will check the stack on each invocation. We need to check
+  -- using of the correct value LJ_STACK_EXTRA slots
+  -- (5+3*LJ_FR2) = 8 for GC64 mode.
+  pcall(pcall, pcall, pcall, pcall, pcall, pcall, pcall, pcall, type, 0)
+end
+
+local function looper(prober, n, ...)
+  prober(...)
+  return looper(prober, n + 1, n, ...)
+end
+
+pcall(coroutine.wrap(looper), prober_1, 0)
+
+test:ok(true, 'no stack overflow with recursive pcall')
+
+-- The testcase demonstrate a segmentation fault due to stack
+-- overflow when `pcall()` is used as `__newindex` metamethod.
+-- The function is vararg because stack check in BC_IFUNCV is off
+-- by one on ARM64 and MIPS64 without the patch.
+
+-- Any fast function can be used as metamethod, but `type` is
+-- convenient here because it works fast and can be used with
+-- any data type. Lua function cannot be used since it
+-- will check the stack on each invocation.
+local t = setmetatable({}, { __newindex = pcall, __call = type })
+
+local function prober_2(...) -- luacheck: no unused
+  -- Invokes `pcall(t, t, t)`.
+  t[t] = t
+end
+
+pcall(coroutine.wrap(looper), prober_2, 0)
+
+test:ok(true, 'no stack overflow with metamethod')
+
+test:done(true)
-- 
2.43.0