librbd: fix rbd_metadata_list and rbd_metadata_get

- properly check for val_len in rbd_metadata_list
- don't expect output buffers are zero pre-filled

Fixes: http://tracker.ceph.com/issues/19588
Signed-off-by: Mykola Golub <mgolub@mirantis.com>
(cherry picked from commit 75afc74ea681402e22b6dec8b83276d145fc786b)

diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc
index abbc873ff5c16cf35b3c5fe6b552ef71eecbbf21..3b18628690c36c70711db206aea9db46feaf0e0e 100644 (file)
--- a/src/librbd/librbd.cc
+++ b/src/librbd/librbd.cc
@@ -2896,12 +2896,12 @@ extern "C" int rbd_metadata_get(rbd_image_t image, const char *key, char *value,
     tracepoint(librbd, metadata_get_exit, r, key, NULL);
     return r;
   }
-  if (*vallen < val_s.size()) {
+  if (*vallen < val_s.size() + 1) {
     r = -ERANGE;
-    *vallen = val_s.size();
+    *vallen = val_s.size() + 1;
     tracepoint(librbd, metadata_get_exit, r, key, NULL);
   } else {
-    strncpy(value, val_s.c_str(), val_s.size());
+    strncpy(value, val_s.c_str(), val_s.size() + 1);
     tracepoint(librbd, metadata_get_exit, r, key, value);
   }
   return r;
@@ -2939,7 +2939,7 @@ extern "C" int rbd_metadata_list(rbd_image_t image, const char *start, uint64_t
     key_total_len += it->first.size() + 1;
     val_total_len += it->second.length() + 1;
   }
-  if (*key_len < key_total_len || *val_len < key_total_len)
+  if (*key_len < key_total_len || *val_len < val_total_len)
     too_short = true;
   *key_len = key_total_len;
   *val_len = val_total_len;
@@ -2952,10 +2952,12 @@ extern "C" int rbd_metadata_list(rbd_image_t image, const char *start, uint64_t
 
   for (map<string, bufferlist>::iterator it = pairs.begin();
        it != pairs.end(); ++it) {
-    strncpy(key_p, it->first.c_str(), it->first.size());
+    strncpy(key_p, it->first.c_str(), it->first.size() + 1);
     key_p += it->first.size() + 1;
     strncpy(value_p, it->second.c_str(), it->second.length());
-    value_p += it->second.length() + 1;
+    value_p += it->second.length();
+    *value_p = '\0';
+    value_p++;
     tracepoint(librbd, metadata_list_entry, it->first.c_str(), it->second.c_str());
   }
   tracepoint(librbd, metadata_list_exit, r);

diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc
index 3f32df1dbe616af4c7e87519604574dceb941065..7344b76e5b1cb24fe563d2ac1af8b7207016e246 100644 (file)
--- a/src/test/librbd/test_librbd.cc
+++ b/src/test/librbd/test_librbd.cc
@@ -3905,10 +3905,208 @@ TEST_F(TestLibRBD, ObjectMapConsistentSnap)
   ASSERT_PASSED(validate_object_map, image1);
 }
 
+void memset_rand(char *buf, size_t len) {
+  for (size_t i = 0; i < len; ++i) {
+    buf[i] = (char) (rand() % (126 - 33) + 33);
+  }
+}
+
 TEST_F(TestLibRBD, Metadata)
 {
   REQUIRE_FEATURE(RBD_FEATURE_LAYERING);
 
+  rados_ioctx_t ioctx;
+  rados_ioctx_create(_cluster, m_pool_name.c_str(), &ioctx);
+
+  std::string name = get_temp_image_name();
+  uint64_t size = 2 << 20;
+  int order = 0;
+  ASSERT_EQ(0, create_image(ioctx, name.c_str(), size, &order));
+
+  rbd_image_t image;
+  ASSERT_EQ(0, rbd_open(ioctx, name.c_str(), &image, NULL));
+
+  rbd_image_t image1;
+  ASSERT_EQ(0, rbd_open(ioctx, name.c_str(), &image1, NULL));
+
+  char keys[1024];
+  char vals[1024];
+  size_t keys_len = sizeof(keys);
+  size_t vals_len = sizeof(vals);
+
+  memset_rand(keys, keys_len);
+  memset_rand(vals, vals_len);
+
+  ASSERT_EQ(0, rbd_metadata_list(image, "", 0, keys, &keys_len, vals,
+                                 &vals_len));
+  ASSERT_EQ(0U, keys_len);
+  ASSERT_EQ(0U, vals_len);
+
+  char value[1024];
+  size_t value_len = sizeof(value);
+  memset_rand(value, value_len);
+
+  ASSERT_EQ(0, rbd_metadata_set(image1, "key1", "value1"));
+  ASSERT_EQ(0, rbd_metadata_set(image1, "key2", "value2"));
+  ASSERT_EQ(0, rbd_metadata_get(image1, "key1", value, &value_len));
+  ASSERT_STREQ(value, "value1");
+  value_len = 1;
+  ASSERT_EQ(-ERANGE, rbd_metadata_get(image1, "key1", value, &value_len));
+  ASSERT_EQ(value_len, strlen("value1") + 1);
+
+  ASSERT_EQ(-ERANGE, rbd_metadata_list(image1, "", 0, keys, &keys_len, vals,
+                                       &vals_len));
+  keys_len = sizeof(keys);
+  vals_len = sizeof(vals);
+  memset_rand(keys, keys_len);
+  memset_rand(vals, vals_len);
+  ASSERT_EQ(0, rbd_metadata_list(image1, "", 0, keys, &keys_len, vals,
+                                 &vals_len));
+  ASSERT_EQ(keys_len, strlen("key1") + 1 + strlen("key2") + 1);
+  ASSERT_EQ(vals_len, strlen("value1") + 1 + strlen("value2") + 1);
+  ASSERT_STREQ(keys, "key1");
+  ASSERT_STREQ(keys + strlen(keys) + 1, "key2");
+  ASSERT_STREQ(vals, "value1");
+  ASSERT_STREQ(vals + strlen(vals) + 1, "value2");
+
+  ASSERT_EQ(0, rbd_metadata_remove(image1, "key1"));
+  ASSERT_EQ(0, rbd_metadata_remove(image1, "key3"));
+  value_len = sizeof(value);
+  ASSERT_EQ(-ENOENT, rbd_metadata_get(image1, "key3", value, &value_len));
+  ASSERT_EQ(0, rbd_metadata_list(image1, "", 0, keys, &keys_len, vals,
+                                 &vals_len));
+  ASSERT_EQ(keys_len, strlen("key2") + 1);
+  ASSERT_EQ(vals_len, strlen("value2") + 1);
+  ASSERT_STREQ(keys, "key2");
+  ASSERT_STREQ(vals, "value2");
+
+  // test config setting
+  ASSERT_EQ(0, rbd_metadata_set(image1, "conf_rbd_cache", "false"));
+  ASSERT_EQ(-EINVAL, rbd_metadata_set(image1, "conf_rbd_cache", "INVALID_VAL"));
+  ASSERT_EQ(0, rbd_metadata_remove(image1, "conf_rbd_cache"));
+
+  // test metadata with snapshot adding
+  ASSERT_EQ(0, rbd_snap_create(image1, "snap1"));
+  ASSERT_EQ(0, rbd_snap_protect(image1, "snap1"));
+  ASSERT_EQ(0, rbd_snap_set(image1, "snap1"));
+
+  ASSERT_EQ(0, rbd_metadata_set(image1, "key1", "value1"));
+  ASSERT_EQ(0, rbd_metadata_set(image1, "key3", "value3"));
+
+  keys_len = sizeof(keys);
+  vals_len = sizeof(vals);
+  memset_rand(keys, keys_len);
+  memset_rand(vals, vals_len);
+  ASSERT_EQ(0, rbd_metadata_list(image1, "", 0, keys, &keys_len, vals,
+                                 &vals_len));
+  ASSERT_EQ(keys_len,
+            strlen("key1") + 1 + strlen("key2") + 1 + strlen("key3") + 1);
+  ASSERT_EQ(vals_len,
+            strlen("value1") + 1 + strlen("value2") + 1 + strlen("value3") + 1);
+  ASSERT_STREQ(keys, "key1");
+  ASSERT_STREQ(keys + strlen("key1") + 1, "key2");
+  ASSERT_STREQ(keys + strlen("key1") + 1 + strlen("key2") + 1, "key3");
+  ASSERT_STREQ(vals, "value1");
+  ASSERT_STREQ(vals + strlen("value1") + 1, "value2");
+  ASSERT_STREQ(vals + strlen("value1") + 1 + strlen("value2") + 1, "value3");
+
+  ASSERT_EQ(0, rbd_snap_set(image1, NULL));
+  keys_len = sizeof(keys);
+  vals_len = sizeof(vals);
+  memset_rand(keys, keys_len);
+  memset_rand(vals, vals_len);
+  ASSERT_EQ(0, rbd_metadata_list(image1, "", 0, keys, &keys_len, vals,
+                                 &vals_len));
+  ASSERT_EQ(keys_len,
+            strlen("key1") + 1 + strlen("key2") + 1 + strlen("key3") + 1);
+  ASSERT_EQ(vals_len,
+            strlen("value1") + 1 + strlen("value2") + 1 + strlen("value3") + 1);
+  ASSERT_STREQ(keys, "key1");
+  ASSERT_STREQ(keys + strlen("key1") + 1, "key2");
+  ASSERT_STREQ(keys + strlen("key1") + 1 + strlen("key2") + 1, "key3");
+  ASSERT_STREQ(vals, "value1");
+  ASSERT_STREQ(vals + strlen("value1") + 1, "value2");
+  ASSERT_STREQ(vals + strlen("value1") + 1 + strlen("value2") + 1, "value3");
+
+  // test metadata with cloning
+  uint64_t features;
+  ASSERT_EQ(0, rbd_get_features(image1, &features));
+
+  string cname = get_temp_image_name();
+  EXPECT_EQ(0, rbd_clone(ioctx, name.c_str(), "snap1", ioctx,
+                         cname.c_str(), features, &order));
+  rbd_image_t image2;
+  ASSERT_EQ(0, rbd_open(ioctx, cname.c_str(), &image2, NULL));
+  ASSERT_EQ(0, rbd_metadata_set(image2, "key4", "value4"));
+
+  keys_len = sizeof(keys);
+  vals_len = sizeof(vals);
+  memset_rand(keys, keys_len);
+  memset_rand(vals, vals_len);
+  ASSERT_EQ(0, rbd_metadata_list(image2, "", 0, keys, &keys_len, vals,
+                                 &vals_len));
+  ASSERT_EQ(keys_len, strlen("key1") + 1 + strlen("key2") + 1 + strlen("key3") +
+            1 + strlen("key4") + 1);
+  ASSERT_EQ(vals_len, strlen("value1") + 1 + strlen("value2") + 1 +
+            strlen("value3") + 1 + strlen("value4") + 1);
+  ASSERT_STREQ(keys + strlen("key1") + 1 + strlen("key2") + 1 + strlen("key3") +
+               1, "key4");
+  ASSERT_STREQ(vals + strlen("value1") + 1 + strlen("value2") + 1 +
+               strlen("value3") + 1, "value4");
+
+  ASSERT_EQ(0, rbd_metadata_list(image1, "", 0, keys, &keys_len, vals,
+                                 &vals_len));
+  ASSERT_EQ(keys_len,
+            strlen("key1") + 1 + strlen("key2") + 1 + strlen("key3") + 1);
+  ASSERT_EQ(vals_len,
+            strlen("value1") + 1 + strlen("value2") + 1 + strlen("value3") + 1);
+  ASSERT_EQ(-ENOENT, rbd_metadata_get(image1, "key4", value, &value_len));
+
+  // test short buffer cases
+  keys_len = strlen("key1") + 1;
+  vals_len = strlen("value1") + 1;
+  memset_rand(keys, keys_len);
+  memset_rand(vals, vals_len);
+  ASSERT_EQ(0, rbd_metadata_list(image2, "", 1, keys, &keys_len, vals,
+                                 &vals_len));
+  ASSERT_EQ(keys_len, strlen("key1") + 1);
+  ASSERT_EQ(vals_len, strlen("value1") + 1);
+  ASSERT_STREQ(keys, "key1");
+  ASSERT_STREQ(vals, "value1");
+
+  ASSERT_EQ(-ERANGE, rbd_metadata_list(image2, "", 2, keys, &keys_len, vals,
+                                       &vals_len));
+  ASSERT_EQ(keys_len, strlen("key1") + 1 + strlen("key2") + 1);
+  ASSERT_EQ(vals_len, strlen("value1") + 1 + strlen("value2") + 1);
+
+  ASSERT_EQ(-ERANGE, rbd_metadata_list(image2, "", 0, keys, &keys_len, vals,
+                                       &vals_len));
+  ASSERT_EQ(keys_len, strlen("key1") + 1 + strlen("key2") + 1 + strlen("key3") +
+            1 + strlen("key4") + 1);
+  ASSERT_EQ(vals_len, strlen("value1") + 1 + strlen("value2") + 1 +
+            strlen("value3") + 1 + strlen("value4") + 1);
+
+  // test `start` param
+  keys_len = sizeof(keys);
+  vals_len = sizeof(vals);
+  memset_rand(keys, keys_len);
+  memset_rand(vals, vals_len);
+  ASSERT_EQ(0, rbd_metadata_list(image2, "key2", 0, keys, &keys_len, vals,
+                                 &vals_len));
+  ASSERT_EQ(keys_len, strlen("key3") + 1 + strlen("key4") + 1);
+  ASSERT_EQ(vals_len, strlen("value3") + 1 + strlen("value4") + 1);
+  ASSERT_STREQ(keys, "key3");
+  ASSERT_STREQ(vals, "value3");
+
+  ASSERT_EQ(0, rbd_close(image));
+  ASSERT_EQ(0, rbd_close(image1));
+  ASSERT_EQ(0, rbd_close(image2));
+}
+
+TEST_F(TestLibRBD, MetadataPP)
+{
+  REQUIRE_FEATURE(RBD_FEATURE_LAYERING);
+
   librados::IoCtx ioctx;
   ASSERT_EQ(0, _rados.ioctx_create(m_pool_name.c_str(), ioctx));