From da8f614180faa32e2da455107ede8f3a2ccbc72c Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Thu, 15 May 2025 21:03:33 +0800 Subject: [PATCH] common: fix backtrace leak in __ceph_abort and friends Previously, in __ceph_abort and related abort handlers, we allocated ClibBackTrace instances using raw pointers without proper cleanup. Since these handlers terminate execution, the leaks didn't affect production systems but were correctly flagged by ASan during testing: ``` Direct leak of 288 byte(s) in 1 object(s) allocated from: #0 0x55aefe8cb65d in operator new(unsigned long) (/home/jenkins-build/build/workspace/ceph-pull-requests/build/bin/unittest_ceph_assert+0x1f465d) (BuildId: a4faeddac80b0d81062bd53ede3388c0c10680bc) #1 0x7f3b84da988d in ceph::__ceph_assertf_fail(char const*, char const*, int, char const*, char const*, ...) /home/jenkins-build/build/workspace/ceph-pull-requests/src/common/assert.cc:157:21 #2 0x55aefe8cf04b in supressed_assertf_line22() /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/ceph_assert.cc:22:3 #3 0x55aefe8ce4e6 in CephAssertDeathTest_ceph_assert_supresssions_Test::TestBody() /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/ceph_assert.cc:31:3 #4 0x55aefe99135d in void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2653:10 #5 0x55aefe94f015 in void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2689:14 ... ``` This commit resolves the issue by using std::unique_ptr to manage the lifecycle of backtrace objects, ensuring proper cleanup even in non-returning functions. While these leaks had no practical impact in production (as the process terminates anyway), fixing them improves code quality and eliminates false positives in memory analysis tools. Signed-off-by: Kefu Chai --- src/common/assert.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/common/assert.cc b/src/common/assert.cc index ff20f55964e..2cd5cadd86d 100644 --- a/src/common/assert.cc +++ b/src/common/assert.cc @@ -154,7 +154,7 @@ namespace ceph { ceph_pthread_getname(g_assert_thread_name, sizeof(g_assert_thread_name)); BufAppender ba(g_assert_msg, sizeof(g_assert_msg)); - BackTrace *bt = new ClibBackTrace(1); + auto bt = std::make_unique(1); ba.printf("%s: In function '%s' thread %llx time %s\n" "%s: %d: FAILED ceph_assert(%s)\n", file, func, (unsigned long long)pthread_self(), tss.str().c_str(), @@ -212,7 +212,7 @@ namespace ceph { g_assert_thread = (unsigned long long)pthread_self(); ceph_pthread_getname(g_assert_thread_name, sizeof(g_assert_thread_name)); - BackTrace *bt = new ClibBackTrace(1); + auto bt = std::make_unique(1); snprintf(g_assert_msg, sizeof(g_assert_msg), "%s: In function '%s' thread %llx time %s\n" "%s: %d: ceph_abort_msg(\"%s\")\n", file, func, @@ -254,7 +254,7 @@ namespace ceph { ceph_pthread_getname(g_assert_thread_name, sizeof(g_assert_thread_name)); BufAppender ba(g_assert_msg, sizeof(g_assert_msg)); - BackTrace *bt = new ClibBackTrace(1); + auto bt = std::make_unique(1); ba.printf("%s: In function '%s' thread %llx time %s\n" "%s: %d: abort()\n", file, func, (unsigned long long)pthread_self(), tss.str().c_str(), -- 2.39.5