Skip to content

Commit aaa9607

Browse files
tchaikovtchaikov
committed
test/objectstore/test_bluefs: fix heap-use-after-free
this change was created in the same spirit of b8c30a7. 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 ceph#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 ceph#2 0x564e7a644006 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:2605:10 ceph#3 0x564e7a5fdbc2 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:2641:14 ceph#4 0x564e7a5ae7ec in testing::Test::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2680:5 ceph#5 0x564e7a5b0822 in testing::TestInfo::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2858:11 ceph#6 0x564e7a5b1e5b in testing::TestSuite::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:3012:28 ceph#7 0x564e7a5cf2e8 in testing::internal::UnitTestImpl::RunAllTests() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:5723:44 ceph#8 0x564e7a64c8b6 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(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 ceph#9 0x564e7a604662 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(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 ceph#10 0x564e7a5ce672 in testing::UnitTest::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:5306:10 ceph#11 0x564e7a55a410 in RUN_ALL_TESTS() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/include/gtest/gtest.h:2486:46 ceph#12 0x564e7a551295 in main /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/objectstore/test_bluefs.cc:1609:10 ceph#13 0x7fc80d775d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 ceph#14 0x7fc80d775e3f in __libc_start_main csu/../csu/libc-start.c:392:3 ceph#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) ceph#1 0x564e7a686ce3 in BlueFS::_shutdown_logger() /home/jenkins-build/build/workspace/ceph-pull-requests/src/os/bluestore/BlueFS.cc:462:3 ceph#2 0x564e7a6a9b55 in BlueFS::umount(bool) /home/jenkins-build/build/workspace/ceph-pull-requests/src/os/bluestore/BlueFS.cc:1076:3 ceph#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 ceph#4 0x564e7a644006 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:2605:10 ceph#5 0x564e7a5fdbc2 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:2641:14 ceph#6 0x564e7a5ae7ec in testing::Test::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2680:5 ceph#7 0x564e7a5b0822 in testing::TestInfo::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:2858:11 ceph#8 0x564e7a5b1e5b in testing::TestSuite::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:3012:28 ceph#9 0x564e7a5cf2e8 in testing::internal::UnitTestImpl::RunAllTests() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:5723:44 ceph#10 0x564e7a64c8b6 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(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 ceph#11 0x564e7a604662 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(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 ceph#12 0x564e7a5ce672 in testing::UnitTest::Run() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:5306:10 ceph#13 0x564e7a55a410 in RUN_ALL_TESTS() /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/include/gtest/gtest.h:2486:46 ceph#14 0x564e7a551295 in main /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/objectstore/test_bluefs.cc:1609:10 ceph#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 <[email protected]>
1 parent 04416f4 commit aaa9607

File tree

1 file changed

+27
-15
lines changed

1 file changed

+27
-15
lines changed

‎src/test/objectstore/test_bluefs.cc‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1256,14 +1256,20 @@ TEST(BlueFS, test_shared_alloc_sparse) {
12561256
}
12571257
}
12581258
fs.compact_log();
1259-
auto *logger = fs.get_perf_counters();
1260-
ASSERT_NE(logger->get(l_bluefs_alloc_shared_size_fallbacks), 0);
1261-
auto num_files = logger->get(l_bluefs_num_files);
1262-
fs.umount();
12631259

1264-
fs.mount();
1265-
ASSERT_EQ(num_files, logger->get(l_bluefs_num_files));
1266-
fs.umount();
1260+
uint64_t num_files = 0;
1261+
{
1262+
auto *logger = fs.get_perf_counters();
1263+
ASSERT_NE(logger->get(l_bluefs_alloc_shared_size_fallbacks), 0);
1264+
num_files = logger->get(l_bluefs_num_files);
1265+
fs.umount();
1266+
}
1267+
{
1268+
fs.mount();
1269+
auto *logger = fs.get_perf_counters();
1270+
ASSERT_EQ(num_files, logger->get(l_bluefs_num_files));
1271+
fs.umount();
1272+
}
12671273
}
12681274

12691275
TEST(BlueFS, test_4k_shared_alloc) {
@@ -1326,15 +1332,21 @@ TEST(BlueFS, test_4k_shared_alloc) {
13261332
}
13271333
}
13281334
fs.compact_log();
1329-
auto *logger = fs.get_perf_counters();
1330-
ASSERT_EQ(logger->get(l_bluefs_alloc_shared_dev_fallbacks), 0);
1331-
ASSERT_EQ(logger->get(l_bluefs_alloc_shared_size_fallbacks), 0);
1332-
auto num_files = logger->get(l_bluefs_num_files);
1333-
fs.umount();
13341335

1335-
fs.mount();
1336-
ASSERT_EQ(num_files, logger->get(l_bluefs_num_files));
1337-
fs.umount();
1336+
uint64_t num_files = 0;
1337+
{
1338+
auto *logger = fs.get_perf_counters();
1339+
ASSERT_EQ(logger->get(l_bluefs_alloc_shared_dev_fallbacks), 0);
1340+
ASSERT_EQ(logger->get(l_bluefs_alloc_shared_size_fallbacks), 0);
1341+
num_files = logger->get(l_bluefs_num_files);
1342+
fs.umount();
1343+
}
1344+
{
1345+
fs.mount();
1346+
auto *logger = fs.get_perf_counters();
1347+
ASSERT_EQ(num_files, logger->get(l_bluefs_num_files));
1348+
fs.umount();
1349+
}
13381350
}
13391351

13401352
void create_files(BlueFS &fs,

0 commit comments

Comments
 (0)