From: Sage Weil <sage@redhat.com>
Date: Fri, 8 Mar 2019 22:54:53 +0000 (-0600)
Subject: crush/CrushWrapper: ensure crush_choose_arg_map.size == max_buckets
X-Git-Tag: v12.2.13~235^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=b8b1cb903e043461a7d044b04fdaf3409efcf21f;p=ceph.git

crush/CrushWrapper: ensure crush_choose_arg_map.size == max_buckets

The crush/builder.c crush_add_bucket method resizes the max_buckets array
but a power of 2 when it has to expand, but the code in CrushWrapper was
assuming that if the array grew the pos for the new bucket would be the
last position in the new array.  This led to a situation where the
crush_choose_arg_map args array size didn't match max_buckets, and
eventually caused a crash.

Fixes: http://tracker.ceph.com/issues/38664
Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit ccda488815eeca608a58092d0d917c8a68a7b93c)

Conflicts:
	src/crush/CrushWrapper.cc : Resolved in add_bucket and
					device_class_clone
- qa/workunits/mon/crush_ops.sh: luminous add-bucket doesn't have the
optional location arg
---

diff --git a/qa/workunits/mon/crush_ops.sh b/qa/workunits/mon/crush_ops.sh
index 2ae8202301e..4e1cbb6e767 100755
--- a/qa/workunits/mon/crush_ops.sh
+++ b/qa/workunits/mon/crush_ops.sh
@@ -213,4 +213,18 @@ ceph osd crush rm fooo
 ceph osd crush rm barr
 ceph osd crush weight-set rm-compat
 
+# this sequence would crash at one point
+ceph osd crush weight-set create-compat
+ceph osd crush add-bucket r1 rack
+ceph osd crush move r1 root=default
+for f in `seq 1 32`; do
+    ceph osd crush add-bucket h$f host
+    ceph osd crush move h$f rack=r1
+done
+for f in `seq 1 32`; do
+    ceph osd crush rm h$f
+done
+ceph osd crush rm r1
+ceph osd crush weight-set rm-compat
+
 echo OK
diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index d3c2a7af48c..bb12510cfb2 100644
--- a/src/crush/CrushWrapper.cc
+++ b/src/crush/CrushWrapper.cc
@@ -408,9 +408,11 @@ void CrushWrapper::update_choose_args(CephContext *cct)
 {
   for (auto& i : choose_args) {
     crush_choose_arg_map &arg_map = i.second;
+    assert(arg_map.size == (unsigned)crush->max_buckets);
     unsigned positions = get_choose_args_positions(arg_map);
     for (int j = 0; j < crush->max_buckets; ++j) {
       crush_bucket *b = crush->buckets[j];
+      assert(j < (int)arg_map.size);
       auto& carg = arg_map.args[j];
       // strip out choose_args for any buckets that no longer exist
       if (!b || b->alg != CRUSH_BUCKET_STRAW2) {
@@ -2344,21 +2346,22 @@ int CrushWrapper::add_bucket(
   int pos = -1 - *idout;
   for (auto& p : choose_args) {
     crush_choose_arg_map& cmap = p.second;
+    unsigned new_size = crush->max_buckets;
     if (cmap.args) {
-      if ((int)cmap.size <= pos) {
+      if ((int)cmap.size < crush->max_buckets) {
 	cmap.args = (crush_choose_arg*)realloc(
 	  cmap.args,
-	  sizeof(crush_choose_arg) * (pos + 1));
+	  sizeof(crush_choose_arg) * new_size);
         assert(cmap.args);
 	memset(&cmap.args[cmap.size], 0,
-	       sizeof(crush_choose_arg) * (pos + 1 - cmap.size));
-	cmap.size = pos + 1;
+	       sizeof(crush_choose_arg) * (new_size - cmap.size));
+	cmap.size = new_size;
       }
     } else {
       cmap.args = (crush_choose_arg*)calloc(sizeof(crush_choose_arg),
-					    pos + 1);
+							new_size);
       assert(cmap.args);
-      cmap.size = pos + 1;
+      cmap.size = new_size;
     }
     if (size > 0) {
       int positions = get_choose_args_positions(cmap);
@@ -2374,6 +2377,7 @@ int CrushWrapper::add_bucket(
 	}
       }
     }
+    assert(crush->max_buckets == (int)cmap.size);
   }
   return r;
 }
@@ -2607,8 +2611,8 @@ int CrushWrapper::device_class_clone(
   // set up choose_args for the new bucket.
   for (auto& w : choose_args) {
     crush_choose_arg_map& cmap = w.second;
-    if (-1-bno >= (int)cmap.size) {
-      unsigned new_size = -1-bno + 1;
+    if (crush->max_buckets > (int)cmap.size) {
+      unsigned new_size = crush->max_buckets;
       cmap.args = (crush_choose_arg*)realloc(cmap.args,
 					     new_size * sizeof(cmap.args[0]));
       assert(cmap.args);