This is an automated email from the ASF dual-hosted git repository.
git-hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new 567fcf6c2 fix(scripting): guard against stack overflow on deeply
nested Lua reply (#3547)
567fcf6c2 is described below
commit 567fcf6c26fc38367c1bda81ca0541c6ab861086
Author: hulk <[email protected]>
AuthorDate: Thu Jul 9 11:11:23 2026 +0800
fix(scripting): guard against stack overflow on deeply nested Lua reply
(#3547)
ReplyToRedisReply() converts a Lua return value into a RESP reply with
native C++ recursion that runs after lua_pcall() returns,
so Lua's own recursion guard does not apply. A deeply self-nested table
could recurse until the worker thread stack overflowed and crashed
the whole server (the Lua VM is shared per worker).
Add a lua_checkstack() guard so such a reply fails with "reached lua
stack limit"
instead of crashing. Includes a Go regression test in the scripting
suite.
Assistant-By Claude Fable 5
---
src/storage/scripting.cc | 64 ++++++++++++++++++++++-----
tests/gocase/unit/scripting/scripting_test.go | 39 ++++++++++++++++
2 files changed, 93 insertions(+), 10 deletions(-)
diff --git a/src/storage/scripting.cc b/src/storage/scripting.cc
index 49727b112..aa31c09d0 100644
--- a/src/storage/scripting.cc
+++ b/src/storage/scripting.cc
@@ -1271,13 +1271,27 @@ void PushError(lua_State *lua, const char *err) {
lua_settable(lua, -3);
}
-// this function does not pop any element on the stack
-std::string ReplyToRedisReply(redis::Connection *conn, lua_State *lua) {
+// Max nesting depth of a Lua reply table. The conversion below recurses on the
+// native C stack (Lua's own recursion guard doesn't apply after lua_pcall
+// returns), and worker threads may have small stacks (512KiB by default on
+// macOS), so the cap must be low enough to keep ~100 frames affordable.
+constexpr int kMaxLuaReplyNestingDepth = 100;
+
+// Convert the Lua value at the top of the stack (and everything nested in it)
+// into RESP protocol bytes. This function does not pop any element on the
stack.
+StatusOr<std::string> LuaTypeToRedisReply(redis::Connection *conn, lua_State
*lua, int depth) {
std::string output;
const char *obj_s = nullptr;
size_t obj_len = 0;
int j = 0, mbulklen = 0;
+ // Also reserve Lua stack room before converting this level (a map reply
+ // pushes up to 4 slots). On failure the whole reply is discarded by the
+ // top-level caller and turned into a single error.
+ if (depth > kMaxLuaReplyNestingDepth || !lua_checkstack(lua, 4)) {
+ return {Status::NotOK, "reached lua stack limit"};
+ }
+
int t = lua_type(lua, -1);
switch (t) {
case LUA_TSTRING:
@@ -1384,11 +1398,21 @@ std::string ReplyToRedisReply(redis::Connection *conn,
lua_State *lua) {
while (lua_next(lua, -2)) {
lua_pushvalue(lua, -2);
// return key
- map_output += ReplyToRedisReply(conn, lua);
- lua_pop(lua, 1);
+ auto key_output = LuaTypeToRedisReply(conn, lua, depth + 1);
+ if (!key_output) {
+ lua_pop(lua, 4); // pop the key copy, value, iteration key and
'map' table
+ return key_output;
+ }
+ map_output += *key_output;
+ lua_pop(lua, 1); // pop the converted key copy
// return value
- map_output += ReplyToRedisReply(conn, lua);
- lua_pop(lua, 1);
+ auto value_output = LuaTypeToRedisReply(conn, lua, depth + 1);
+ if (!value_output) {
+ lua_pop(lua, 3); // pop the value, iteration key and 'map' table
+ return value_output;
+ }
+ map_output += *value_output;
+ lua_pop(lua, 1); // pop the converted value
map_len++;
}
output = conn->HeaderOfMap(map_len) + std::move(map_output);
@@ -1408,8 +1432,13 @@ std::string ReplyToRedisReply(redis::Connection *conn,
lua_State *lua) {
while (lua_next(lua, -2)) {
lua_pop(lua, 1);
lua_pushvalue(lua, -1);
- set_output += ReplyToRedisReply(conn, lua);
- lua_pop(lua, 1);
+ auto entry_output = LuaTypeToRedisReply(conn, lua, depth + 1);
+ if (!entry_output) {
+ lua_pop(lua, 3); // pop the entry copy, iteration key and 'set'
table
+ return entry_output;
+ }
+ set_output += *entry_output;
+ lua_pop(lua, 1); // pop the converted entry copy
set_len++;
}
output = conn->HeaderOfSet(set_len) + std::move(set_output);
@@ -1428,8 +1457,13 @@ std::string ReplyToRedisReply(redis::Connection *conn,
lua_State *lua) {
break;
}
mbulklen++;
- output += ReplyToRedisReply(conn, lua);
- lua_pop(lua, 1);
+ auto element_output = LuaTypeToRedisReply(conn, lua, depth + 1);
+ if (!element_output) {
+ lua_pop(lua, 1); // pop the element
+ return element_output;
+ }
+ output += *element_output;
+ lua_pop(lua, 1); // pop the converted element
}
output = redis::MultiLen(mbulklen) + output;
break;
@@ -1439,6 +1473,16 @@ std::string ReplyToRedisReply(redis::Connection *conn,
lua_State *lua) {
return output;
}
+std::string ReplyToRedisReply(redis::Connection *conn, lua_State *lua) {
+ auto reply = LuaTypeToRedisReply(conn, lua, 1);
+ if (!reply) {
+ // Discard the partially converted reply and report a single top-level
+ // error instead of embedding it deep inside nested array headers.
+ return redis::Error({Status::RedisErrorNoPrefix, reply.Msg()});
+ }
+ return std::move(*reply);
+}
+
/* In case the error set into the Lua stack by pushError() was generated
* by the non-error-trapping version of redis.pcall(), which is redis.call(),
* this function will raise the Lua error so that the execution of the
diff --git a/tests/gocase/unit/scripting/scripting_test.go
b/tests/gocase/unit/scripting/scripting_test.go
index e1318977b..47bd5bbd8 100644
--- a/tests/gocase/unit/scripting/scripting_test.go
+++ b/tests/gocase/unit/scripting/scripting_test.go
@@ -958,3 +958,42 @@ func TestLuaJITBytecodeDoS(t *testing.T) {
require.Equal(t, []interface{}{"load_failed:attempt to load chunk with
wrong mode"}, r.Val())
require.NoError(t, rdb.Ping(ctx).Err())
}
+
+// TestScriptEvalDeeplyNestedTableReply is a regression test for a deeply
+// self-nested Lua reply that used to overflow the stack and crash the server.
+func TestScriptEvalDeeplyNestedTableReply(t *testing.T) {
+ srv := util.StartServer(t, map[string]string{})
+ defer srv.Close()
+
+ ctx := context.Background()
+ rdb := srv.NewClient()
+ defer func() { require.NoError(t, rdb.Close()) }()
+
+ require.Equal(t, "PONG", rdb.Ping(ctx).Val())
+
+ require.NoError(t, rdb.Eval(ctx, `local t={} for i=1,50 do t={t} end
return t`, nil).Err())
+
+ // One script per recursive conversion site in ReplyToRedisReply
+ const depth = 200000
+ for name, format := range map[string]string{
+ "array element": `local t={} for i=1,%d do t={t}
end return t`,
+ "array element after sibling": `local t={} for i=1,%d do t={t}
end return {1, t, 3}`,
+ "map key": `local t={} for i=1,%d do t={t}
end return {map={[t]=1}}`,
+ "map value": `local t={} for i=1,%d do t={t}
end return {map={key=t}}`,
+ "set entry": `local t={} for i=1,%d do t={t}
end return {set={[t]=true}}`,
+ } {
+ t.Run(name, func(t *testing.T) {
+ err := rdb.Eval(ctx, fmt.Sprintf(format, depth),
nil).Err()
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "reached lua stack
limit")
+
+ // confirm the error path left the Lua stack top in a
good state
+ require.Equal(t, []interface{}{int64(1), int64(2),
int64(3)},
+ rdb.Eval(ctx, `return {1, 2, 3}`, nil).Val())
+ })
+ }
+
+ require.Eventually(t, func() bool {
+ return rdb.Ping(ctx).Val() == "PONG"
+ }, 5*time.Second, 100*time.Millisecond)
+}
