Skip to content

osd/scrub: fix heap-buffer-overflow when checking digest emptiness#65193

Merged
tchaikov merged 1 commit intoceph:mainfrom
tchaikov:wip-osd-scrub-fix-buffer-overflow
Sep 7, 2025
Merged

osd/scrub: fix heap-buffer-overflow when checking digest emptiness#65193
tchaikov merged 1 commit intoceph:mainfrom
tchaikov:wip-osd-scrub-fix-buffer-overflow

Conversation

@tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Aug 23, 2025

Instead of constructing an std::string, let's check the digest emptiness with bufferlist::empty() to avoid reading beyond buffer boundaries.

In commit 92ccbff, we introduced a bug when checking if a digest was empty by constructing a std::string from bufferlist:

std::string(digest.second.c_str()).empty()

This is unsafe because bufferlist data is not guaranteed to be null- terminated. The std::string constructor searches for a null terminator and may read beyond the bufferlist's allocated memory, causing a heap-buffer-overflow detected by AddressSanitizer:

==66092==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7e0c65215004 at pc 0x7fbc6e27c597 bp 0x7ffe29fb6100 sp 0x7ffe29fb58b8
READ of size 5 at 0x7e0c65215004 thread T0
    #0 0x7fbc6e27c596 in strlen /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:425
    #1 0x562c75fad91a in std::char_traits<char>::length(char const*) /usr/include/c++/15.2.1/bits/char_traits.h:393
    #2 0x562c75fb4222 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string<std::allocator<char> >(char const*, std::allocator<char> const&) /usr/include/c++/15.2.1/bits/b
asic_string.h:713
    #3 0x562c761b81ae in operator() /home/kefu/dev/ceph/src/osd/scrubber/scrub_backend.cc:1300
    #4 0x562c761d7d53 in operator()<mini_flat_map<shard_id_t, ceph::buffer::v15_2_0::list, signed char>::_iterator<false> > /usr/include/c++/15.2.1/bits/predefined_ops.h:318
    #5 0x562c761d789c in __find_if<mini_flat_map<shard_id_t, ceph::buffer::v15_2_0::list, signed char>::_iterator<false>, __gnu_cxx::__ops::_Iter_pred<ScrubBackend::match_in_shards(const hobject_t&, auth_selection_
t&, inconsistent_obj_wrapper&, std::stringstream&)::<lambda(const std::pair<const shard_id_t, ceph::buffer::v15_2_0::list&>&)> > > /usr/include/c++/15.2.1/bits/stl_algobase.h:2095
    #6 0x562c761d72b2 in find_if<mini_flat_map<shard_id_t, ceph::buffer::v15_2_0::list, signed char>::_iterator<false>, ScrubBackend::match_in_shards(const hobject_t&, auth_selection_t&, inconsistent_obj_wrapper&,
std::stringstream&)::<lambda(const std::pair<const shard_id_t, ceph::buffer::v15_2_0::list&>&)> > /usr/include/c++/15.2.1/bits/stl_algo.h:3921
    #7 0x562c761d5f6f in none_of<mini_flat_map<shard_id_t, ceph::buffer::v15_2_0::list, signed char>::_iterator<false>, ScrubBackend::match_in_shards(const hobject_t&, auth_selection_t&, inconsistent_obj_wrapper&,
std::stringstream&)::<lambda(const std::pair<const shard_id_t, ceph::buffer::v15_2_0::list&>&)> > /usr/include/c++/15.2.1/bits/stl_algo.h:431
    #8 0x562c761d4a50 in any_of<mini_flat_map<shard_id_t, ceph::buffer::v15_2_0::list, signed char>::_iterator<false>, ScrubBackend::match_in_shards(const hobject_t&, auth_selection_t&, inconsistent_obj_wrapper&, s
td::stringstream&)::<lambda(const std::pair<const shard_id_t, ceph::buffer::v15_2_0::list&>&)> > /usr/include/c++/15.2.1/bits/stl_algo.h:450
    #9 0x562c761bb84b in ScrubBackend::match_in_shards(hobject_t const&, auth_selection_t&, inconsistent_obj_wrapper&, std::__cxx11::basic_stringstream<char, std::char_traits<char>, std::allocator<char> >&) /home/k
efu/dev/ceph/src/osd/scrubber/scrub_backend.cc:1297
    #10 0x562c761b4282 in ScrubBackend::compare_obj_in_maps[abi:cxx11](hobject_t const&) /home/kefu/dev/ceph/src/osd/scrubber/scrub_backend.cc:941
    #11 0x562c761d44af in operator()<hobject_t> /home/kefu/dev/ceph/src/osd/scrubber/scrub_backend.cc:887
    #12 0x562c761d4836 in for_each<std::_Rb_tree_const_iterator<hobject_t>, ScrubBackend::compare_smaps()::<lambda(const auto:422&)> > /usr/include/c++/15.2.1/bits/stl_algo.h:3798
    #13 0x562c761b3259 in ScrubBackend::compare_smaps() /home/kefu/dev/ceph/src/osd/scrubber/scrub_backend.cc:884
    #14 0x562c761a478d in ScrubBackend::update_authoritative() /home/kefu/dev/ceph/src/osd/scrubber/scrub_backend.cc:315`

Fix by using bufferlist::empty() which tells if the bufferlist is empty.

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands

@tchaikov tchaikov requested a review from a team as a code owner August 23, 2025 08:58
@github-actions github-actions bot added the core label Aug 23, 2025
@tchaikov tchaikov mentioned this pull request Aug 23, 2025
14 tasks
@tchaikov tchaikov force-pushed the wip-osd-scrub-fix-buffer-overflow branch from 662f35b to b4aa7c0 Compare August 27, 2025 13:48
@tchaikov
Copy link
Contributor Author

tchaikov commented Aug 27, 2025

changelog

  • switch to bufferlist::empty()

hi @JonBailey1993 and @ronen-fr , could you help review this change at your convenience?

Copy link
Contributor

@JonBailey1993 JonBailey1993 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Replace unsafe string construction with bufferlist::length() to avoid
reading beyond buffer boundaries.

In commit 92ccbff, we introduced a bug when checking if a digest was
empty by constructing a std::string from bufferlist:

```
std::string(digest.second.c_str()).empty()
```

This is unsafe because bufferlist data is not guaranteed to be null-
terminated. The std::string constructor searches for a null terminator
and may read beyond the bufferlist's allocated memory, causing a
heap-buffer-overflow detected by AddressSanitizer:

```
==66092==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7e0c65215004 at pc 0x7fbc6e27c597 bp 0x7ffe29fb6100 sp 0x7ffe29fb58b8
READ of size 5 at 0x7e0c65215004 thread T0
    #0 0x7fbc6e27c596 in strlen /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:425
    #1 0x562c75fad91a in std::char_traits<char>::length(char const*) /usr/include/c++/15.2.1/bits/char_traits.h:393
    #2 0x562c75fb4222 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string<std::allocator<char> >(char const*, std::allocator<char> const&) /usr/include/c++/15.2.1/bits/b
asic_string.h:713
    #3 0x562c761b81ae in operator() /home/kefu/dev/ceph/src/osd/scrubber/scrub_backend.cc:1300
    #4 0x562c761d7d53 in operator()<mini_flat_map<shard_id_t, ceph::buffer::v15_2_0::list, signed char>::_iterator<false> > /usr/include/c++/15.2.1/bits/predefined_ops.h:318
    #5 0x562c761d789c in __find_if<mini_flat_map<shard_id_t, ceph::buffer::v15_2_0::list, signed char>::_iterator<false>, __gnu_cxx::__ops::_Iter_pred<ScrubBackend::match_in_shards(const hobject_t&, auth_selection_
t&, inconsistent_obj_wrapper&, std::stringstream&)::<lambda(const std::pair<const shard_id_t, ceph::buffer::v15_2_0::list&>&)> > > /usr/include/c++/15.2.1/bits/stl_algobase.h:2095
    #6 0x562c761d72b2 in find_if<mini_flat_map<shard_id_t, ceph::buffer::v15_2_0::list, signed char>::_iterator<false>, ScrubBackend::match_in_shards(const hobject_t&, auth_selection_t&, inconsistent_obj_wrapper&,
std::stringstream&)::<lambda(const std::pair<const shard_id_t, ceph::buffer::v15_2_0::list&>&)> > /usr/include/c++/15.2.1/bits/stl_algo.h:3921
    #7 0x562c761d5f6f in none_of<mini_flat_map<shard_id_t, ceph::buffer::v15_2_0::list, signed char>::_iterator<false>, ScrubBackend::match_in_shards(const hobject_t&, auth_selection_t&, inconsistent_obj_wrapper&,
std::stringstream&)::<lambda(const std::pair<const shard_id_t, ceph::buffer::v15_2_0::list&>&)> > /usr/include/c++/15.2.1/bits/stl_algo.h:431
    #8 0x562c761d4a50 in any_of<mini_flat_map<shard_id_t, ceph::buffer::v15_2_0::list, signed char>::_iterator<false>, ScrubBackend::match_in_shards(const hobject_t&, auth_selection_t&, inconsistent_obj_wrapper&, s
td::stringstream&)::<lambda(const std::pair<const shard_id_t, ceph::buffer::v15_2_0::list&>&)> > /usr/include/c++/15.2.1/bits/stl_algo.h:450
    #9 0x562c761bb84b in ScrubBackend::match_in_shards(hobject_t const&, auth_selection_t&, inconsistent_obj_wrapper&, std::__cxx11::basic_stringstream<char, std::char_traits<char>, std::allocator<char> >&) /home/k
efu/dev/ceph/src/osd/scrubber/scrub_backend.cc:1297
    #10 0x562c761b4282 in ScrubBackend::compare_obj_in_maps[abi:cxx11](hobject_t const&) /home/kefu/dev/ceph/src/osd/scrubber/scrub_backend.cc:941
    #11 0x562c761d44af in operator()<hobject_t> /home/kefu/dev/ceph/src/osd/scrubber/scrub_backend.cc:887
    #12 0x562c761d4836 in for_each<std::_Rb_tree_const_iterator<hobject_t>, ScrubBackend::compare_smaps()::<lambda(const auto:422&)> > /usr/include/c++/15.2.1/bits/stl_algo.h:3798
    ceph#13 0x562c761b3259 in ScrubBackend::compare_smaps() /home/kefu/dev/ceph/src/osd/scrubber/scrub_backend.cc:884
    ceph#14 0x562c761a478d in ScrubBackend::update_authoritative() /home/kefu/dev/ceph/src/osd/scrubber/scrub_backend.cc:315`
```

Fix by using bufferlist::length() which tells if the given buffer is
empty instead of converting the buffer content to a string.

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
@tchaikov tchaikov force-pushed the wip-osd-scrub-fix-buffer-overflow branch from b4aa7c0 to 100c20b Compare August 29, 2025 02:02
@tchaikov
Copy link
Contributor Author

tchaikov commented Sep 2, 2025

jenkins test api

2 similar comments
@tchaikov
Copy link
Contributor Author

tchaikov commented Sep 2, 2025

jenkins test api

@tchaikov
Copy link
Contributor Author

tchaikov commented Sep 2, 2025

jenkins test api

@ronen-fr
Copy link
Contributor

ronen-fr commented Sep 2, 2025

@tchaikov - API tests are failing for everyone since (I think) yesterday

@tchaikov
Copy link
Contributor Author

tchaikov commented Sep 3, 2025

@tchaikov - API tests are failing for everyone since (I think) yesterday

@ronen-fr thank you! actually, i noticed this failure since last weekend. but was not sure if i was alone before seeing the message on slack. just wanted to try my luck =)

@tchaikov
Copy link
Contributor Author

tchaikov commented Sep 4, 2025

jenkins test api

1 similar comment
@tchaikov
Copy link
Contributor Author

tchaikov commented Sep 6, 2025

jenkins test api

@tchaikov tchaikov merged commit 990dba2 into ceph:main Sep 7, 2025
13 checks passed
@tchaikov tchaikov deleted the wip-osd-scrub-fix-buffer-overflow branch September 7, 2025 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants