From: Radoslaw Zarzynski <rzarzyns@redhat.com>
Date: Thu, 16 Jan 2020 12:17:41 +0000 (+0100)
Subject: common/bl: fix the dangling last_p issue.
X-Git-Tag: v14.2.8~8^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=bda388c19a0a3c22b11e48ba4ff52ecb977bfd48;p=ceph.git

common/bl: fix the dangling last_p issue.

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

Conflicts:
	src/test/bufferlist.cc
---

diff --git a/src/include/buffer.h b/src/include/buffer.h
index b8c78210eae0..774ca052268c 100644
--- a/src/include/buffer.h
+++ b/src/include/buffer.h
@@ -1002,6 +1002,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 8a5bc65d31da..b07872731c9d 100644
--- a/src/test/bufferlist.cc
+++ b/src/test/bufferlist.cc
@@ -2905,6 +2905,35 @@ TEST(BufferList, TestSHA1) {
   }
 }
 
+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;