]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commitdiff
crush: avoid out-of-bound access and simplify enlarging buckets 57786/head
authorRongqi Sun <sunrongqi@huawei.com>
Thu, 30 May 2024 08:13:35 +0000 (08:13 +0000)
committerRongqi Sun <sunrongqi@huawei.com>
Mon, 8 Jul 2024 12:31:26 +0000 (12:31 +0000)
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 <sunrongqi@huawei.com>
src/crush/CrushWrapper.cc

index e434d1a17d8512bf512aa9e967beecb1d8f4c96a..755c7d1d17d41fd369970f666a9c8893a099206d 100644 (file)
@@ -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)