Skip to content

Commit e6468f3

Browse files
committed
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 ceph#1 0x7f80a4b03f20 in CrushWrapper::reclassify(ceph::common::CephContext*, std::ostream&, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > const&, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > > const&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/crush/CrushWrapper.cc:1957:20 ceph#2 0x55d567dfbcec in main /home/jenkins-build/build/workspace/ceph-pull-requests/src/tools/crushtool.cc:1215:19 ceph#3 0x7f80a06c7d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 ceph#4 0x7f80a06c7e3f in __libc_start_main csu/../csu/libc-start.c:392:3 ceph#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) ceph#1 0x7f80a4b164cf in CrushWrapper::decode(ceph::buffer::v15_2_0::list::iterator_impl<true>&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/crush/CrushWrapper.cc:3267:38 ceph#2 0x55d567df69eb in main /home/jenkins-build/build/workspace/ceph-pull-requests/src/tools/crushtool.cc:919:13 ceph#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 <[email protected]>
1 parent dbd161d commit e6468f3

File tree

1 file changed

+15
-17
lines changed

1 file changed

+15
-17
lines changed

src/crush/CrushWrapper.cc

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2185,25 +2185,23 @@ int CrushWrapper::reclassify(
21852185

21862186
int CrushWrapper::get_new_bucket_id()
21872187
{
2188-
int id = -1;
2189-
while (crush->buckets[-1-id] &&
2190-
-1-id < crush->max_buckets) {
2191-
id--;
2192-
}
2193-
if (-1-id == crush->max_buckets) {
2194-
++crush->max_buckets;
2195-
crush->buckets = (struct crush_bucket**)realloc(
2196-
crush->buckets,
2197-
sizeof(crush->buckets[0]) * crush->max_buckets);
2198-
for (auto& i : choose_args) {
2199-
assert(i.second.size == (__u32)crush->max_buckets - 1);
2200-
++i.second.size;
2201-
i.second.args = (struct crush_choose_arg*)realloc(
2202-
i.second.args,
2203-
sizeof(i.second.args[0]) * i.second.size);
2188+
for (int index = 0; index < crush->max_buckets; index++) {
2189+
if (crush->buckets[index] == nullptr) {
2190+
return -index - 1;
22042191
}
22052192
}
2206-
return id;
2193+
++crush->max_buckets;
2194+
crush->buckets = (struct crush_bucket**)realloc(
2195+
crush->buckets,
2196+
sizeof(crush->buckets[0]) * crush->max_buckets);
2197+
for (auto& i : choose_args) {
2198+
assert(i.second.size == (__u32)crush->max_buckets - 1);
2199+
++i.second.size;
2200+
i.second.args = (struct crush_choose_arg*)realloc(
2201+
i.second.args,
2202+
sizeof(i.second.args[0]) * i.second.size);
2203+
}
2204+
return -crush->max_buckets;
22072205
}
22082206

22092207
void CrushWrapper::reweight(CephContext *cct)

0 commit comments

Comments
 (0)