common/bl: fix memory corruption in bufferlist::claim_append()

Fixes: https://tracker.ceph.com/issues/43814
Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
(cherry picked from commit d50980a99e3ddb0c3cccd4b385e455bddb65f9e7)

diff --git a/src/common/buffer.cc b/src/common/buffer.cc
index 02a6f6624085d9df50507eb629a0ea29bc649b54..7ff943953056fbc779221526ee7405c03df68672 100644 (file)
--- a/src/common/buffer.cc
+++ b/src/common/buffer.cc
@@ -1344,7 +1344,8 @@ static ceph::spinlock debug_lock;
        if (unlikely(raw && !raw->is_shareable())) {
          auto* clone = ptr_node::copy_hypercombined(*curbuf);
          curbuf = bl._buffers.erase_after_and_dispose(curbuf_prev);
-         bl._buffers.insert_after(curbuf_prev++, *clone);
+         bl._buffers.insert_after(curbuf_prev, *clone);
+         ++curbuf_prev;
        } else {
          curbuf_prev = curbuf++;
        }

diff --git a/src/test/bufferlist.cc b/src/test/bufferlist.cc
index b07872731c9d57f6f5c6cc6fc2beeff885a5dbd5..ed8d6d10c5084728bff218f17c50ac7b750f8a0b 100644 (file)
--- a/src/test/bufferlist.cc
+++ b/src/test/bufferlist.cc
@@ -2934,6 +2934,28 @@ TEST(BufferList, DanglingLastP) {
   EXPECT_EQ(0, ::memcmp("12C", bl.c_str(), 3));
 }
 
+TEST(BufferList, ClaimingTwoUnsharablePtrs) {
+  // two or more consecutive, to be precise. Otherwise the problem
+  // is nonexistent or self-healing.
+  // See: https://tracker.ceph.com/issues/43814.
+  bufferlist to_claim;
+  {
+    bufferptr one(buffer::create_unshareable(3));
+    one.copy_in(0, 3, "ABC");
+    to_claim.push_back(std::move(one));
+
+    bufferptr two(buffer::create_unshareable(3));
+    two.copy_in(0, 3, "123");
+    to_claim.push_back(std::move(two));
+  }
+  bufferlist claimer;
+  // this is supposed to not fail because of destructing wrong bptr:
+  // [ RUN      ] BufferList.ClaimingTwoUnsharablePtrs
+  // *** Error in `./bin/unittest_bufferlist': free(): invalid pointer: 0x00007ffe20f03e20 ***
+  claimer.claim_append(to_claim);
+  EXPECT_EQ(0, ::memcmp("ABC123", claimer.c_str(), 6));
+}
+
 TEST(BufferHash, all) {
   {
     bufferlist bl;