osd: Fix segfault in EC debug string

The old debug_string implementation was potentially reading up to 3
bytes off the end of an array. It was also doing lots of unnecessary
bufferlist reconstructs. This refactor of this function fixes both
issues.

Signed-off-by: Alex Ainscow <aainscow@uk.ibm.com>
(cherry picked from commit da3ccdf4d03e40b747f8876449199102e53e00ce)

diff --git a/src/osd/ECUtil.cc b/src/osd/ECUtil.cc
index cd25c3305fe40eaa1e006d3174a6d3e0c2cff1c4..a9f534761943cd2fe8a847cb7b14d26a942647f8 100644 (file)
--- a/src/osd/ECUtil.cc
+++ b/src/osd/ECUtil.cc
@@ -987,25 +987,47 @@ bufferlist shard_extent_map_t::get_ro_buffer() const {
   return get_ro_buffer(ro_start, ro_end - ro_start);
 }
 
+bool get_int_from_bufferlist(bufferlist bl, int offset, uint32_t *value) {
+  *value = 0;
+
+  if (bl.length() < offset + sizeof(uint32_t)) {
+    return false;
+  }
+
+  auto bl_iter = bl.begin();
+  bl_iter += offset;
+  unsigned b = 0;
+  for (; b < sizeof(uint32_t); ++b, ++bl_iter) {
+    ceph_assert(bl_iter != bl.end());
+    *value |= (((unsigned int)bl_iter.get_current_ptr().c_str()[0]) << b);
+  }
+  return true;
+}
+
 std::string shard_extent_map_t::debug_string(uint64_t interval, uint64_t offset) const {
   std::stringstream str;
   str << "shard_extent_map_t: " << *this << " bufs: [";
 
   bool s_comma = false;
   for (auto &&[shard, emap] : get_extent_maps()) {
-    if (s_comma) str << ", ";
+    if (s_comma) {
+      str << ", ";
+    }
     s_comma = true;
     str << shard << ": [";
 
     bool comma = false;
     for (auto &&extent : emap) {
       bufferlist bl = extent.get_val();
-      char *buf = bl.c_str();
-      for (uint64_t i = 0; i < extent.get_len(); i += interval) {
-        int *seed = (int*)&buf[i + offset];
-        if (comma) str << ", ";
-        str << (i + extent.get_off()) << ":" << std::to_string(*seed);
+      uint32_t seed;
+      int i = 0;
+      while (get_int_from_bufferlist(bl, i + offset, &seed)) {
+        if (comma) {
+          str << ", ";
+        }
+        str << (i + extent.get_off()) << ":" << seed;
         comma = true;
+        i += interval;
       }
     }
     str << "]";

diff --git a/src/test/osd/TestECBackend.cc b/src/test/osd/TestECBackend.cc
index 62d70b64d97c091e08772a7ed690458e3d9bfda4..2ee2ea8ca8d4961575824b2c92e136bc4abe9696 100644 (file)
--- a/src/test/osd/TestECBackend.cc
+++ b/src/test/osd/TestECBackend.cc
@@ -297,6 +297,9 @@ public:
 };
 
 class ECListenerStub : public ECListener {
+
+
+private:
   OSDMapRef osd_map_ref;
   pg_info_t pg_info;
   set<pg_shard_t> backfill_shards;

diff --git a/src/test/osd/TestECUtil.cc b/src/test/osd/TestECUtil.cc
index c9fa63b8a6b46af2ae6ef839fff567b2f2020d61..14b2409380e8020ba701d689f569837dccc97ee9 100644 (file)
--- a/src/test/osd/TestECUtil.cc
+++ b/src/test/osd/TestECUtil.cc
@@ -1080,4 +1080,24 @@ TEST(ECUtil, insert_parity_buffer_into_sem) {
     ASSERT_EQ(4096, sem.ro_start);
     ASSERT_EQ(8192, sem.ro_end);
   }
+}
+
+// Debug String test, to track down seg-fault found by teuthology.
+TEST(ECUtil, debug_string)
+{
+  int k=3;
+  int m=2;
+  int chunk_size = 4096;
+
+  stripe_info_t sinfo(k, m, chunk_size*k, vector<shard_id_t>(0));
+  shard_extent_map_t semap(&sinfo);
+
+  bufferlist bl0, bl1;
+  bl0.append_zero(750);
+  bl1.append_zero(3516);
+
+  semap.insert_in_shard(shard_id_t(0), 352256, bl0);
+  semap.insert_in_shard(shard_id_t(0), 348740, bl1);
+
+  semap.debug_string(2048, 0);
 }
\ No newline at end of file