From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 95FE0133BC30; Fri, 25 Apr 2025 16:42:12 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 95FE0133BC30 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1745588532; bh=q97QJv/rKm7puvtytE6bDqJrGPPMTU5UAvH91BYZfIY=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=qq4ZEd1MghoqpndlYXbZ7wpeObCaspnedlawmU13sNz4XJAj/tuMNtvIM+aCAIbWS 6Z7C35/8jRJi5/imAMySCx6Kwmh08lWeWQqVAYY1bxsKBO3j1ePhbz70X77RmxMZCg 0+8RRic2LoQsewc1Wu4LX5W0B3coijfUbNmvBNCU= Received: from send58.i.mail.ru (send58.i.mail.ru [89.221.237.153]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 5EDB354EB90 for ; Fri, 25 Apr 2025 16:42:11 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5EDB354EB90 Received: by exim-smtp-546bdf5556-zhz4c with esmtpa (envelope-from ) id 1u8JJi-00000000NCj-14Wl; Fri, 25 Apr 2025 16:42:10 +0300 To: Sergey Bronnikov Date: Fri, 25 Apr 2025 16:42:15 +0300 Message-ID: <20250425134215.8184-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.49.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD96B6D2FE69D52BFEDE3B3B84A171E0446C9308A33D982AAB8182A05F5380850400092778ACA1D17003DE06ABAFEAF6705D2BDF1B9D08E5FF10564BC1FD2AC650740D08F30CB48EA5E X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7011EB7026DD4A9BAEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB553375668B77978056C3336183827536A76D0FB999D9B305280EB4A6426C423CD3C76A96389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C0ECC8AC47CD0EDEFF8941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B6F459A8243F1D1D44CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB86D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166176DF2183F8FC7C04E672349037D5FA5725E5C173C3A84C3089A2CD95EED70F435872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A55DFD52DDD62F32BD5002B1117B3ED6961F57769DC4500C70D57BAD45EC4C5DE1823CB91A9FED034534781492E4B8EEADBD2EDBEAC172B1CCC79554A2A72441328621D336A7BC284946AD531847A6065A535571D14F44ED41 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742D3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF8FF3475D02998CB05DC553A07AD40EA740E8841EA47BDE018843A0950685C804696309706696DB62401B3302A3442B480467D857CC2A420A34A31C2E6C466BBDF09171D5DC887FF45F4332CA8FE04980913E6812662D5F2A5EAB5682573093F7837F15F2B5E4A70B33F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVS+uSU+BUhgvWtpdrczkvi8= X-DA7885C5: 415858EDFB44FD65F255D290C0D534F98834FF5DB316DD13A7F9E8CA52E3A2B1959C9FF18B8EDE145B1A4C17EAA7BC4BEF2421ABFA55128DAF83EF9164C44C7E X-Mailru-Sender: 689FA8AB762F7393FE9E42A757851DB62DAC30666F7E3CAB27B9178048307AB94E6482BCF42DE17EE49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] Rework stack overflow handling. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" From: Mike Pall Reported by pwnhacker0x18. Fixed by Peter Cawley. (cherry picked from commit defe61a56751a0db5f00ff3ab7b8f45436ba74c8) In case of the Lua stack overflow error, LuaJIT restores the `L->top` value and pushes the error message above. It is possible that the restored value is greater than `L->maxstack`, so pushing the error message causes dirty write out-of-bounds. This patch prevents it by overwriting stack overflow handling machinery. Now, in the aforementioned case, the last frame is replaced with a dummy frame to avoid dirty writes. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#11278 --- Related issues: * https://github.com/LuaJIT/LuaJIT/issues/1152 * https://github.com/tarantool/tarantool/issues/11278 Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1152-stack-buffer-overflow-on-error The CI is red due to CMake minimum required version in c-dt module in Tarantool. The cherry-pick also included a part fixed in the merge commit: https://github.com/LuaJIT/LuaJIT/commit/0d313b243 src/lj_debug.c | 1 + src/lj_err.c | 23 +++++- src/lj_err.h | 1 + src/lj_state.c | 58 +++++++++----- test/LuaJIT-tests/lang/stackov.lua | 8 +- ...52-stack-buffer-overflow-on-error.test.lua | 79 +++++++++++++++++++ test/tarantool-tests/utils/CMakeLists.txt | 1 + 7 files changed, 148 insertions(+), 23 deletions(-) create mode 100644 test/tarantool-tests/lj-1152-stack-buffer-overflow-on-error.test.lua diff --git a/src/lj_debug.c b/src/lj_debug.c index 107f464c..76e48aca 100644 --- a/src/lj_debug.c +++ b/src/lj_debug.c @@ -64,6 +64,7 @@ static BCPos debug_framepc(lua_State *L, GCfunc *fn, cTValue *nextframe) if (cf == NULL || (char *)cframe_pc(cf) == (char *)cframe_L(cf)) return NO_BCPOS; ins = cframe_pc(cf); /* Only happens during error/hook handling. */ + if (!ins) return NO_BCPOS; } else { if (frame_islua(nextframe)) { ins = frame_pc(nextframe); diff --git a/src/lj_err.c b/src/lj_err.c index 1a9a2f2b..80dca847 100644 --- a/src/lj_err.c +++ b/src/lj_err.c @@ -784,7 +784,14 @@ LJ_NOINLINE void lj_err_mem(lua_State *L) TValue *base = tvref(G(L)->jit_base); if (base) L->base = base; } - if (curr_funcisL(L)) L->top = curr_topL(L); + if (curr_funcisL(L)) { + L->top = curr_topL(L); + if (LJ_UNLIKELY(L->top > tvref(L->maxstack))) { + /* The current Lua frame violates the stack. Replace it with a dummy. */ + L->top = L->base; + setframe_gc(L->base - 1 - LJ_FR2, obj2gco(L), LJ_TTHREAD); + } + } setstrV(L, L->top++, lj_err_str(L, LJ_ERR_ERRMEM)); lj_err_throw(L, LUA_ERRMEM); } @@ -845,9 +852,11 @@ LJ_NOINLINE void LJ_FASTCALL lj_err_run(lua_State *L) { ptrdiff_t ef = (LJ_HASJIT && tvref(G(L)->jit_base)) ? 0 : finderrfunc(L); if (ef) { - TValue *errfunc = restorestack(L, ef); - TValue *top = L->top; + TValue *errfunc, *top; + lj_state_checkstack(L, LUA_MINSTACK * 2); /* Might raise new error. */ lj_trace_abort(G(L)); + errfunc = restorestack(L, ef); + top = L->top; if (!tvisfunc(errfunc) || L->status == LUA_ERRERR) { setstrV(L, top-1, lj_err_str(L, LJ_ERR_ERRERR)); lj_err_throw(L, LUA_ERRERR); @@ -862,7 +871,15 @@ LJ_NOINLINE void LJ_FASTCALL lj_err_run(lua_State *L) lj_err_throw(L, LUA_ERRRUN); } +/* Stack overflow error. */ +void LJ_FASTCALL lj_err_stkov(lua_State *L) +{ + lj_debug_addloc(L, err2msg(LJ_ERR_STKOV), L->base-1, NULL); + lj_err_run(L); +} + #if LJ_HASJIT +/* Rethrow error after doing a trace exit. */ LJ_NOINLINE void LJ_FASTCALL lj_err_trace(lua_State *L, int errcode) { if (errcode == LUA_ERRRUN) diff --git a/src/lj_err.h b/src/lj_err.h index b0c72c24..56bd0375 100644 --- a/src/lj_err.h +++ b/src/lj_err.h @@ -23,6 +23,7 @@ LJ_DATA const char *lj_err_allmsg; LJ_FUNC GCstr *lj_err_str(lua_State *L, ErrMsg em); LJ_FUNCA_NORET void LJ_FASTCALL lj_err_throw(lua_State *L, int errcode); LJ_FUNC_NORET void lj_err_mem(lua_State *L); +LJ_FUNC_NORET void LJ_FASTCALL lj_err_stkov(lua_State *L); LJ_FUNC_NORET void LJ_FASTCALL lj_err_run(lua_State *L); #if LJ_HASJIT LJ_FUNCA_NORET void LJ_FASTCALL lj_err_trace(lua_State *L, int errcode); diff --git a/src/lj_state.c b/src/lj_state.c index 5a920102..053e5ec9 100644 --- a/src/lj_state.c +++ b/src/lj_state.c @@ -120,27 +120,49 @@ void lj_state_shrinkstack(lua_State *L, MSize used) /* Try to grow stack. */ void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need) { - MSize n; - if (L->stacksize >= LJ_STACK_MAXEX) { - /* 4. Throw 'error in error handling' when we are _over_ the limit. */ - if (L->stacksize > LJ_STACK_MAXEX) + MSize n = L->stacksize + need; + if (LJ_LIKELY(n < LJ_STACK_MAX)) { /* The stack can grow as requested. */ + if (n < 2 * L->stacksize) { /* Try to double the size. */ + n = 2 * L->stacksize; + if (n > LJ_STACK_MAX) + n = LJ_STACK_MAX; + } + resizestack(L, n); + } else { /* Request would overflow. Raise a stack overflow error. */ + if (LJ_HASJIT) { + TValue *base = tvref(G(L)->jit_base); + if (base) L->base = base; + } + if (curr_funcisL(L)) { + L->top = curr_topL(L); + if (L->top > tvref(L->maxstack)) { + /* The current Lua frame violates the stack, so replace it with a + ** dummy. This can happen when BC_IFUNCF is trying to grow the stack. + */ + L->top = L->base; + setframe_gc(L->base - 1 - LJ_FR2, obj2gco(L), LJ_TTHREAD); + } + } + if (L->stacksize <= LJ_STACK_MAXEX) { + /* 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(). + */ + resizestack(L, LJ_STACK_MAX + 1 + 2 * LUA_MINSTACK); + lj_err_stkov(L); /* May invoke an error handler. */ + } else { + /* If we're here, then the stack overflow error handler is requesting + ** to grow the stack even further. We have no choice but to abort the + ** error handler. + */ + GCstr *em = lj_err_str(L, LJ_ERR_STKOV); /* Might OOM. */ + setstrV(L, L->top++, em); /* There is always space to push an error. */ lj_err_throw(L, LUA_ERRERR); /* Does not invoke an error handler. */ - /* 1. We are _at_ the limit after the last growth. */ - if (L->status < LUA_ERRRUN) { /* 2. Throw 'stack overflow'. */ - L->status = LUA_ERRRUN; /* Prevent ending here again for pushed msg. */ - lj_err_msg(L, LJ_ERR_STKOV); /* May invoke an error handler. */ } - /* 3. Add space (over the limit) for pushed message and error handler. */ - } - n = L->stacksize + need; - if (n > LJ_STACK_MAX) { - n += 2*LUA_MINSTACK; - } else if (n < 2*L->stacksize) { - n = 2*L->stacksize; - if (n >= LJ_STACK_MAX) - n = LJ_STACK_MAX; } - resizestack(L, n); } void LJ_FASTCALL lj_state_growstack1(lua_State *L) diff --git a/test/LuaJIT-tests/lang/stackov.lua b/test/LuaJIT-tests/lang/stackov.lua index 8afa86b4..b052ad80 100644 --- a/test/LuaJIT-tests/lang/stackov.lua +++ b/test/LuaJIT-tests/lang/stackov.lua @@ -31,13 +31,17 @@ end do --- Base test. local err, s = xpcall(f, debug.traceback) assert(err == false) - test_error_msg(f, s) + -- There is no place on the stack to invoke the handler. + -- Just test the error reason. + assert(string.match(s, "stack overflow")) end do --- Stack overflow with non-empty arg list. local err, s = xpcall(g, debug.traceback, 1) assert(err == false) - test_error_msg(g, s) + -- There is no place on the stack to invoke the handler. + -- Just test the error reason. + assert(string.match(s, "stack overflow")) end do --- Vararg tail call with non-empty arg list. +slow diff --git a/test/tarantool-tests/lj-1152-stack-buffer-overflow-on-error.test.lua b/test/tarantool-tests/lj-1152-stack-buffer-overflow-on-error.test.lua new file mode 100644 index 00000000..08e344de --- /dev/null +++ b/test/tarantool-tests/lj-1152-stack-buffer-overflow-on-error.test.lua @@ -0,0 +1,79 @@ +local tap = require('tap') +local allocinject = require('allocinject') + +local test = tap.test('lj-1152-stack-buffer-overflow-on-error') +test:plan(6) + +local LJ_MAX_LOCVAR = 200 + +-- Generate the following Lua chunk: +-- local function recursive_f() +-- local _ +-- ... +-- recursive_f() +-- end +-- return recursive_f +local function generate_recursive_f(n_locals) + local chunk = 'local function recursive_f()\n' + for _ = 1, n_locals do + chunk = chunk .. 'local _\n' + end + chunk = chunk .. [[ + recursive_f() + end + return recursive_f + ]] + local f = assert(loadstring(chunk)) + return f() +end + +-- Use `coroutine.wrap()` for functions to use newly created stack +-- with fixed number of stack slots. + +-- Check that we still got the correct error message in case of +-- the unsafe error handler function. +coroutine.wrap(function() + -- XXX: Use different recursive functions as callee and handler + -- to be sure to get the invalid stack value instead of `nil`. + local function recursive() recursive() end + local function bad_errfunc() bad_errfunc() end + local r, e = xpcall(recursive, bad_errfunc) + -- XXX: Don't create a constant string that is anchored to the + -- prototype. It is necessary to make the error message freed by + -- the GC and OOM raising in the last test. + local EXPECTED_MSG = 'stack ' .. 'overflow' + test:ok(not r, 'correct status') + test:like(e, EXPECTED_MSG, 'correct error message') +end)() + +coroutine.wrap(function() + -- Collect all strings including the possibly-existed string + -- with the 'stack overflow' error message. + collectgarbage() + local function recursive() recursive() end + -- Avoid trace recording. A trace can't be allocated anyway. + jit.off(recursive) + + -- Check the case when the error + allocinject.enable_null_alloc() + local r, e = pcall(recursive) + allocinject.disable() + + test:ok(not r, 'correct status') + test:like(e, 'not enough memory', 'correct error message') +end)() + +-- Check overflow of the buffer related to the Lua stack. +-- This test fails under ASAN without the patch. +local recursive_f = generate_recursive_f(LJ_MAX_LOCVAR) +coroutine.wrap(function() + local r, e = pcall(recursive_f) + test:ok(not r, 'correct status') + -- XXX: Don't create a constant string that is anchored to the + -- prototype. It is necessary to make the error message freed by + -- the GC and OOM raising in the last test. + local EXPECTED_MSG = 'stack ' .. 'overflow' + test:like(e, EXPECTED_MSG, 'correct error message') +end)() + +test:done(true) diff --git a/test/tarantool-tests/utils/CMakeLists.txt b/test/tarantool-tests/utils/CMakeLists.txt index 15871934..e74e184b 100644 --- a/test/tarantool-tests/utils/CMakeLists.txt +++ b/test/tarantool-tests/utils/CMakeLists.txt @@ -1,4 +1,5 @@ list(APPEND tests + lj-1152-stack-buffer-overflow-on-error.test.lua lj-1166-error-stitch-oom-ir-buff.test.lua lj-1166-error-stitch-oom-snap-buff.test.lua lj-1247-fin-tab-rehashing-on-trace.test.lua -- 2.49.0