From 196bb94d66e727e0aec053a0276c3ad701500762 Mon Sep 17 00:00:00 2001 From: Roberto Ierusalimschy Date: Wed, 25 May 2022 17:41:39 -0300 Subject: Bug: 'lua_settop' may use an invalid pointer to stack --- lapi.c | 5 ++--- ldo.c | 12 ++++++------ lfunc.c | 5 +++-- lfunc.h | 2 +- testes/locals.lua | 22 ++++++++++++++++++++++ 5 files changed, 34 insertions(+), 12 deletions(-) diff --git a/lapi.c b/lapi.c index 352a385a..5833c7b0 100644 --- a/lapi.c +++ b/lapi.c @@ -197,7 +197,7 @@ LUA_API void lua_settop (lua_State *L, int idx) { newtop = L->top + diff; if (diff < 0 && L->tbclist >= newtop) { lua_assert(hastocloseCfunc(ci->nresults)); - luaF_close(L, newtop, CLOSEKTOP, 0); + newtop = luaF_close(L, newtop, CLOSEKTOP, 0); } L->top = newtop; /* correct top only after closing any upvalue */ lua_unlock(L); @@ -210,8 +210,7 @@ LUA_API void lua_closeslot (lua_State *L, int idx) { level = index2stack(L, idx); api_check(L, hastocloseCfunc(L->ci->nresults) && L->tbclist == level, "no variable to close at given level"); - luaF_close(L, level, CLOSEKTOP, 0); - level = index2stack(L, idx); /* stack may be moved */ + level = luaF_close(L, level, CLOSEKTOP, 0); setnilvalue(s2v(level)); lua_unlock(L); } diff --git a/ldo.c b/ldo.c index 5aa6d59d..13498905 100644 --- a/ldo.c +++ b/ldo.c @@ -430,14 +430,15 @@ l_sinline void moveresults (lua_State *L, StkId res, int nres, int wanted) { break; default: /* two/more results and/or to-be-closed variables */ if (hastocloseCfunc(wanted)) { /* to-be-closed variables? */ - ptrdiff_t savedres = savestack(L, res); L->ci->callstatus |= CIST_CLSRET; /* in case of yields */ L->ci->u2.nres = nres; - luaF_close(L, res, CLOSEKTOP, 1); + res = luaF_close(L, res, CLOSEKTOP, 1); L->ci->callstatus &= ~CIST_CLSRET; - if (L->hookmask) /* if needed, call hook after '__close's */ + if (L->hookmask) { /* if needed, call hook after '__close's */ + ptrdiff_t savedres = savestack(L, res); rethook(L, L->ci, nres); - res = restorestack(L, savedres); /* close and hook can move stack */ + res = restorestack(L, savedres); /* hook can move stack */ + } wanted = decodeNresults(wanted); if (wanted == LUA_MULTRET) wanted = nres; /* we want all results */ @@ -654,8 +655,7 @@ static int finishpcallk (lua_State *L, CallInfo *ci) { else { /* error */ StkId func = restorestack(L, ci->u2.funcidx); L->allowhook = getoah(ci->callstatus); /* restore 'allowhook' */ - luaF_close(L, func, status, 1); /* can yield or raise an error */ - func = restorestack(L, ci->u2.funcidx); /* stack may be moved */ + func = luaF_close(L, func, status, 1); /* can yield or raise an error */ luaD_seterrorobj(L, status, func); luaD_shrinkstack(L); /* restore stack size in case of overflow */ setcistrecst(ci, LUA_OK); /* clear original status */ diff --git a/lfunc.c b/lfunc.c index f5889a21..3ed65de2 100644 --- a/lfunc.c +++ b/lfunc.c @@ -223,9 +223,9 @@ static void poptbclist (lua_State *L) { /* ** Close all upvalues and to-be-closed variables up to the given stack -** level. +** level. Return restored 'level'. */ -void luaF_close (lua_State *L, StkId level, int status, int yy) { +StkId luaF_close (lua_State *L, StkId level, int status, int yy) { ptrdiff_t levelrel = savestack(L, level); luaF_closeupval(L, level); /* first, close the upvalues */ while (L->tbclist >= level) { /* traverse tbc's down to that level */ @@ -234,6 +234,7 @@ void luaF_close (lua_State *L, StkId level, int status, int yy) { prepcallclosemth(L, tbc, status, yy); /* close variable */ level = restorestack(L, levelrel); } + return level; } diff --git a/lfunc.h b/lfunc.h index dc1cebcc..3d296971 100644 --- a/lfunc.h +++ b/lfunc.h @@ -54,7 +54,7 @@ LUAI_FUNC void luaF_initupvals (lua_State *L, LClosure *cl); LUAI_FUNC UpVal *luaF_findupval (lua_State *L, StkId level); LUAI_FUNC void luaF_newtbcupval (lua_State *L, StkId level); LUAI_FUNC void luaF_closeupval (lua_State *L, StkId level); -LUAI_FUNC void luaF_close (lua_State *L, StkId level, int status, int yy); +LUAI_FUNC StkId luaF_close (lua_State *L, StkId level, int status, int yy); LUAI_FUNC void luaF_unlinkupval (UpVal *uv); LUAI_FUNC void luaF_freeproto (lua_State *L, Proto *f); LUAI_FUNC const char *luaF_getlocalname (const Proto *func, int local_number, diff --git a/testes/locals.lua b/testes/locals.lua index 62a88df5..ddb75054 100644 --- a/testes/locals.lua +++ b/testes/locals.lua @@ -592,6 +592,28 @@ end if rawget(_G, "T") then + do + -- bug in 5.4.3 + -- 'lua_settop' may use a pointer to stack invalidated by 'luaF_close' + + -- reduce stack size + collectgarbage(); collectgarbage(); collectgarbage() + + -- force a stack reallocation + local function loop (n) + if n < 400 then loop(n + 1) end + end + + -- close metamethod will reallocate the stack + local o = setmetatable({}, {__close = function () loop(0) end}) + + local script = [[toclose 2; settop 1; return 1]] + + assert(T.testC(script, o) == script) + + end + + -- memory error inside closing function local function foo () local y = func2close(function () T.alloccount() end) -- cgit v1.2.1