From 27cbff111458e77baae617c7a222610c8ff0abee Mon Sep 17 00:00:00 2001 From: Mykola Golub Date: Tue, 11 Apr 2017 22:31:43 +0200 Subject: [PATCH] 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 (cherry picked from commit 75afc74ea681402e22b6dec8b83276d145fc786b) --- src/librbd/librbd.cc | 14 ++- src/test/librbd/test_librbd.cc | 198 +++++++++++++++++++++++++++++++++ 2 files changed, 206 insertions(+), 6 deletions(-) diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc index abbc873ff5c16..3b18628690c36 100644 --- 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::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 3f32df1dbe616..7344b76e5b1cb 100644 --- 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)); -- 2.39.5