Skip to content

Commit

Permalink
crush: avoid out-of-bound access and simplify enlarging buckets
Browse files Browse the repository at this point in the history
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::__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
      #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<true>&) /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 <[email protected]>
  • Loading branch information
Svelar committed Jul 8, 2024
1 parent dbd161d commit e6468f3
Showing 1 changed file with 15 additions and 17 deletions.
32 changes: 15 additions & 17 deletions src/crush/CrushWrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit e6468f3

Please sign in to comment.