From aaa96072fef630da0804b359d2e5e8303f4c63f2 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sat, 4 May 2024 11:50:54 +0800 Subject: [PATCH] test/objectstore/test_bluefs: fix heap-use-after-free this change was created in the same spirit of b8c30a79. in BlueFS.test_shared_alloc and BlueFS.test_shared_alloc_sparse, we keep the return value of `fs.get_perf_counters()`, and dereference it after umounting the fs, but the `PerfCounters*` pointer returned from `fs.get_perf_counters()` is destroyed in `BlueFS::_shutdown_logger()` which is in turn called by `BlueFS::umount()`. so ASan points this out: ``` ==548153==ERROR: AddressSanitizer: heap-use-after-free on address 0x6110000336c0 at pc 0x7fc810326654 bp 0x7ffd869be8f0 sp 0x7ffd869be8e8 READ of size 8 at 0x6110000336c0 thread T0 #0 0x7fc810326653 in ceph::common::PerfCounters::get(int) const /home/jenkins-build/build/workspace/ceph-pull-requests/src/common/perf_counters.cc:246:8 #1 0x564e7a5397a5 in BlueFS_test_shared_alloc_sparse_Test::TestBody() /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/objectstore/test_bluefs.cc:1265:3 #2 0x564e7a644006 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:2605:10 #3 0x564e7a5fdbc2 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:2641:14 #4 0x564e7a5ae7ec in testing::Test::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2680:5 #5 0x564e7a5b0822 in testing::TestInfo::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2858:11 #6 0x564e7a5b1e5b in testing::TestSuite::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:3012:28 #7 0x564e7a5cf2e8 in testing::internal::UnitTestImpl::RunAllTests() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:5723:44 #8 0x564e7a64c8b6 in bool testing::internal::HandleSehExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2605:10 #9 0x564e7a604662 in bool testing::internal::HandleExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2641:14 #10 0x564e7a5ce672 in testing::UnitTest::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:5306:10 #11 0x564e7a55a410 in RUN_ALL_TESTS() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/include/gtest/gtest.h:2486:46 #12 0x564e7a551295 in main /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/objectstore/test_bluefs.cc:1609:10 #13 0x7fc80d775d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 #14 0x7fc80d775e3f in __libc_start_main csu/../csu/libc-start.c:392:3 #15 0x564e7a4296a4 in _start (/home/jenkins-build/build/workspace/ceph-pull-requests/build/bin/unittest_bluefs+0x2856a4) (BuildId: fd4e4e0b1c2f9a3b0c1a7051d8ed68b3576e3277) 0x6110000336c0 is located 0 bytes inside of 208-byte region [0x6110000336c0,0x611000033790) freed by thread T0 here: #0 0x564e7a4e7b1d in operator delete(void*) (/home/jenkins-build/build/workspace/ceph-pull-requests/build/bin/unittest_bluefs+0x343b1d) (BuildId: fd4e4e0b1c2f9a3b0c1a7051d8ed68b3576e3277) #1 0x564e7a686ce3 in BlueFS::_shutdown_logger() /home/jenkins-build/build/workspace/ceph-pull-requests/src/os/bluestore/BlueFS.cc:462:3 #2 0x564e7a6a9b55 in BlueFS::umount(bool) /home/jenkins-build/build/workspace/ceph-pull-requests/src/os/bluestore/BlueFS.cc:1076:3 #3 0x564e7a539767 in BlueFS_test_shared_alloc_sparse_Test::TestBody() /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/objectstore/test_bluefs.cc:1262:6 #4 0x564e7a644006 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:2605:10 #5 0x564e7a5fdbc2 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:2641:14 #6 0x564e7a5ae7ec in testing::Test::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2680:5 #7 0x564e7a5b0822 in testing::TestInfo::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2858:11 #8 0x564e7a5b1e5b in testing::TestSuite::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:3012:28 #9 0x564e7a5cf2e8 in testing::internal::UnitTestImpl::RunAllTests() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:5723:44 #10 0x564e7a64c8b6 in bool testing::internal::HandleSehExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2605:10 #11 0x564e7a604662 in bool testing::internal::HandleExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2641:14 #12 0x564e7a5ce672 in testing::UnitTest::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:5306:10 #13 0x564e7a55a410 in RUN_ALL_TESTS() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/include/gtest/gtest.h:2486:46 #14 0x564e7a551295 in main /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/objectstore/test_bluefs.cc:1609:10 #15 0x7fc80d775d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 ``` in this change, instead of keeping `logger` across the `umount()` and `mount()` calls, we get another instance of `logger`, query it for the perf counter that we are interested, and compare the value to see if it is unchanged. this should address the ASan warning above. Signed-off-by: Kefu Chai --- src/test/objectstore/test_bluefs.cc | 42 ++++++++++++++++++----------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/src/test/objectstore/test_bluefs.cc b/src/test/objectstore/test_bluefs.cc index 4b8dc242720d1..66451e76bca37 100644 --- a/src/test/objectstore/test_bluefs.cc +++ b/src/test/objectstore/test_bluefs.cc @@ -1256,14 +1256,20 @@ TEST(BlueFS, test_shared_alloc_sparse) { } } fs.compact_log(); - auto *logger = fs.get_perf_counters(); - ASSERT_NE(logger->get(l_bluefs_alloc_shared_size_fallbacks), 0); - auto num_files = logger->get(l_bluefs_num_files); - fs.umount(); - fs.mount(); - ASSERT_EQ(num_files, logger->get(l_bluefs_num_files)); - fs.umount(); + uint64_t num_files = 0; + { + auto *logger = fs.get_perf_counters(); + ASSERT_NE(logger->get(l_bluefs_alloc_shared_size_fallbacks), 0); + num_files = logger->get(l_bluefs_num_files); + fs.umount(); + } + { + fs.mount(); + auto *logger = fs.get_perf_counters(); + ASSERT_EQ(num_files, logger->get(l_bluefs_num_files)); + fs.umount(); + } } TEST(BlueFS, test_4k_shared_alloc) { @@ -1326,15 +1332,21 @@ TEST(BlueFS, test_4k_shared_alloc) { } } fs.compact_log(); - auto *logger = fs.get_perf_counters(); - ASSERT_EQ(logger->get(l_bluefs_alloc_shared_dev_fallbacks), 0); - ASSERT_EQ(logger->get(l_bluefs_alloc_shared_size_fallbacks), 0); - auto num_files = logger->get(l_bluefs_num_files); - fs.umount(); - fs.mount(); - ASSERT_EQ(num_files, logger->get(l_bluefs_num_files)); - fs.umount(); + uint64_t num_files = 0; + { + auto *logger = fs.get_perf_counters(); + ASSERT_EQ(logger->get(l_bluefs_alloc_shared_dev_fallbacks), 0); + ASSERT_EQ(logger->get(l_bluefs_alloc_shared_size_fallbacks), 0); + num_files = logger->get(l_bluefs_num_files); + fs.umount(); + } + { + fs.mount(); + auto *logger = fs.get_perf_counters(); + ASSERT_EQ(num_files, logger->get(l_bluefs_num_files)); + fs.umount(); + } } void create_files(BlueFS &fs,