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 <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Allan Jude <[email protected]>
Reviewed-by: Serapheim Dimitropoulos <[email protected]>
Signed-off-by: Matthew Ahrens <[email protected]>
Closes #10611 
Closes #10613 

7 files changed, 79 insertions, 1 deletions

diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run
index 61c095290..931eee4fa 100644
--- a/tests/runfiles/common.run
+++ b/tests/runfiles/common.run
@@ -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.recursive_neg', 'tst.recursive_pos', 'tst.return_large',
     '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']
 
 [tests/functional/channel_program/synctask_core]
diff --git a/tests/zfs-tests/include/tunables.cfg b/tests/zfs-tests/include/tunables.cfg
index df8824b1e..9191f2955 100644
--- a/tests/zfs-tests/include/tunables.cfg
+++ b/tests/zfs-tests/include/tunables.cfg
@@ -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_VALUE		initialize_value		zfs_initialize_value
 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_REBUILD_BLOCKS_MIN_L2SIZE	l2arc.rebuild_blocks_min_l2size	l2arc_rebuild_blocks_min_l2size
 L2ARC_REBUILD_ENABLED		l2arc.rebuild_enabled		l2arc_rebuild_enabled
diff --git a/tests/zfs-tests/tests/functional/channel_program/lua_core/Makefile.am b/tests/zfs-tests/tests/functional/channel_program/lua_core/Makefile.am
index e06b145dc..fb3520811 100644
--- a/tests/zfs-tests/tests/functional/channel_program/lua_core/Makefile.am
+++ b/tests/zfs-tests/tests/functional/channel_program/lua_core/Makefile.am
@@ -21,6 +21,7 @@ dist_pkgdata_SCRIPTS = \
 	tst.return_nvlist_neg.ksh \
 	tst.return_nvlist_pos.ksh \
 	tst.return_recursive_table.ksh \
+	tst.stack_gsub.ksh \
 	tst.timeout.ksh
 
 dist_pkgdata_DATA = \
@@ -40,4 +41,6 @@ dist_pkgdata_DATA = \
 	tst.recursive.zcp \
 	tst.return_large.zcp \
 	tst.return_recursive_table.zcp \
+	tst.stack_gsub.err \
+	tst.stack_gsub.zcp \
 	tst.timeout.zcp
diff --git a/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.memory_limit.ksh b/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.memory_limit.ksh
index 288577568..0533b8fa3 100755
--- a/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.memory_limit.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.memory_limit.ksh
@@ -61,6 +61,9 @@ log_mustnot_checkerror_program "Memory limit exhausted" -m 1 $TESTPOOL - <<-EOF
 	return s
 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" \
     -m 200000000 $TESTPOOL - <<-EOF
 	return 1;
diff --git a/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.stack_gsub.err b/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.stack_gsub.err
new file mode 100644
index 000000000..45f2d9ef0
--- /dev/null
+++ b/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.stack_gsub.err
@@ -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...)
\ No newline at end of file
diff --git a/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.stack_gsub.ksh b/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.stack_gsub.ksh
new file mode 100755
index 000000000..ecabf3a3f
--- /dev/null
+++ b/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.stack_gsub.ksh
@@ -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"
diff --git a/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.stack_gsub.zcp b/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.stack_gsub.zcp
new file mode 100644
index 000000000..a493363ca
--- /dev/null
+++ b/tests/zfs-tests/tests/functional/channel_program/lua_core/tst.stack_gsub.zcp
@@ -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")