Skip to content

Commit

Permalink
common/bl: fix the dangling last_p issue.
Browse files Browse the repository at this point in the history
Fixes: https://tracker.ceph.com/issues/43646
Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
  • Loading branch information
rzarzynski committed Jan 17, 2020
1 parent af0e482 commit 8198332
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/include/buffer.h
Expand Up @@ -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;
}
Expand Down
29 changes: 29 additions & 0 deletions src/test/bufferlist.cc
Expand Up @@ -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;
Expand Down

0 comments on commit 8198332

Please sign in to comment.