From: Alex Ainscow Date: Fri, 8 Aug 2025 09:25:53 +0000 (+0100) Subject: osd: Fix segfault in EC debug string X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=2026d8d0ed76cd6a44251488087f99ec4490f526;p=ceph.git 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 (cherry picked from commit da3ccdf4d03e40b747f8876449199102e53e00ce) --- diff --git a/src/osd/ECUtil.cc b/src/osd/ECUtil.cc index cd25c3305fe40..a9f534761943c 100644 --- 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 62d70b64d97c0..2ee2ea8ca8d49 100644 --- 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 backfill_shards; diff --git a/src/test/osd/TestECUtil.cc b/src/test/osd/TestECUtil.cc index c9fa63b8a6b46..14b2409380e80 100644 --- 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(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