]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
common: fix backtrace leak in __ceph_abort and friends 63300/head
authorKefu Chai <tchaikov@gmail.com>
Thu, 15 May 2025 13:03:33 +0000 (21:03 +0800)
committerKefu Chai <tchaikov@gmail.com>
Thu, 15 May 2025 13:13:50 +0000 (21:13 +0800)
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*, 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*, 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 <tchaikov@gmail.com>
src/common/assert.cc

index ff20f55964eee6a7fecc1a4093ebd695b5636be3..2cd5cadd86df29c52902f611949aae3fe338cf32 100644 (file)
@@ -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<ClibBackTrace>(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<ClibBackTrace>(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<ClibBackTrace>(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(),