Fix lua stack overflow on recursive call to gsub()
The `zfs program` subcommand invokes a LUA interpreter to run ZFS "channel programs". This interpreter runs in a constrained environment, with defined memory limits. The LUA stack (used for LUA functions that call each other) is allocated in the kernel's heap, and is limited by the `-m MEMORY-LIMIT` flag and the `zfs_lua_max_memlimit` module parameter. The C stack is used by certain LUA features that are implemented in C. The C stack is limited by `LUAI_MAXCCALLS=20`, which limits call depth. Some LUA C calls use more stack space than others, and `gsub()` uses an unusually large amount. With a programming trick, it can be invoked recursively using the C stack (rather than the LUA stack). This overflows the 16KB Linux kernel stack after about 11 iterations, less than the limit of 20. One solution would be to decrease `LUAI_MAXCCALLS`. This could be made to work, but it has a few drawbacks: 1. The existing test suite does not pass with `LUAI_MAXCCALLS=10`. 2. There may be other LUA functions that use a lot of stack space, and the stack space may change depending on compiler version and options. This commit addresses the problem by adding a new limit on the amount of free space (in bytes) remaining on the C stack while running the LUA interpreter: `LUAI_MINCSTACK=4096`. If there is less than this amount of stack space remaining, a LUA runtime error is generated. Reviewed-by: George Wilson <gwilson@delphix.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Ryan Moeller <ryan@ixsystems.com> Reviewed-by: Allan Jude <allanjude@freebsd.org> Reviewed-by: Serapheim Dimitropoulos <serapheim@delphix.com> Signed-off-by: Matthew Ahrens <mahrens@delphix.com> Closes #10611 Closes #10613
This commit is contained in:
parent
e64cc4954c
commit
3eabed74c0
|
@ -598,10 +598,12 @@ l_noret luaG_errormsg (lua_State *L) {
|
||||||
|
|
||||||
|
|
||||||
l_noret luaG_runerror (lua_State *L, const char *fmt, ...) {
|
l_noret luaG_runerror (lua_State *L, const char *fmt, ...) {
|
||||||
|
L->runerror++;
|
||||||
va_list argp;
|
va_list argp;
|
||||||
va_start(argp, fmt);
|
va_start(argp, fmt);
|
||||||
addinfo(L, luaO_pushvfstring(L, fmt, argp));
|
addinfo(L, luaO_pushvfstring(L, fmt, argp));
|
||||||
va_end(argp);
|
va_end(argp);
|
||||||
luaG_errormsg(L);
|
luaG_errormsg(L);
|
||||||
|
L->runerror--;
|
||||||
}
|
}
|
||||||
/* END CSTYLED */
|
/* END CSTYLED */
|
||||||
|
|
|
@ -29,6 +29,24 @@
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
/* Return the number of bytes available on the stack. */
|
||||||
|
#if defined (_KERNEL) && defined(__linux__)
|
||||||
|
#include <asm/current.h>
|
||||||
|
static intptr_t stack_remaining(void) {
|
||||||
|
char local;
|
||||||
|
return (intptr_t)(&local - (char *)current->stack);
|
||||||
|
}
|
||||||
|
#elif defined (_KERNEL) && defined(__FreeBSD__)
|
||||||
|
#include <sys/pcpu.h>
|
||||||
|
static intptr_t stack_remaining(void) {
|
||||||
|
char local;
|
||||||
|
return (intptr_t)(&local - (char *)curthread->td_kstack);
|
||||||
|
}
|
||||||
|
#else
|
||||||
|
static intptr_t stack_remaining(void) {
|
||||||
|
return INTPTR_MAX;
|
||||||
|
}
|
||||||
|
#endif
|
||||||
|
|
||||||
/*
|
/*
|
||||||
** {======================================================
|
** {======================================================
|
||||||
|
@ -445,8 +463,13 @@ void luaD_call (lua_State *L, StkId func, int nResults, int allowyield) {
|
||||||
if (L->nCcalls == LUAI_MAXCCALLS)
|
if (L->nCcalls == LUAI_MAXCCALLS)
|
||||||
luaG_runerror(L, "C stack overflow");
|
luaG_runerror(L, "C stack overflow");
|
||||||
else if (L->nCcalls >= (LUAI_MAXCCALLS + (LUAI_MAXCCALLS>>3)))
|
else if (L->nCcalls >= (LUAI_MAXCCALLS + (LUAI_MAXCCALLS>>3)))
|
||||||
luaD_throw(L, LUA_ERRERR); /* error while handing stack error */
|
luaD_throw(L, LUA_ERRERR); /* error while handling stack error */
|
||||||
}
|
}
|
||||||
|
intptr_t remaining = stack_remaining();
|
||||||
|
if (L->runerror == 0 && remaining < LUAI_MINCSTACK)
|
||||||
|
luaG_runerror(L, "C stack overflow");
|
||||||
|
if (L->runerror != 0 && remaining < LUAI_MINCSTACK / 2)
|
||||||
|
luaD_throw(L, LUA_ERRERR); /* error while handling stack error */
|
||||||
if (!allowyield) L->nny++;
|
if (!allowyield) L->nny++;
|
||||||
if (!luaD_precall(L, func, nResults)) /* is a Lua function? */
|
if (!luaD_precall(L, func, nResults)) /* is a Lua function? */
|
||||||
luaV_execute(L); /* call it */
|
luaV_execute(L); /* call it */
|
||||||
|
|
|
@ -122,6 +122,12 @@ typedef LUAI_UACNUMBER l_uacNumber;
|
||||||
#define LUAI_MAXCCALLS 20
|
#define LUAI_MAXCCALLS 20
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Minimum amount of available stack space (in bytes) to make a C call. With
|
||||||
|
* gsub() recursion, the stack space between each luaD_call() is 1256 bytes.
|
||||||
|
*/
|
||||||
|
#define LUAI_MINCSTACK 4096
|
||||||
|
|
||||||
/*
|
/*
|
||||||
** maximum number of upvalues in a closure (both C and Lua). (Value
|
** maximum number of upvalues in a closure (both C and Lua). (Value
|
||||||
** must fit in an unsigned char.)
|
** must fit in an unsigned char.)
|
||||||
|
|
|
@ -214,6 +214,7 @@ static void preinit_state (lua_State *L, global_State *g) {
|
||||||
L->nny = 1;
|
L->nny = 1;
|
||||||
L->status = LUA_OK;
|
L->status = LUA_OK;
|
||||||
L->errfunc = 0;
|
L->errfunc = 0;
|
||||||
|
L->runerror = 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -166,6 +166,7 @@ struct lua_State {
|
||||||
unsigned short nCcalls; /* number of nested C calls */
|
unsigned short nCcalls; /* number of nested C calls */
|
||||||
lu_byte hookmask;
|
lu_byte hookmask;
|
||||||
lu_byte allowhook;
|
lu_byte allowhook;
|
||||||
|
lu_byte runerror; /* handling a runtime error */
|
||||||
int basehookcount;
|
int basehookcount;
|
||||||
int hookcount;
|
int hookcount;
|
||||||
lua_Hook hook;
|
lua_Hook hook;
|
||||||
|
|
|
@ -74,7 +74,7 @@ tests = ['tst.args_to_lua', 'tst.divide_by_zero', 'tst.exists',
|
||||||
'tst.memory_limit', 'tst.nested_neg', 'tst.nested_pos', 'tst.nvlist_to_lua',
|
'tst.memory_limit', 'tst.nested_neg', 'tst.nested_pos', 'tst.nvlist_to_lua',
|
||||||
'tst.recursive_neg', 'tst.recursive_pos', 'tst.return_large',
|
'tst.recursive_neg', 'tst.recursive_pos', 'tst.return_large',
|
||||||
'tst.return_nvlist_neg', 'tst.return_nvlist_pos',
|
'tst.return_nvlist_neg', 'tst.return_nvlist_pos',
|
||||||
'tst.return_recursive_table', 'tst.timeout']
|
'tst.return_recursive_table', 'tst.stack_gsub', 'tst.timeout']
|
||||||
tags = ['functional', 'channel_program', 'lua_core']
|
tags = ['functional', 'channel_program', 'lua_core']
|
||||||
|
|
||||||
[tests/functional/channel_program/synctask_core]
|
[tests/functional/channel_program/synctask_core]
|
||||||
|
|
|
@ -35,6 +35,7 @@ DISABLE_IVSET_GUID_CHECK disable_ivset_guid_check zfs_disable_ivset_guid_check
|
||||||
INITIALIZE_CHUNK_SIZE initialize_chunk_size zfs_initialize_chunk_size
|
INITIALIZE_CHUNK_SIZE initialize_chunk_size zfs_initialize_chunk_size
|
||||||
INITIALIZE_VALUE initialize_value zfs_initialize_value
|
INITIALIZE_VALUE initialize_value zfs_initialize_value
|
||||||
KEEP_LOG_SPACEMAPS_AT_EXPORT keep_log_spacemaps_at_export zfs_keep_log_spacemaps_at_export
|
KEEP_LOG_SPACEMAPS_AT_EXPORT keep_log_spacemaps_at_export zfs_keep_log_spacemaps_at_export
|
||||||
|
LUA_MAX_MEMLIMIT lua.max_memlimit zfs_lua_max_memlimit
|
||||||
L2ARC_NOPREFETCH l2arc.noprefetch l2arc_noprefetch
|
L2ARC_NOPREFETCH l2arc.noprefetch l2arc_noprefetch
|
||||||
L2ARC_REBUILD_BLOCKS_MIN_L2SIZE l2arc.rebuild_blocks_min_l2size l2arc_rebuild_blocks_min_l2size
|
L2ARC_REBUILD_BLOCKS_MIN_L2SIZE l2arc.rebuild_blocks_min_l2size l2arc_rebuild_blocks_min_l2size
|
||||||
L2ARC_REBUILD_ENABLED l2arc.rebuild_enabled l2arc_rebuild_enabled
|
L2ARC_REBUILD_ENABLED l2arc.rebuild_enabled l2arc_rebuild_enabled
|
||||||
|
|
|
@ -21,6 +21,7 @@ dist_pkgdata_SCRIPTS = \
|
||||||
tst.return_nvlist_neg.ksh \
|
tst.return_nvlist_neg.ksh \
|
||||||
tst.return_nvlist_pos.ksh \
|
tst.return_nvlist_pos.ksh \
|
||||||
tst.return_recursive_table.ksh \
|
tst.return_recursive_table.ksh \
|
||||||
|
tst.stack_gsub.ksh \
|
||||||
tst.timeout.ksh
|
tst.timeout.ksh
|
||||||
|
|
||||||
dist_pkgdata_DATA = \
|
dist_pkgdata_DATA = \
|
||||||
|
@ -40,4 +41,6 @@ dist_pkgdata_DATA = \
|
||||||
tst.recursive.zcp \
|
tst.recursive.zcp \
|
||||||
tst.return_large.zcp \
|
tst.return_large.zcp \
|
||||||
tst.return_recursive_table.zcp \
|
tst.return_recursive_table.zcp \
|
||||||
|
tst.stack_gsub.err \
|
||||||
|
tst.stack_gsub.zcp \
|
||||||
tst.timeout.zcp
|
tst.timeout.zcp
|
||||||
|
|
|
@ -61,6 +61,9 @@ log_mustnot_checkerror_program "Memory limit exhausted" -m 1 $TESTPOOL - <<-EOF
|
||||||
return s
|
return s
|
||||||
EOF
|
EOF
|
||||||
|
|
||||||
|
# Set the memlimit, in case it is a non-default value
|
||||||
|
log_must set_tunable32 LUA_MAX_MEMLIMIT 100000000
|
||||||
|
|
||||||
log_mustnot_checkerror_program "Invalid instruction or memory limit" \
|
log_mustnot_checkerror_program "Invalid instruction or memory limit" \
|
||||||
-m 200000000 $TESTPOOL - <<-EOF
|
-m 200000000 $TESTPOOL - <<-EOF
|
||||||
return 1;
|
return 1;
|
||||||
|
|
|
@ -0,0 +1,18 @@
|
||||||
|
Channel program execution failed:
|
||||||
|
C stack overflow
|
||||||
|
stack traceback:
|
||||||
|
[C]: in function 'gsub'
|
||||||
|
[string "channel program"]:17: in function <[string "channel program"]:16>
|
||||||
|
[C]: in function 'gsub'
|
||||||
|
[string "channel program"]:17: in function <[string "channel program"]:16>
|
||||||
|
[C]: in function 'gsub'
|
||||||
|
[string "channel program"]:17: in function <[string "channel program"]:16>
|
||||||
|
[C]: in function 'gsub'
|
||||||
|
[string "channel program"]:17: in function <[string "channel program"]:16>
|
||||||
|
[C]: in function 'gsub'
|
||||||
|
[string "channel program"]:17: in function <[string "channel program"]:16>
|
||||||
|
[C]: in function 'gsub'
|
||||||
|
[string "channel program"]:17: in function <[string "channel program"]:16>
|
||||||
|
[C]: in function 'gsub'
|
||||||
|
[string "channel program"]:17: in function <[string "channel program"]:16>
|
||||||
|
(...tail calls...)
|
|
@ -0,0 +1,33 @@
|
||||||
|
#!/bin/ksh -p
|
||||||
|
#
|
||||||
|
# This file and its contents are supplied under the terms of the
|
||||||
|
# Common Development and Distribution License ("CDDL"), version 1.0.
|
||||||
|
# You may only use this file in accordance with the terms of version
|
||||||
|
# 1.0 of the CDDL.
|
||||||
|
#
|
||||||
|
# A full copy of the text of the CDDL should have accompanied this
|
||||||
|
# source. A copy of the CDDL is also available via the Internet at
|
||||||
|
# http://www.illumos.org/license/CDDL.
|
||||||
|
#
|
||||||
|
|
||||||
|
#
|
||||||
|
# Copyright (c) 2020 by Delphix. All rights reserved.
|
||||||
|
#
|
||||||
|
|
||||||
|
. $STF_SUITE/tests/functional/channel_program/channel_common.kshlib
|
||||||
|
|
||||||
|
#
|
||||||
|
# DESCRIPTION:
|
||||||
|
# Overflowing the C stack using recursive gsub() should be handled
|
||||||
|
# gracefully. gsub() uses more stack space than typical, so it relies
|
||||||
|
# on LUAI_MINCSTACK to ensure that we don't overflow the Linux kernel's
|
||||||
|
# stack.
|
||||||
|
#
|
||||||
|
|
||||||
|
verify_runnable "global"
|
||||||
|
|
||||||
|
log_assert "recursive gsub() should be handled gracefully"
|
||||||
|
|
||||||
|
log_mustnot_program $TESTPOOL $ZCP_ROOT/lua_core/tst.stack_gsub.zcp
|
||||||
|
|
||||||
|
log_pass "recursive gsub() should be handled gracefully"
|
|
@ -0,0 +1,20 @@
|
||||||
|
--
|
||||||
|
-- This file and its contents are supplied under the terms of the
|
||||||
|
-- Common Development and Distribution License ("CDDL"), version 1.0.
|
||||||
|
-- You may only use this file in accordance with the terms of version
|
||||||
|
-- 1.0 of the CDDL.
|
||||||
|
--
|
||||||
|
-- A full copy of the text of the CDDL should have accompanied this
|
||||||
|
-- source. A copy of the CDDL is also available via the Internet at
|
||||||
|
-- http://www.illumos.org/license/CDDL.
|
||||||
|
--
|
||||||
|
|
||||||
|
--
|
||||||
|
-- Copyright (c) 2020 by Delphix. All rights reserved.
|
||||||
|
--
|
||||||
|
|
||||||
|
function f(s)
|
||||||
|
return string.gsub(s, ".", f)
|
||||||
|
end
|
||||||
|
|
||||||
|
return f("foo")
|
Loading…
Reference in New Issue