From d8adc49fa9dc9b13dd2da0fca3ea33582410bf5d Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Mon, 9 Jun 2025 18:26:21 +0800 Subject: [PATCH] rgw/rgw_lua_utils: fix memory leak in luaL_error() formatting Previously, error messages passed to luaL_error() were formatted using std::string concatenation. Since luaL_error() never returns (it throws a Lua exception via longjmp), the allocated std::string memory was leaked, as detected by AddressSanitizer: ``` Direct leak of 105 byte(s) in 1 object(s) allocated from: #0 0x7fc5f1921a2d in operator new(unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:86 #1 0x563bd89144c7 in std::__new_allocator::allocate(unsigned long, void const*) /usr/include/c++/15.1.1/bits/new_allocator.h:151 #2 0x563bd89144c7 in std::allocator::allocate(unsigned long) /usr/include/c++/15.1.1/bits/allocator.h:203 #3 0x563bd89144c7 in std::allocator_traits >::allocate(std::allocator&, unsigned long) /usr/include/c++/15.1.1/bits/alloc_traits.h:614 #4 0x563bd89144c7 in std::__cxx11::basic_string, std::allocator >::_S_allocate(std::allocator&, unsigned long) /usr/include/c++/15.1.1/bits/basic_string.h:142 #5 0x563bd89144c7 in std::__cxx11::basic_string, std::allocator >::_M_create(unsigned long&, unsigned long) /usr/include/c++/15.1.1/bits/basic_string.tcc:164 #6 0x563bd896ae1b in std::__cxx11::basic_string, std::allocator >::_M_mutate(unsigned long, unsigned long, char const*, unsigned long) /usr/include/c++/15.1.1/bits/basic_string.tcc:363 #7 0x563bd896b256 in std::__cxx11::basic_string, std::allocator >::_M_append(char const*, unsigned long) /usr/include/c++/15.1.1/bits/basic_string.tcc:455 #8 0x563bd896b2bb in std::__cxx11::basic_string, std::allocator >::append(char const*) /usr/include/c++/15.1.1/bits/basic_string.h:1585 #9 0x563bd943c2f2 in std::__cxx11::basic_string, std::allocator > std::operator+, std::allocator >(std::__cxx11::basic_string, std::allocator >&&, char const*) /usr/include/c++/15.1.1/bits/basic_string.h:3977 #10 0x563bd943c2f2 in rgw::lua::lua_state_guard::runtime_hook(lua_State*, lua_Debug*) /home/kefu/dev/ceph/src/rgw/rgw_lua_utils.cc:245 #11 0x7fc5f139f8ef (/usr/lib/liblua.so.5.4+0xe8ef) (BuildId: b7533e2973d4b0d82e10fc29973ec5a8d355d2b8) #12 0x7fc5f139fbfe (/usr/lib/liblua.so.5.4+0xebfe) (BuildId: b7533e2973d4b0d82e10fc29973ec5a8d355d2b8) #13 0x7fc5f13b26fe (/usr/lib/liblua.so.5.4+0x216fe) (BuildId: b7533e2973d4b0d82e10fc29973ec5a8d355d2b8) #14 0x7fc5f139f581 (/usr/lib/liblua.so.5.4+0xe581) (BuildId: b7533e2973d4b0d82e10fc29973ec5a8d355d2b8) #15 0x7fc5f139b735 (/usr/lib/liblua.so.5.4+0xa735) (BuildId: b7533e2973d4b0d82e10fc29973ec5a8d355d2b8) #16 0x7fc5f139ba8f (/usr/lib/liblua.so.5.4+0xaa8f) (BuildId: b7533e2973d4b0d82e10fc29973ec5a8d355d2b8) #17 0x7fc5f139f696 in lua_pcallk (/usr/lib/liblua.so.5.4+0xe696) (BuildId: b7533e2973d4b0d82e10fc29973ec5a8d355d2b8) #18 0x563bd8a925ef in rgw::lua::request::execute(rgw::sal::Driver*, RGWREST*, OpsLogSink*, req_state*, RGWOp*, std::__cxx11::basic_string, std::allocator > const&) /home/kefu/dev/ceph/src/rgw/rgw_lua_request.cc:824 #19 0x563bd8952e3d in TestRGWLua_LuaRuntimeLimit_Test::TestBody() /home/kefu/dev/ceph/src/test/rgw/test_rgw_lua.cc:1628 #20 0x563bd8a37817 in void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) /home/kefu/dev/ceph/src/googletest/googletest/src/gtest.cc:2653 #21 0x563bd8a509b5 in void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (/home/kefu/dev/ceph/build/bin/unittest_rgw_lua+0x11199b5) (BuildId: b2628caba5290d882d25f7bea166f058b682bc85)` ``` This change replaces std::string formatting with stack-allocated buffer and std::to_chars() to eliminate the memory leak. Note: We cannot format int64_t directly through luaL_error() because lua_pushfstring() does not support long long or int64_t format specifiers, even in Lua 5.4 (see https://www.lua.org/manual/5.4/manual.html#lua_pushfstring). Since libstdc++ uses int64_t for std::chrono::milliseconds::rep, we use std::to_chars() for safe, efficient conversion without heap allocation. The maximum runtime limit was a configuration introduced by 3e3cb156. Fixes: https://tracker.ceph.com/issues/71595 Signed-off-by: Kefu Chai --- src/rgw/rgw_lua_utils.cc | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/rgw/rgw_lua_utils.cc b/src/rgw/rgw_lua_utils.cc index b4dbc75e4fa..b5448189dbe 100644 --- a/src/rgw/rgw_lua_utils.cc +++ b/src/rgw/rgw_lua_utils.cc @@ -241,9 +241,16 @@ void lua_state_guard::runtime_hook(lua_State* L, lua_Debug* ar) { std::chrono::duration_cast(now - start_time); if (elapsed > max_runtime) { - std::string err = "Lua runtime limit exceeded: total elapsed time is " + - std::to_string(elapsed.count()) + " ms"; - luaL_error(L, "%s", err.c_str()); + // +1 for max digit, +1 for null terminator + constexpr size_t max_digits = std::numeric_limits::digits10 + 2; + char elapsed_str[max_digits] = {}; + auto [ptr, ec] = std::to_chars(std::begin(elapsed_str), std::end(elapsed_str), elapsed.count()); + // buffer too small, should never happen though + assert(ec == std::errc{}); + *ptr = '\0'; + // luaL_error() never returns, so we have to format the number in the hard way instead of + // using std::to_string or fmt::to_string + luaL_error(L, "Lua runtime limit exceeded: total elapsed time is %s ms", elapsed_str); } } -- 2.39.5