Skip to content

Commit 324fb33

Browse files
Mike PallBuristan
authored andcommitted
Handle OOM error on stack resize in coroutine.resume and lua_checkstack.
Thanks to Peter Cawley. (cherry picked from commit a5d2f70) If the OOM or stack overflow error is obtained during `lj_state_growstack()` in the `coroutine.resume()` it may raise an error leading to the incorrect error message and unexpected behaviour for this corner case. Also, `lua_checkstack()` may raise an OOM error leading to panic on an unprotected lua_State, for example. This patch handles these cases by protecting `lj_state_growstack()`. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#12134 Reviewed-by: Sergey Bronnikov <sergeyb@tarantool.org> Signed-off-by: Sergey Kaplun <skaplun@tarantool.org> (cherry picked from commit 357a260)
1 parent c44d1de commit 324fb33

File tree

6 files changed

+134
-2
lines changed

6 files changed

+134
-2
lines changed

src/lib_base.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,10 @@ static int ffh_resume(lua_State *L, lua_State *co, int wrap)
606606
setstrV(L, L->base-LJ_FR2, lj_err_str(L, em));
607607
return FFH_RES(2);
608608
}
609-
lj_state_growstack(co, (MSize)(L->top - L->base));
609+
if (lj_state_cpgrowstack(co, (MSize)(L->top - L->base)) != LUA_OK) {
610+
cTValue *msg = --co->top;
611+
lj_err_callermsg(L, strVdata(msg));
612+
}
610613
return FFH_RETRY;
611614
}
612615

src/lj_api.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,12 @@ LUA_API int lua_checkstack(lua_State *L, int size)
116116
if (size > LUAI_MAXCSTACK || (L->top - L->base + size) > LUAI_MAXCSTACK) {
117117
return 0; /* Stack overflow. */
118118
} else if (size > 0) {
119-
lj_state_checkstack(L, (MSize)size);
119+
int avail = (int)(mref(L->maxstack, TValue) - L->top);
120+
if (size > avail &&
121+
lj_state_cpgrowstack(L, (MSize)(size - avail)) != LUA_OK) {
122+
L->top--;
123+
return 0; /* Out of memory. */
124+
}
120125
}
121126
return 1;
122127
}

src/lj_state.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,18 @@ void LJ_FASTCALL lj_state_growstack1(lua_State *L)
170170
lj_state_growstack(L, 1);
171171
}
172172

173+
static TValue *cpgrowstack(lua_State *co, lua_CFunction dummy, void *ud)
174+
{
175+
UNUSED(dummy);
176+
lj_state_growstack(co, *(MSize *)ud);
177+
return NULL;
178+
}
179+
180+
int LJ_FASTCALL lj_state_cpgrowstack(lua_State *L, MSize need)
181+
{
182+
return lj_vm_cpcall(L, NULL, &need, cpgrowstack);
183+
}
184+
173185
/* Allocate basic stack for new state. */
174186
static void stack_init(lua_State *L1, lua_State *L)
175187
{

src/lj_state.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ LJ_FUNC void lj_state_relimitstack(lua_State *L);
1818
LJ_FUNC void lj_state_shrinkstack(lua_State *L, MSize used);
1919
LJ_FUNCA void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need);
2020
LJ_FUNC void LJ_FASTCALL lj_state_growstack1(lua_State *L);
21+
LJ_FUNC int LJ_FASTCALL lj_state_cpgrowstack(lua_State *L, MSize need);
2122

2223
static LJ_AINLINE void lj_state_checkstack(lua_State *L, MSize need)
2324
{
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
#include "lua.h"
2+
#include "luaconf.h"
3+
4+
#include "test.h"
5+
#include "utils.h"
6+
7+
#include <stdlib.h>
8+
9+
/* XXX: Still need normal assert for sanity checks. */
10+
#undef NDEBUG
11+
#include <assert.h>
12+
13+
static lua_Alloc old_allocf = NULL;
14+
static void *old_alloc_state = NULL;
15+
16+
/* Always OOM on reallocation (not on allocation). */
17+
static void *allocf_null_realloc(void *ud, void *ptr, size_t osize,
18+
size_t nsize)
19+
{
20+
assert(old_allocf != NULL);
21+
if (ptr != NULL && osize < nsize)
22+
return NULL;
23+
else
24+
return old_allocf(ud, ptr, osize, nsize);
25+
}
26+
27+
static void enable_allocinject(lua_State *L)
28+
{
29+
assert(old_allocf == NULL);
30+
old_allocf = lua_getallocf(L, &old_alloc_state);
31+
lua_setallocf(L, allocf_null_realloc, old_alloc_state);
32+
}
33+
34+
/* Restore the default allocator function. */
35+
static void disable_allocinject(lua_State *L)
36+
{
37+
assert(old_allocf != NULL);
38+
lua_setallocf(L, old_allocf, old_alloc_state);
39+
old_allocf = NULL;
40+
old_alloc_state = NULL;
41+
}
42+
43+
static int checkstack_res = -1;
44+
45+
static int test_checkstack(lua_State *L)
46+
{
47+
/*
48+
* There is no stack overflow error, but the OOM error
49+
* due to stack reallocation, before the patch.
50+
*/
51+
checkstack_res = lua_checkstack(L, LUAI_MAXCSTACK / 2);
52+
return 0;
53+
}
54+
55+
static int oom_on_lua_checkstack(void *test_state)
56+
{
57+
lua_State *L = test_state;
58+
/* Use fresh-new coroutine for stack manipulations. */
59+
lua_State *L1 = lua_newthread(L);
60+
61+
enable_allocinject(L);
62+
/*
63+
* `L1` should have enough space to `cpcall()` without
64+
* stack reallocation.
65+
*/
66+
int status = lua_cpcall(L1, test_checkstack, NULL);
67+
disable_allocinject(L);
68+
69+
assert_true(status == LUA_OK);
70+
assert_true(checkstack_res == 0);
71+
72+
return TEST_EXIT_SUCCESS;
73+
}
74+
75+
int main(void)
76+
{
77+
lua_State *L = utils_lua_init();
78+
const struct test_unit tgroup[] = {
79+
test_unit_def(oom_on_lua_checkstack),
80+
};
81+
const int test_result = test_run_group(tgroup, L);
82+
utils_lua_close(L);
83+
return test_result;
84+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
local tap = require('tap')
2+
3+
-- Test file to demonstrate LuaJIT misbehaviour in case of error
4+
-- on growing stack for the coroutine during `coroutine.resume()`.
5+
-- See also: https://github.com/LuaJIT/LuaJIT/issues/1066.
6+
7+
local test = tap.test('lj-1066-err-coroutine-resume')
8+
9+
test:plan(2)
10+
11+
local function resume_wrap()
12+
local function recurser()
13+
recurser(coroutine.yield())
14+
end
15+
local co = coroutine.create(recurser)
16+
-- Cause the stack overflow and throws an error with incorrect
17+
-- error message before the patch. Use some arguments to obtain
18+
-- the stack overflow faster.
19+
while coroutine.resume(co, 1, 2, 3, 4, 5, 6, 7, 8, 9) do end
20+
end
21+
22+
local status, errmsg = pcall(resume_wrap)
23+
24+
test:is(status, false, 'status is correct')
25+
test:like(errmsg, 'stack overflow', 'error message is correct')
26+
27+
test:done(true)

0 commit comments

Comments
 (0)