From 3eeab975b604d9fc4290747f85a78d59a2452c2e Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Wed, 20 Feb 2019 22:30:29 +0100 Subject: [PATCH] osdc/Objecter: invalidate crcs on preallocated rx buffers Both simple and async messengers use c_str() when copying the data from the socket into the receive buffer, going behind bufferlist's back. If the receive buffer is preallocated, we need to invalidate its crc cache by hand to avoid possible data crc mismatches on the client side. Fixes: https://tracker.ceph.com/issues/38416 Signed-off-by: Ilya Dryomov --- src/osdc/Objecter.cc | 1 + src/test/librados/io_cxx.cc | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc index 841cfabf99d..1f839cabc7a 100644 --- a/src/osdc/Objecter.cc +++ b/src/osdc/Objecter.cc @@ -3255,6 +3255,7 @@ void Objecter::_send_op(Op *op) if (op->outbl && op->ontimeout == 0 && // only post rx_buffer if no timeout; see #9582 op->outbl->length()) { + op->outbl->invalidate_crc(); // messenger writes through c_str() ldout(cct, 20) << " posting rx buffer for " << op->tid << " on " << con << dendl; op->con = con; diff --git a/src/test/librados/io_cxx.cc b/src/test/librados/io_cxx.cc index 5930849100b..c5a01215fc1 100644 --- a/src/test/librados/io_cxx.cc +++ b/src/test/librados/io_cxx.cc @@ -167,6 +167,21 @@ TEST_F(LibRadosIoPP, ReadOpPP) { ASSERT_EQ(0, memcmp(read_bl1.c_str(), buf, sizeof(buf))); ASSERT_EQ(0, memcmp(read_bl2.c_str(), buf, sizeof(buf))); } + + // read into a preallocated buffer with a cached crc + { + bufferlist op_bl; + op_bl.append(std::string(sizeof(buf), 'x')); + ASSERT_NE(op_bl.crc32c(0), bl.crc32c(0)); // cache 'x' crc + + ObjectReadOperation op; + op.read(0, sizeof(buf), NULL, NULL); + ASSERT_EQ(0, ioctx.operate("foo", &op, &op_bl)); + + ASSERT_EQ(sizeof(buf), op_bl.length()); + ASSERT_EQ(0, memcmp(op_bl.c_str(), buf, sizeof(buf))); + ASSERT_EQ(op_bl.crc32c(0), bl.crc32c(0)); + } } TEST_F(LibRadosIoPP, SparseReadOpPP) { @@ -551,6 +566,21 @@ TEST_F(LibRadosIoECPP, ReadOpPP) { ASSERT_EQ(0, memcmp(read_bl1.c_str(), buf, sizeof(buf))); ASSERT_EQ(0, memcmp(read_bl2.c_str(), buf, sizeof(buf))); } + + // read into a preallocated buffer with a cached crc + { + bufferlist op_bl; + op_bl.append(std::string(sizeof(buf), 'x')); + ASSERT_NE(op_bl.crc32c(0), bl.crc32c(0)); // cache 'x' crc + + ObjectReadOperation op; + op.read(0, sizeof(buf), NULL, NULL); + ASSERT_EQ(0, ioctx.operate("foo", &op, &op_bl)); + + ASSERT_EQ(sizeof(buf), op_bl.length()); + ASSERT_EQ(0, memcmp(op_bl.c_str(), buf, sizeof(buf))); + ASSERT_EQ(op_bl.crc32c(0), bl.crc32c(0)); + } } TEST_F(LibRadosIoECPP, SparseReadOpPP) { -- 2.47.3