[Tarantool-patches] [PATCH luajit 3/3][v3] Add stack check to pcall/xpcall.

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

From: Mike Pall <mike>

Analyzed by Peter Cawley.

(cherry picked from commit a4c1640432a9d8a60624cdc8065b15078c228e36)

The patch adds the stack check to fast functions `pcall()` and
`xpcall()`.

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

Part of tarantool/tarantool#12134
---
 src/vm_arm.dasc                               |  7 ++++
 src/vm_arm64.dasc                             |  8 +++++
 src/vm_mips.dasc                              | 10 +++++-
 src/vm_mips64.dasc                            | 14 ++++++--
 src/vm_ppc.dasc                               |  9 +++++
 src/vm_x64.dasc                               |  6 ++++
 src/vm_x86.dasc                               |  6 ++++
 ...048-fix-stack-checks-vararg-calls.test.lua | 35 ++++++++++++++++++-
 8 files changed, 90 insertions(+), 5 deletions(-)

diff --git a/src/vm_arm.dasc b/src/vm_arm.dasc
index 7095e660..efe9dcb2 100644
--- a/src/vm_arm.dasc
+++ b/src/vm_arm.dasc
@@ -1201,8 +1201,11 @@ static void build_subroutines(BuildCtx *ctx)
   |//-- Base library: catch errors ----------------------------------------
   |
   |.ffunc pcall
+  |   ldr RB, L->maxstack
+  |   add INS, BASE, NARGS8:RC
   |  ldrb RA, [DISPATCH, #DISPATCH_GL(hookmask)]
   |   cmp NARGS8:RC, #8
+  |   cmphs RB, INS
   |   blo ->fff_fallback
   |  tst RA, #HOOK_ACTIVE		// Remember active hook before pcall.
   |   mov RB, BASE
@@ -1213,7 +1216,11 @@ static void build_subroutines(BuildCtx *ctx)
   |  b ->vm_call_dispatch
   |
   |.ffunc_2 xpcall
+  |   ldr RB, L->maxstack
+  |   add INS, BASE, NARGS8:RC
   |  ldrb RA, [DISPATCH, #DISPATCH_GL(hookmask)]
+  |   cmp RB, INS
+  |   blo ->fff_fallback
   |  checkfunc CARG4, ->fff_fallback	// Traceback must be a function.
   |   mov RB, BASE
   |  strd CARG12, [BASE, #8]		// Swap function and traceback.
diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
index 5ef37243..074c1f31 100644
--- a/src/vm_arm64.dasc
+++ b/src/vm_arm64.dasc
@@ -1166,6 +1166,10 @@ static void build_subroutines(BuildCtx *ctx)
   |//-- Base library: catch errors ----------------------------------------
   |
   |.ffunc pcall
+  |  ldr TMP1, L->maxstack
+  |  add TMP2, BASE, NARGS8:RC
+  |  cmp TMP1, TMP2
+  |  blo ->fff_fallback
   |   cmp NARGS8:RC, #8
   |  ldrb TMP0w, GL->hookmask
   |   blo ->fff_fallback
@@ -1185,6 +1189,10 @@ static void build_subroutines(BuildCtx *ctx)
   |  b ->vm_call_dispatch
   |
   |.ffunc xpcall
+  |  ldr TMP1, L->maxstack
+  |  add TMP2, BASE, NARGS8:RC
+  |  cmp TMP1, TMP2
+  |  blo ->fff_fallback
   |     ldp CARG1, CARG2, [BASE]
   |  ldrb TMP0w, GL->hookmask
   |   subs NARGS8:TMP1, NARGS8:RC, #16
diff --git a/src/vm_mips.dasc b/src/vm_mips.dasc
index 32caabf7..69d09d52 100644
--- a/src/vm_mips.dasc
+++ b/src/vm_mips.dasc
@@ -1382,9 +1382,13 @@ static void build_subroutines(BuildCtx *ctx)
   |//-- Base library: catch errors ----------------------------------------
   |
   |.ffunc pcall
+  |   lw TMP1, L->maxstack
+  |   addu TMP2, BASE, NARGS8:RC
   |  lbu TMP3, DISPATCH_GL(hookmask)(DISPATCH)
   |  beqz NARGS8:RC, ->fff_fallback
-  |   move TMP2, BASE
+  |.  sltu AT, TMP1, TMP2
+  |   bnez AT, ->fff_fallback
+  |.   move TMP2, BASE
   |   addiu BASE, BASE, 8
   |  // Remember active hook before pcall.
   |  srl TMP3, TMP3, HOOK_ACTIVE_SHIFT
@@ -1394,8 +1398,12 @@ static void build_subroutines(BuildCtx *ctx)
   |.  addiu NARGS8:RC, NARGS8:RC, -8
   |
   |.ffunc xpcall
+  |   lw TMP1, L->maxstack
+  |   addu TMP2, BASE, NARGS8:RC
   |    sltiu AT, NARGS8:RC, 16
   |  lw CARG4, 8+HI(BASE)
+  |   sltu TMP1, TMP1, TMP2
+  |    or AT, AT, TMP1
   |    bnez AT, ->fff_fallback
   |.  lw CARG3, 8+LO(BASE)
   |   lw CARG1, LO(BASE)
diff --git a/src/vm_mips64.dasc b/src/vm_mips64.dasc
index 6c2975b4..4e60ee07 100644
--- a/src/vm_mips64.dasc
+++ b/src/vm_mips64.dasc
@@ -1418,8 +1418,12 @@ static void build_subroutines(BuildCtx *ctx)
   |//-- Base library: catch errors ----------------------------------------
   |
   |.ffunc pcall
+  |  ld TMP1, L->maxstack
+  |  daddu TMP2, BASE, NARGS8:RC
+  |  sltu AT, TMP1, TMP2
+  |  bnez AT, ->fff_fallback
+  |.  lbu TMP3, DISPATCH_GL(hookmask)(DISPATCH)
   |  daddiu NARGS8:RC, NARGS8:RC, -8
-  |  lbu TMP3, DISPATCH_GL(hookmask)(DISPATCH)
   |  bltz NARGS8:RC, ->fff_fallback
   |.   move TMP2, BASE
   |   daddiu BASE, BASE, 16
@@ -1440,8 +1444,12 @@ static void build_subroutines(BuildCtx *ctx)
   |.  nop
   |
   |.ffunc xpcall
-  |  daddiu NARGS8:TMP0, NARGS8:RC, -16
-  |  ld CARG1, 0(BASE)
+  |  ld TMP1, L->maxstack
+  |  daddu TMP2, BASE, NARGS8:RC
+  |  sltu AT, TMP1, TMP2
+  |  bnez AT, ->fff_fallback
+  |.  ld CARG1, 0(BASE)
+  |  daddiu NARGS8:RC, NARGS8:RC, -16
   |   ld CARG2, 8(BASE)
   |    bltz NARGS8:TMP0, ->fff_fallback
   |.    lbu TMP1, DISPATCH_GL(hookmask)(DISPATCH)
diff --git a/src/vm_ppc.dasc b/src/vm_ppc.dasc
index 980ad897..f2ea933b 100644
--- a/src/vm_ppc.dasc
+++ b/src/vm_ppc.dasc
@@ -1755,8 +1755,12 @@ static void build_subroutines(BuildCtx *ctx)
   |//-- Base library: catch errors ----------------------------------------
   |
   |.ffunc pcall
+  |    lwz TMP1, L->maxstack
+  |    add TMP2, BASE, NARGS8:RC
   |  cmplwi NARGS8:RC, 8
   |   lbz TMP3, DISPATCH_GL(hookmask)(DISPATCH)
+  |    cmplw cr1, TMP1, TMP2
+  |  cror 4*cr0+lt, 4*cr0+lt, 4*cr1+lt
   |  blt ->fff_fallback
   |   mr TMP2, BASE
   |   la BASE, 8(BASE)
@@ -1767,14 +1771,19 @@ static void build_subroutines(BuildCtx *ctx)
   |  b ->vm_call_dispatch
   |
   |.ffunc xpcall
+  |     lwz TMP1, L->maxstack
+  |     add TMP2, BASE, NARGS8:RC
   |  cmplwi NARGS8:RC, 16
   |   lwz CARG3, 8(BASE)
+  |     cmplw cr1, TMP1, TMP2
   |.if FPU
   |    lfd FARG2, 8(BASE)
+  |  cror 4*cr0+lt, 4*cr0+lt, 4*cr1+lt
   |    lfd FARG1, 0(BASE)
   |.else
   |    lwz CARG1, 0(BASE)
   |    lwz CARG2, 4(BASE)
+  |  cror 4*cr0+lt, 4*cr0+lt, 4*cr1+lt
   |    lwz CARG4, 12(BASE)
   |.endif
   |  blt ->fff_fallback
diff --git a/src/vm_x64.dasc b/src/vm_x64.dasc
index 8b6781a6..c57b76b7 100644
--- a/src/vm_x64.dasc
+++ b/src/vm_x64.dasc
@@ -1545,6 +1545,9 @@ static void build_subroutines(BuildCtx *ctx)
   |//-- Base library: catch errors ----------------------------------------
   |
   |.ffunc_1 pcall
+  |  mov L:RB, SAVE_L
+  |  lea RA, [BASE+NARGS:RD*8]
+  |  cmp RA, L:RB->maxstack; ja ->fff_fallback
   |  lea RA, [BASE+16]
   |  sub NARGS:RDd, 1
   |  mov PCd, 16+FRAME_PCALL
@@ -1563,6 +1566,9 @@ static void build_subroutines(BuildCtx *ctx)
   |  jmp ->vm_call_dispatch
   |
   |.ffunc_2 xpcall
+  |  mov L:RB, SAVE_L
+  |  lea RA, [BASE+NARGS:RD*8]
+  |  cmp RA, L:RB->maxstack; ja ->fff_fallback
   |  mov LFUNC:RA, [BASE+8]
   |  checktp_nc LFUNC:RA, LJ_TFUNC, ->fff_fallback
   |  mov LFUNC:RB, [BASE]		// Swap function and traceback.
diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc
index 7c11c78e..36804d11 100644
--- a/src/vm_x86.dasc
+++ b/src/vm_x86.dasc
@@ -1914,6 +1914,9 @@ static void build_subroutines(BuildCtx *ctx)
   |//-- Base library: catch errors ----------------------------------------
   |
   |.ffunc_1 pcall
+  |  mov L:RB, SAVE_L
+  |  lea RA, [BASE+NARGS:RD*8]
+  |  cmp RA, L:RB->maxstack; ja ->fff_fallback
   |  lea RA, [BASE+8]
   |  sub NARGS:RD, 1
   |  mov PC, 8+FRAME_PCALL
@@ -1925,6 +1928,9 @@ static void build_subroutines(BuildCtx *ctx)
   |  jmp ->vm_call_dispatch
   |
   |.ffunc_2 xpcall
+  |  mov L:RB, SAVE_L
+  |  lea RA, [BASE+NARGS:RD*8]
+  |  cmp RA, L:RB->maxstack; ja ->fff_fallback
   |  cmp dword [BASE+12], LJ_TFUNC;  jne ->fff_fallback
   |  mov RB, [BASE+4]			// Swap function and traceback.
   |  mov [BASE+12], RB
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
index 3a8ad63d..ad8b151b 100644
--- 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
@@ -5,7 +5,7 @@ local tap = require('tap')
 -- See also https://github.com/LuaJIT/LuaJIT/issues/1048.
 local test = tap.test('lj-1048-fix-stack-checks-vararg-calls')
 
-test:plan(2)
+test:plan(5)
 
 -- The test case demonstrates a segmentation fault due to stack
 -- overflow by recursive calling `pcall()`. The functions are
@@ -50,4 +50,37 @@ pcall(coroutine.wrap(looper), prober_2, 0)
 
 test:ok(true, 'no stack overflow with metamethod')
 
+-- The testcases demonstrates a stack overflow in
+-- `pcall()`/xpcall()` triggered using metamethod `__call`.
+
+t = coroutine.wrap(setmetatable)({}, { __call = pcall })
+
+test:ok(true, 'no stack overflow with metamethod __call with pcall()')
+
+t = coroutine.wrap(setmetatable)({}, { __call = xpcall })
+
+test:ok(true, 'no stack overflow with metamethod __call with xpcall()')
+
+-- The testcase demonstrates a stack overflow in
+-- `pcall()`/`xpcall()` similar to the first testcase, but it is
+-- triggered using `unpack()`.
+
+t = {}
+local function f()
+  return pcall(unpack(t))
+end
+
+-- The problem is only reproduced on LuaJIT GC64 and is best
+-- reproduced under Valgrind than AddressSanitizer. The chosen
+-- value was found experimentally and always results in an attempt
+-- to write beyond the allocated memory.
+local N_ITERATIONS = 200
+
+for i = 1, N_ITERATIONS do
+  t[i], t[i + 1], t[i + 2] = pcall, type, {}
+  coroutine.wrap(f)()
+end
+
+test:ok(true, 'no stack overflow with unpacked pcalls')
+
 test:done(true)
-- 
2.43.0