Project

General

Profile

Bug #14400

Snappy decompressor may assert when handling segmented input bufferlist

Added by Igor Fedotov about 3 years ago. Updated about 3 years ago.

Status:
Resolved
Priority:
Urgent
Assignee:
-
Category:
-
Target version:
-
Start date:
01/18/2016
Due date:
% Done:

0%

Source:
other
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:

Description

The issue may occur when the first data segment in an input bufferlist for SnappyCompressor::decompress call is less then 8 bytes.

The root cause is bufferlist::get_contiguous() method call that might invalidate bufferlist iterator initialized in BufferlistSource ctor:

class BufferlistSource : public snappy::Source {
list<bufferptr>::const_iterator pb;
...
public:
BufferlistSource(bufferlist &data): pb(data.buffers().begin()), pb_off(0), left(data.length()) {}

class SnappyCompressor : public Compressor {
...
virtual int decompress(bufferlist &src, bufferlist &dst) {
BufferlistSource source(src);
size_t res_len = 0;
// Trick, decompress only need first 32bits buffer
if (snappy::GetUncompressedLength(src.get_contiguous(0, 8), 8, &res_len))
return -1;
...

Following patch for snappy compressor UT demonstrates the issue:
diff --git a/src/test/compressor/test_compression_snappy.cc b/src/test/compressor/test_compression_snappy.cc
index 69b0ebf..6768136 100644
--- a/src/test/compressor/test_compression_snappy.cc
+++ b/src/test/compressor/test_compression_snappy.cc
@ -38,6 +38,39 @ TEST
EXPECT_EQ(res, 0);
}

TEST
{
+ const size_t small_prefix_size=3;

SnappyCompressor sp;
+ EXPECT_EQ(sp.get_method_name(), "snappy");
+ string test(128*1024,0);
+ int len = test.size();
+ bufferlist in, out;
+ in.append(test.c_str(), len);
+ int res = sp.compress(in, out);
+ EXPECT_EQ(res, 0);
+ EXPECT_GT(out.length(), 3u);
+
+ bufferlist out2, tmp;
+ const size_t small_prefix=3;
+ tmp.substr_of(out, 0, small_prefix_size );
+ out2.append( tmp );
+ size_t left = out.length()-small_prefix_size;
+ size_t offs = small_prefix;
+ while( left > 0 ){
+ size_t shard_size = MIN;
+ tmp.substr_of(out, offs, shard_size );
+ out2.append( tmp );
+ left -= shard_size;
+ offs = shard_size;
}

bufferlist after;
+ res = sp.decompress(out2, after);
+ EXPECT_EQ(res, 0);
}

int main(int argc, char *argv) {
vector<const char
> args;
argv_to_vec(argc, (const char **)argv, args);

Associated revisions

Revision 887fbe1c (diff)
Added by Igor Fedotov about 3 years ago

Fixes #14400 + some refactoring to avoid input buffer modification in future.

Signed-off-by: Igor Fedotov <>

History

#1 Updated by Igor Fedotov about 3 years ago

Here is a backtrace for the assert:

common/buffer.cc: In function 'const char* ceph::buffer::ptr::c_str() const' thread 7fe5bf6a8bc0 time 2016-01-18 21:00:17.147693
common/buffer.cc: 792: FAILED assert(raw)
ceph version 10.0.2-900-g3286106 (328610631d4cfff6f9b4f18b4c0e25dec91eb059)
1: (ceph::
_ceph_assert_fail(char const*, char const*, int, char const*)+0x82) [0x525ce2]
2: ./unittest_compression_snappy() [0x527666]
3: (BufferlistSource::Peek(unsigned long*)+0x26) [0x4e52c6]
4: (snappy::RawUncompress(snappy::Source*, char*)+0x59) [0x7fe5bf2b0139]
5: (SnappyCompressor_sharded_input_decompress_Test::TestBody()+0x78a) [0x4e4b5a]
6: (void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*)+0x65) [0x50f09c]
7: (void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*)+0x5a) [0x509d69]
8: (testing::Test::Run()+0xd5) [0x4eeea9]
9: (testing::TestInfo::Run()+0x105) [0x4ef747]
10: (testing::TestCase::Run()+0xf4) [0x4efe50]
11: (testing::internal::UnitTestImpl::RunAllTests()+0x2ac) [0x4f6d22]
12: (bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*)+0x65) [0x510667]
13: (bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*)+0x5a) [0x50ac01]
14: (testing::UnitTest::Run()+0xc6) [0x4f57e4]
15: (main()+0xa4) [0x4e2bf4]
16: (__libc_start_main()+0xf5) [0x7fe5bd4aaec5]
17: ./unittest_compression_snappy() [0x4e3987]

#3 Updated by Samuel Just about 3 years ago

  • Priority changed from Normal to Urgent

#4 Updated by Kefu Chai about 3 years ago

  • Status changed from New to Resolved

Also available in: Atom PDF