]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph-ci.git/commitdiff
common/bl: fix memory corruption in bufferlist::claim_append()
authorRadoslaw Zarzynski <rzarzyns@redhat.com>
Fri, 24 Jan 2020 14:43:37 +0000 (15:43 +0100)
committerRadoslaw Zarzynski <rzarzyns@redhat.com>
Fri, 24 Jan 2020 14:50:27 +0000 (15:50 +0100)
Fixes: https://tracker.ceph.com/issues/43814
Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
src/common/buffer.cc
src/test/bufferlist.cc

index 01a3dc82f0d8a41676a918161b1c9b2999b1ff5c..71709137d72839bf42898ae0af58d4e6d87c7d3e 100644 (file)
@@ -1309,7 +1309,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++;
        }
index 43cf565233d97f4b77ec763976f1472b6332e1d6..e5f1626f60049229961f42e6df6d7813f4bde7e4 100644 (file)
@@ -2900,6 +2900,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;