From e6468f3db174a41dd0c5b9b05e870a2c05f0d042 Mon Sep 17 00:00:00 2001 From: Rongqi Sun Date: Thu, 30 May 2024 08:13:35 +0000 Subject: [PATCH] crush: avoid out-of-bound access and simplify enlarging buckets When sanitizer is enabled, a part of 'run-cli-tests' output shows, ``` ================================================================= ==1263095==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60c00000c000 at pc 0x7f80a4b0a040 bp 0x7ffe3176d550 sp 0x7ffe3176d548 READ of size 8 at 0x60c00000c000 thread T0 #0 0x7f80a4b0a03f in CrushWrapper::get_new_bucket_id() /home/jenkins-build/build/workspace/ceph-pull-requests/src/crush/CrushWrapper.cc:2189:10 #1 0x7f80a4b03f20 in CrushWrapper::reclassify(ceph::common::CephContext*, std::ostream&, std::map, std::allocator >, std::__cxx11::basic_string, std::allocator >, std::less, std::allocator > >, std::allocator, std::allocator > const, std::__cxx11::basic_string, std::allocator > > > > const&, std::map, std::allocator >, std::pair, std::allocator >, std::__cxx11::basic_string, std::allocator > >, std::less, std::allocator > >, std::allocator, std::allocator > const, std::pair, std::allocator >, std::__cxx11::basic_string, std::allocator > > > > > const&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/crush/CrushWrapper.cc:1957:20 #2 0x55d567dfbcec in main /home/jenkins-build/build/workspace/ceph-pull-requests/src/tools/crushtool.cc:1215:19 #3 0x7f80a06c7d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 #4 0x7f80a06c7e3f in __libc_start_main csu/../csu/libc-start.c:392:3 #5 0x55d567d2b4d4 in _start (/home/jenkins-build/build/workspace/ceph-pull-requests/build/bin/crushtool+0xb54d4) (BuildId: ce3df2d268a883ca3965158085f32e534cbedaf5) 0x60c00000c000 is located 0 bytes to the right of 128-byte region [0x60c00000bf80,0x60c00000c000) allocated by thread T0 here: #0 0x55d567dae508 in __interceptor_calloc (/home/jenkins-build/build/workspace/ceph-pull-requests/build/bin/crushtool+0x138508) (BuildId: ce3df2d268a883ca3965158085f32e534cbedaf5) #1 0x7f80a4b164cf in CrushWrapper::decode(ceph::buffer::v15_2_0::list::iterator_impl&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/crush/CrushWrapper.cc:3267:38 #2 0x55d567df69eb in main /home/jenkins-build/build/workspace/ceph-pull-requests/src/tools/crushtool.cc:919:13 #3 0x7f80a06c7d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 SUMMARY: AddressSanitizer: heap-buffer-overflow /home/jenkins-build/build/workspace/ceph-pull-requests/src/crush/CrushWrapper.cc:2189:10 in CrushWrapper::get_new_bucket_id() ``` fixes: https://tracker.ceph.com/issues/66861 Signed-off-by: Rongqi Sun --- src/crush/CrushWrapper.cc | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc index e434d1a17d851..755c7d1d17d41 100644 --- a/src/crush/CrushWrapper.cc +++ b/src/crush/CrushWrapper.cc @@ -2185,25 +2185,23 @@ int CrushWrapper::reclassify( int CrushWrapper::get_new_bucket_id() { - int id = -1; - while (crush->buckets[-1-id] && - -1-id < crush->max_buckets) { - id--; - } - if (-1-id == crush->max_buckets) { - ++crush->max_buckets; - crush->buckets = (struct crush_bucket**)realloc( - crush->buckets, - sizeof(crush->buckets[0]) * crush->max_buckets); - for (auto& i : choose_args) { - assert(i.second.size == (__u32)crush->max_buckets - 1); - ++i.second.size; - i.second.args = (struct crush_choose_arg*)realloc( - i.second.args, - sizeof(i.second.args[0]) * i.second.size); + for (int index = 0; index < crush->max_buckets; index++) { + if (crush->buckets[index] == nullptr) { + return -index - 1; } } - return id; + ++crush->max_buckets; + crush->buckets = (struct crush_bucket**)realloc( + crush->buckets, + sizeof(crush->buckets[0]) * crush->max_buckets); + for (auto& i : choose_args) { + assert(i.second.size == (__u32)crush->max_buckets - 1); + ++i.second.size; + i.second.args = (struct crush_choose_arg*)realloc( + i.second.args, + sizeof(i.second.args[0]) * i.second.size); + } + return -crush->max_buckets; } void CrushWrapper::reweight(CephContext *cct) -- 2.47.3