common/bl: fix the dangling last_p issue.

Fixes: https://tracker.ceph.com/issues/43646
Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>

diff --git a/src/include/buffer.h b/src/include/buffer.h
index 98358e450f647e5e3192947c10b55d22174a446b..dcf6602445b82301de5be86027fbfe25bbeb4986 100644 (file)
--- a/src/include/buffer.h
+++ b/src/include/buffer.h
@@ -990,6 +990,7 @@ inline namespace v14_2_0 {
         _carriage = &always_empty_bptr;
         _buffers.clone_from(other._buffers);
         _len = other._len;
+        last_p = begin();
       }
       return *this;
     }

diff --git a/src/test/bufferlist.cc b/src/test/bufferlist.cc
index a7ce87aefb37055f857b54b3122fbbae62100f4d..43cf565233d97f4b77ec763976f1472b6332e1d6 100644 (file)
--- a/src/test/bufferlist.cc
+++ b/src/test/bufferlist.cc
@@ -2871,6 +2871,35 @@ TEST(BufferList, TestIsProvidedBuffer) {
   ASSERT_FALSE(bl.is_provided_buffer(buff));
 }
 
+TEST(BufferList, DanglingLastP) {
+  bufferlist bl;
+  {
+    // going with the unsharable buffer type to distinguish this problem
+    // from the generic crosstalk issue we had since the very beginning:
+    // https://gist.github.com/rzarzynski/aed18372e88aed392101adac3bd87bbc
+    bufferptr bp(buffer::create_unshareable(10));
+    bp.copy_in(0, 3, "XXX");
+    bl.push_back(std::move(bp));
+    EXPECT_EQ(0, ::memcmp("XXX", bl.c_str(), 3));
+
+    // let `copy_in` to set `last_p` member of bufferlist
+    bl.copy_in(0, 2, "AB");
+    EXPECT_EQ(0, ::memcmp("ABX", bl.c_str(), 3));
+  }
+
+  bufferlist empty;
+  // before the fix this would have left `last_p` unchanged leading to
+  // the dangerous dangling state – keep in mind that the initial,
+  // unsharable bptr will be freed.
+  bl = const_cast<const bufferlist&>(empty);
+  bl.append("123");
+
+  // we must continue from where the previous copy_in had finished.
+  // Otherwise `bl::copy_in` will call `seek()` and refresh `last_p`.
+  bl.copy_in(2, 1, "C");
+  EXPECT_EQ(0, ::memcmp("12C", bl.c_str(), 3));
+}
+
 TEST(BufferHash, all) {
   {
     bufferlist bl;