Skip to content

common/options: disable elastic shared blobs.#2

Merged
ifed01 merged 1 commit intocroit:croit_edgefrom
ifed01:wip-ifed-disable-esb-croit
Oct 30, 2025
Merged

common/options: disable elastic shared blobs.#2
ifed01 merged 1 commit intocroit:croit_edgefrom
ifed01:wip-ifed-disable-esb-croit

Conversation

@ifed01
Copy link

@ifed01 ifed01 commented Oct 30, 2025

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

You must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.

Signed-off-by: Igor Fedotov <igor.fedotov@croit.io>
@ifed01 ifed01 merged commit e882e51 into croit:croit_edge Oct 30, 2025
2 of 4 checks passed
@ifed01 ifed01 deleted the wip-ifed-disable-esb-croit branch October 30, 2025 19:30
@ifed01 ifed01 restored the wip-ifed-disable-esb-croit branch October 30, 2025 19:30
ifed01 pushed a commit that referenced this pull request Dec 6, 2025
…ives

Add suppression rules for two categories of false positive warnings
encountered during ASan-enabled testing:

1. PyModule_ExecDef memory leaks: ASan incorrectly interprets Python's
   module loading behavior as memory leaks when the interpreter loads
   extension modules.

2. __cxa_throw interception failures: ASan's interceptor cannot properly
   intercept exception handling when libstdc++.so is loaded after the
   ASan shared library, causing CHECK failures.

3. ErasureCodePluginRegistry::load:
   `ceph::ErasureCodePluginRegistry::load()` is known to leak, as we
   don't free the memory allocated by the ec plugins which are
   registered in the `ErasureCodePluginRegistry` singleton. this is a
   known issue, but since the `ErasureCodePluginRegistry` instance is a
   singleton. we can live with it. in this change, we add the rule to
   suppress the leak report from LeakSanitizer. this rule also exist in
   qa/valgrind.supp.

All warnings are confirmed false positives that should be suppressed
to reduce noise in test output.

Example warnings:

```
Direct leak of 3264 byte(s) in 1 object(s) allocated from:
    #0 0x7f6027d20cb5 in malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:67
    #1 0x7f60277557ad  (/usr/lib/libpython3.13.so.1.0+0x1557ad) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    #2 0x7f6027756067  (/usr/lib/libpython3.13.so.1.0+0x156067) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    #3 0x7f60278471a0  (/usr/lib/libpython3.13.so.1.0+0x2471a0) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    #4 0x7f602774d031  (/usr/lib/libpython3.13.so.1.0+0x14d031) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    #5 0x7b60234093bb in __Pyx_modinit_type_init_code.constprop.0 /home/kefu/dev/ceph/build/src/pybind/rados/rados.c:82066
    #6 0x7b602340a826 in __pyx_pymod_exec_rados /home/kefu/dev/ceph/build/src/pybind/rados/rados.c:82755
    #7 0x7f6027856777 in PyModule_ExecDef (/usr/lib/libpython3.13.so.1.0+0x256777) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#8 0x7f602785baa3  (/usr/lib/libpython3.13.so.1.0+0x25baa3) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#9 0x7f6027793df2  (/usr/lib/libpython3.13.so.1.0+0x193df2) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#10 0x7f6027777cbe in _PyEval_EvalFrameDefault (/usr/lib/libpython3.13.so.1.0+0x177cbe) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#11 0x7f60277957de  (/usr/lib/libpython3.13.so.1.0+0x1957de) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#12 0x7f60277d11b9 in PyObject_CallMethodObjArgs (/usr/lib/libpython3.13.so.1.0+0x1d11b9) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#13 0x7f60277d0ee4 in PyImport_ImportModuleLevelObject (/usr/lib/libpython3.13.so.1.0+0x1d0ee4) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#14 0x7f6027779c0c in _PyEval_EvalFrameDefault (/usr/lib/libpython3.13.so.1.0+0x179c0c) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#15 0x7f602784e2c8 in PyEval_EvalCode (/usr/lib/libpython3.13.so.1.0+0x24e2c8) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#16 0x7f602788c88b  (/usr/lib/libpython3.13.so.1.0+0x28c88b) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#17 0x7f602788985c  (/usr/lib/libpython3.13.so.1.0+0x28985c) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#18 0x7f6027886f57  (/usr/lib/libpython3.13.so.1.0+0x286f57) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#19 0x7f6027886211  (/usr/lib/libpython3.13.so.1.0+0x286211) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#20 0x7f6027885b82  (/usr/lib/libpython3.13.so.1.0+0x285b82) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#21 0x7f6027883e50 in Py_RunMain (/usr/lib/libpython3.13.so.1.0+0x283e50) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#22 0x7f602783bbea in Py_BytesMain (/usr/lib/libpython3.13.so.1.0+0x23bbea) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
    ceph#23 0x7f6027227674  (/usr/lib/libc.so.6+0x27674) (BuildId: 4fe011c94a88e8aeb6f2201b9eb369f42b4a1e9e)
    ceph#24 0x7f6027227728 in __libc_start_main (/usr/lib/libc.so.6+0x27728) (BuildId: 4fe011c94a88e8aeb6f2201b9eb369f42b4a1e9e)
    ceph#25 0x55dae17e6044 in _start (/usr/bin/python3.13+0x1044) (BuildId: 8c0dc848f5b978c56ebeb07255bb332b4b37ae4e)
```

```
AddressSanitizer: CHECK failed: asan_interceptors.cpp:335 "((__interception::real___cxa_throw)) != (0)" (0x0, 0x0) (tid=3246455)
    #0 0x7f345ea81979 in CheckUnwind ../../../../src/libsanitizer/asan/asan_rtl.cpp:69
    #1 0x7f345eaa790d in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) ../../../../src/libsanitizer/sanitizer_common/sanitizer_termination.cpp:86
    #2 0x7f345e9e1d54 in __interceptor___cxa_throw ../../../../src/libsanitizer/asan/asan_interceptors.cpp:335
    #3 0x7f345e9e1d54 in __interceptor___cxa_throw ../../../../src/libsanitizer/asan/asan_interceptors.cpp:334
    #4 0x7f3458623def in void boost::throw_exception<boost::bad_lexical_cast>(boost::bad_lexical_cast const&) /opt/ceph/include/boost/throw_exception.hpp:165
    #5 0x7f345997ad3b in void boost::conversion::detail::throw_bad_cast<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, unsigned long>() /opt/ceph/include/boost/lexical_cast/bad_lexical_cast.hpp:93
    #6 0x7f3459979d35 in unsigned long boost::lexical_cast<unsigned long, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /opt/ceph/include/boost/lexical_cast.hpp:43`
```

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
ifed01 pushed a commit that referenced this pull request Dec 6, 2025
The static std::map max_prio_map was defined in the osd_types.h header
file, causing every translation unit that included this header to get
its own copy of the variable. This led to One Definition Rule (ODR)
violations where multiple instances of the same variable existed at
runtime.

During program cleanup, destructors for these multiple instances would
attempt to free the same memory regions, resulting in segmentation
faults in tcmalloc/memory allocator as seen with ceph-dencoder.

This issue surfaced after a yet-merged-change which converts erasure_code
and json_spirit to OBJECT libraries. Before that change, these were
STATIC libraries that were linked via target_link_libraries. The
incorrect linkage meant their object files (and thus their copies of
max_prio_map) were kept separate and didn't conflict at runtime.

After converting to OBJECT libraries and properly incorporating them
into libceph-common.so (commit 8b0e3fb2c23), the multiple copies of
max_prio_map from different translation units all ended up in the same
shared library, exposing the ODR violation. During program exit, the
dynamic linker attempted to run destructors for all instances, leading
to double-free crashes.

Fix by moving the map into a static helper function in PeeringState.cc
(the only file that uses it). The map is now a function-local static
const variable, ensuring a single instance that is properly initialized
and destructed.

Backtrace before fix:
```
    #0  0x00007ffff7dbb1a0 in tcmalloc::ThreadCache::ReleaseToCentralCache(tcmalloc::ThreadCache::FreeList*, unsigned int, int) () from /lib/x86_64-linux-gnu/libtcmalloc.so.4
    #1  0x00007ffff7dbb57f in tcmalloc::ThreadCache::Scavenge() () from /lib/x86_64-linux-gnu/libtcmalloc.so.4
    #2  0x00007ffff6bc8aa2 in std::__new_allocator<std::_Rb_tree_node<std::pair<int const, int> > >::deallocate (this=0x7ffff7d48f78 <max_prio_map>, __p=0x555555f43890, __n=1)
    #3  0x00007ffff6bc89f9 in std::allocator<std::_Rb_tree_node<std::pair<int const, int> > >::deallocate (this=0x7ffff7d48f78 <max_prio_map>, __p=0x555555f43890, __n=1)
    #4  std::allocator_traits<std::allocator<std::_Rb_tree_node<std::pair<int const, int> > > >::deallocate (__a=..., __p=0x555555f43890, __n=1)
    #5  std::_Rb_tree<int, std::pair<int const, int>, std::_Select1st<std::pair<int const, int> >, std::less<int>, std::allocator<std::pair<int const, int> > >::_M_put_node (this=0x7ffff7d48f78 <max_prio_map>, __p=0x555555f43890)
    #6  0x00007ffff6bc892e in std::_Rb_tree<int, std::pair<int const, int>, std::_Select1st<std::pair<int const, int> >, std::less<int>, std::allocator<std::pair<int const, int> > >::_M_drop_node (this=0x7ffff7d48f78 <max_prio_map>, __p=0x555555f43890)
    #7  0x00007ffff6bc886e in std::_Rb_tree<int, std::pair<int const, int>, std::_Select1st<std::pair<int const, int> >, std::less<int>, std::allocator<std::pair<int const, int> > >::_M_erase (this=0x7ffff7d48f78 <max_prio_map>, __x=0x555555f43890)
    ceph#8  0x00007ffff6bc8854 in std::_Rb_tree<int, std::pair<int const, int>, std::_Select1st<std::pair<int const, int> >, std::less<int>, std::allocator<std::pair<int const, int> > >::_M_erase (this=0x7ffff7d48f78 <max_prio_map>, __x=0x555555f43cb0)
    ceph#9  0x00007ffff6bc8854 in std::_Rb_tree<int, std::pair<int const, int>, std::_Select1st<std::pair<int const, int> >, std::less<int>, std::allocator<std::pair<int const, int> > >::_M_erase (this=0x7ffff7d48f78 <max_prio_map>, __x=0x555555f43ad0)
    ceph#10 0x00007ffff6bc8805 in std::_Rb_tree<int, std::pair<int const, int>, std::_Select1st<std::pair<int const, int> >, std::less<int>, std::allocator<std::pair<int const, int> > >::~_Rb_tree (this=0x7ffff7d48f78 <max_prio_map>)
    ceph#11 0x00007ffff6bc7345 in std::map<int, int, std::less<int>, std::allocator<std::pair<int const, int> > >::~map (this=0x7ffff7d48f78 <max_prio_map>)
    ceph#12 0x00007ffff484bd51 in __cxa_finalize (d=0x7ffff7d3f440) at ./stdlib/cxa_finalize.c:97
    ceph#13 0x00007ffff6af9487 in __do_global_dtors_aux () from /home/kefu/dev/ceph/build/lib/libceph-common.so.2
    ceph#14 0x00007ffff7fbfd20 in ?? ()
    ceph#15 0x00007ffff7fc8fc2 in _dl_call_fini (closure_map=0x7fffffffd0f0, closure_map@entry=0x7ffff7fbfd20) at ./elf/dl-call_fini.c:43
    ceph#16 0x00007ffff7fcbe72 in _dl_fini () at ./elf/dl-fini.c:120
    ceph#17 0x00007ffff484c291 in __run_exit_handlers (status=0, listp=0x7ffff49f1680 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true, run_dtors=run_dtors@entry=true) at ./stdlib/exit.c:118
    ceph#18 0x00007ffff484c35a in __GI_exit (status=<optimized out>) at ./stdlib/exit.c:148
    ceph#19 0x00007ffff4833caf in __libc_start_call_main (main=main@entry=0x55555556cd90 <main(int, char const**)>, argc=argc@entry=2, argv=argv@entry=0x7fffffffd488) at ../sysdeps/nptl/libc_start_call_main.h:74
    ceph#20 0x00007ffff4833d65 in __libc_start_main_impl (main=0x55555556cd90 <main(int, char const**)>, argc=2, argv=0x7fffffffd488, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffd478) at ../csu/libc-start.c:360
    ceph#21 0x00005555555695e1 in _start ()
```

Signed-off-by: Kefu Chai <k.chai@proxmox.com>
sajibreadd-croit pushed a commit that referenced this pull request Dec 18, 2025
See comment:
```
  //TODO: should be changed to return future<> once all calls
  //	  to refresh are through co_await. We return LBAMapping
  //	  for now to avoid mandating the callers to make sure
  //	  the life of the lba mapping survives the refresh.
```

For now introduce co_refresh and mark the existing refresh as
deprecated. Following work will audit all the existing users of
refresh and move them to the new method. This change is not trivial
so I prefer to follow up on this as a separate PR.

This should help avoiding UAR in suspension points:
```
==103588==ERROR: AddressSanitizer: stack-use-after-return on address 0xffff80197e90 at pc 0xaaaacb941b24 bp 0xffff7e48dd80 sp 0xffff7e48dd78
READ of size 8 at 0xffff80197e90 thread T1
    #0 0xaaaacb941b20 in boost::intrusive_ptr<crimson::os::seastore::LBACursor>::swap(boost::intrusive_ptr<crimson::os::seastore::LBACursor>&) /opt/ceph/include/boost/smart_ptr/intrusive_ptr.hpp:172:18
    #1 0xaaaacb941998 in boost::intrusive_ptr<crimson::os::seastore::LBACursor>::operator=(boost::intrusive_ptr<crimson::os::seastore::LBACursor>&&) /opt/ceph/include/boost/smart_ptr/intrusive_ptr.hpp:93:61
    #2 0xaaaacb933758 in crimson::os::seastore::LBAMapping::operator=(crimson::os::seastore::LBAMapping&&) /ceph/src/crimson/os/seastore/lba_mapping.h:46:48
    #3 0xaaaacde2fa54 in ... crimson::os::seastore::LBAMapping&&, std::array<crimson::os::seastore::LBAManager::remap_entry_t, 1ul>) (.resume) /ceph/src/crimson/os/seastore/transaction_manager.h:1282:11
```

Deprecate is commented out since otherwise make check would fail.

Signed-off-by: Matan Breizman <mbreizma@redhat.com>
sajibreadd-croit pushed a commit that referenced this pull request Feb 11, 2026
Fix memory leaks detected by AddressSanitizer in unittest_dbstore_tests.
The test was failing with ASan enabled due to SQLObjectOp objects not
being properly cleaned up.

ASan reported the following leaks:

  Direct leak of 200 byte(s) in 1 object(s) allocated from:
    #0 operator new(unsigned long)
    #1 SQLGetBucket::Execute(DoutPrefixProvider const*, rgw::store::DBOpParams*)
       /src/rgw/driver/dbstore/sqlite/sqliteDB.cc:1689
    #2 rgw::store::DB::ProcessOp(DoutPrefixProvider const*, ...)
       /src/rgw/driver/dbstore/common/dbstore.cc:258

  Direct leak of 200 byte(s) in 1 object(s) allocated from:
    #0 operator new(unsigned long)
    #1 SQLInsertBucket::Execute(DoutPrefixProvider const*, rgw::store::DBOpParams*)
       /src/rgw/driver/dbstore/sqlite/sqliteDB.cc:1433
    #2 rgw::store::DB::ProcessOp(DoutPrefixProvider const*, ...)
       /src/rgw/driver/dbstore/common/dbstore.cc:258

  SUMMARY: AddressSanitizer: 460550 byte(s) leaked in 1823 allocation(s).

Root cause: The DB::Destroy() method had an early return when the db
pointer was NULL, preventing cleanup of the objectmap which stores
SQLObjectOp pointers. These objects were allocated during test execution
but never freed.

Changes:
- Modified DB::Destroy() to always clean up objectmap even when db is NULL
- Added explicit delete in objectmapDelete() for consistency
- Added lsan suppression for SQLite internal allocations (indirect leaks)

After the fix, all direct leaks are eliminated. Only indirect leaks from
SQLite's internal memory management remain, which are now suppressed.

Test results:
- Before: 460,550 bytes leaked (including 2 direct leaks of 200 bytes each)
- After: 0 direct leaks, unittest_dbstore_tests passes with ASan

Signed-off-by: Kefu Chai <k.chai@proxmox.com>
sajibreadd-croit pushed a commit that referenced this pull request Feb 11, 2026
The ExtentMap.reshard_failure test was leaking memory by not properly
cleaning up the OnodeCacheShard and BufferCacheShard objects it created.

ASan reported:
  Direct leak of 9928 byte(s) in 1 object(s) allocated from:
    #1 BlueStore::OnodeCacheShard::create() BlueStore.cc:1221
    #2 ExtentMap_reshard_failure_Test::TestBody() test_bluestore_types.cc:1244

  Direct leak of 224 byte(s) in 1 object(s) allocated from:
    #1 BlueStore::BufferCacheShard::create() BlueStore.cc:1680
    #2 ExtentMap_reshard_failure_Test::TestBody() test_bluestore_types.cc:1246

  SUMMARY: AddressSanitizer: 10288 byte(s) leaked in 8 allocation(s).

Fix by:
1. Wrapping coll and onode in an additional scope block to ensure they
   are destroyed before the cache shards (releasing all blob references)
2. Adding proper cleanup with delete bc and delete oc at test end

This matches the cleanup pattern used in BlueStoreFixture::TearDown().

Signed-off-by: Kefu Chai <k.chai@proxmox.com>
sajibreadd-croit pushed a commit that referenced this pull request Feb 11, 2026
…yed static"

```
Jan 20 09:27:16 ceph-node-0 ceph-e818662e-f5e1-11f0-b263-525400908ba7-osd-1[12300]: AddressSanitizer:DEADLYSIGNAL
Jan 20 09:27:16 ceph-node-0 ceph-e818662e-f5e1-11f0-b263-525400908ba7-osd-1[12300]: =================================================================
Jan 20 09:27:16 ceph-node-0 ceph-e818662e-f5e1-11f0-b263-525400908ba7-osd-1[12300]: ==3==ERROR: AddressSanitizer: stack-overflow on address 0x7b512f6c8dd8 (pc 0x0000046e7a72 bp 0x7b512de7c900 sp 0x7b512f6c8dd8 T0)
Jan 20 09:27:17 ceph-node-0 ceph-e818662e-f5e1-11f0-b263-525400908ba7-osd-1[12300]:     #0 0x0000046e7a72 in get_global_options() (/usr/bin/ceph-osd-crimson+0x46e7a72) (BuildId: 2a86043f51c9be9cb19801e276fb3ee36239556a)
Jan 20 09:27:17 ceph-node-0 ceph-e818662e-f5e1-11f0-b263-525400908ba7-osd-1[12300]:     #1 0x0000046e540e in build_options() (/usr/bin/ceph-osd-crimson+0x46e540e) (BuildId: 2a86043f51c9be9cb19801e276fb3ee36239556a)
Jan 20 09:27:17 ceph-node-0 ceph-e818662e-f5e1-11f0-b263-525400908ba7-osd-1[12300]:     #2 0x0000033b7949 in get_ceph_options() (/usr/bin/ceph-osd-crimson+0x33b7949) (BuildId: 2a86043f51c9be9cb19801e276fb3ee36239556a)
Jan 20 09:27:17 ceph-node-0 ceph-e818662e-f5e1-11f0-b263-525400908ba7-osd-1[12300]:     #3 0x000003440540 in md_config_t::md_config_t(ConfigValues&, ConfigTracker const&, bool) (/usr/bin/ceph-osd-crimson+0x3440540) (BuildId: 2a860>
Jan 20 09:27:17 ceph-node-0 ceph-e818662e-f5e1-11f0-b263-525400908ba7-osd-1[12300]:     #4 0x0000046856a8 in crimson::common::ConfigProxy::ConfigProxy(EntityName const&, std::basic_string_view<char, std::char_traits<char> >) (/usr>
Jan 20 09:27:17 ceph-node-0 ceph-e818662e-f5e1-11f0-b263-525400908ba7-osd-1[12300]:     #5 0x000000eb6cb5 in seastar::shared_ptr_count_for<crimson::common::ConfigProxy>::shared_ptr_count_for<EntityName&, std::__cxx11::basic_string>
..
Jan 20 09:27:17 ceph-node-0 ceph-e818662e-f5e1-11f0-b263-525400908ba7-osd-1[12300]:     ceph#40 0x000000ed6434 in seastar::future<int> seastar::futurize<int>::apply<crimson::osd::_get_early_config(int, char const**)::{lambda()#1}::ope>
Jan 20 09:27:17 ceph-node-0 ceph-e818662e-f5e1-11f0-b263-525400908ba7-osd-1[12300]:     ceph#41 0x000000ed672b in seastar::async<crimson::osd::_get_early_config(int, char const**)::{lambda()#1}::operator()() const::{lambda()#1}>(seast>
```

This reverts commit 1ab0a8c.

Fixes: https://tracker.ceph.com/issues/74481

Signed-off-by: Matan Breizman <mbreizma@redhat.com>
sajibreadd-croit pushed a commit that referenced this pull request Feb 11, 2026
Fix memory leak detected by AddressSanitizer in unittest_http_manager.
The test was failing with ASan enabled due to rgw_http_req_data objects
not being properly cleaned up when the HTTP manager thread exits.

ASan reported the following leaks:

  Direct leak of 17152 byte(s) in 32 object(s) allocated from:
    #0 operator new(unsigned long)
    #1 RGWHTTPManager::add_request(RGWHTTPClient*)
       /ceph/src/rgw/rgw_http_client.cc:946:33
    #2 HTTPManager_SignalThread_Test::TestBody()
       /ceph/src/test/rgw/test_http_manager.cc:132:10

  Indirect leak of 768 byte(s) in 32 object(s) allocated from:
    #0 operator new(unsigned long)
    #1 rgw_http_req_data::rgw_http_req_data()
       /ceph/src/rgw/rgw_http_client.cc:52:22
    #2 RGWHTTPManager::add_request(RGWHTTPClient*)
       /ceph/src/rgw/rgw_http_client.cc:946:37

  SUMMARY: AddressSanitizer: 17920 byte(s) leaked in 64 allocation(s).

Root cause: The rgw_http_req_data class uses reference counting
(inherits from RefCountedObject). When a request is unregistered,
unregister_request() calls get() to increment the refcount, expecting
a corresponding put() to be called later.

In manage_pending_requests(), unregistered requests are properly
handled with both _unlink_request() and put(). However, in the thread
cleanup code (reqs_thread_entry exit path), only _unlink_request() was
called without the matching put(), causing a reference count leak.

The fix adds the missing put() call in the thread cleanup code to match
the reference counting pattern used in manage_pending_requests().

Test results:
- Before: 17,920 bytes leaked in 64 allocations
- After: 0 leaks, unittest_http_manager passes with ASan

Fixes: https://tracker.ceph.com/issues/74762
Signed-off-by: Kefu Chai <k.chai@proxmox.com>
sajibreadd-croit pushed a commit that referenced this pull request Feb 16, 2026
This commit fixes a critical cache key collision bug in the ISA erasure
code plugin that could lead to heap-buffer-overflow and silent data
corruption.

Problem:
--------
The decoding table cache was indexed only by matrix type and erasure
signature (available/missing chunk pattern), but did NOT include the
(k,m) erasure code configuration parameters. This caused different EC
configurations with similar erasure patterns to collide in the cache,
leading to incorrectly-sized cached buffers being reused.

AddressSanitizer Report:
------------------------
==4904==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x5160001397b8 at pc 0x5de8e415296b bp 0x7ffc82260310 sp 0x7ffc8225fad0
READ of size 576 at 0x5160001397b8 thread T0
    #0 __asan_memcpy
    #1 ErasureCodeIsaTableCache::getDecodingTableFromCache()
       .../ErasureCodeIsaTableCache.cc:260:5
    #2 ErasureCodeIsaDefault::isa_decode()
       .../ErasureCodeIsa.cc:490:15

0x5160001397b8 is located 0 bytes after 568-byte region
[0x516000139580,0x5160001397b8) allocated by:
    #0 posix_memalign
    #1 ceph::buffer::raw_combined::alloc_data_n_controlblock()
    #2 ErasureCodeIsaTableCache::putDecodingTableToCache()
       .../ErasureCodeIsaTableCache.cc:319:18

Root Cause:
-----------
Scenario illustrating the bug:
1. First decode operation: k=2, m=1, erasure pattern "+0+2-1"
   - Creates cache entry with key "+0+2-1"
   - Allocates buffer: 2*(1+2)*32 = 192 bytes
2. Second decode operation: k=3, m=3, same erasure pattern "+0+2-1"
   - Looks up cache with key "+0+2-1" → COLLISION
   - Retrieves 192-byte buffer but needs 3*(3+3)*32 = 576 bytes
   - Result: Heap-buffer-overflow (reads 384 bytes beyond allocation)

Worse scenario (silent corruption):
1. First decode: k=3, m=3 → caches 576-byte table
2. Second decode: k=2, m=1 → retrieves wrong table
   - Uses incorrect decoding matrix
   - Result: Silent data corruption during recovery

Solution:
---------
Include k and m parameters in cache signature
 - Old format: "+0+2+3-1-4"
 - New format: "k3m2a+0+2+3e-1-4"

Test Fix:
---------
Also fixes a buffer overflow in TestErasureCodePlugins.cc where
hashes_bl offset was calculated using chunk_size instead of sizeof(uint32_t),
causing reads beyond the CRC buffer.

Production Impact:
------------------

Backward Compatibility: FULLY COMPATIBLE
- Cache is ephemeral (in-memory only, not persisted)
- Cache cleared on process restart
- Rolling upgrades safe - each OSD restart gets fixed code
- Old cache entries automatically invalidated on upgrade
- No wire protocol or on-disk format changes
- No configuration changes required
- No breaking changes

Data Integrity:
- Eliminates silent data corruption risk
- Eliminates heap-buffer-overflow crashes
- Cache now correctly isolated by (k,m) configuration
- Correct decoding tables always used for recovery
- No risk of corrupting user data from the fix itself

Why Users Haven't Complained:
------------------------------

Several factors likely prevented widespread reports:

1. Low probability conditions required:
   - Need multiple EC pools with DIFFERENT (k,m) configurations
   - Need similar erasure patterns across pools
   - Need cache collision to occur during actual recovery operations
   - Recovery operations are relatively rare in healthy clusters

2. Crash vs silent corruption detection:
   - Buffer overflows (easier to detect) occur when k2,m2 > k1,m1
   - Silent corruption (harder to detect) occurs when k2,m2 < k1,m1
   - Crashes might be attributed to other causes
   - Data corruption only detected during scrub or data verification

3. Common deployment patterns:
   - Many deployments use single EC configuration cluster-wide
   - Default EC configurations (k=2,m=1 or k=4,m=2) reduce collision space
   - Erasure pattern variety may be insufficient for collisions

4. ISA plugin usage:
   - Not universally deployed (requires Intel ISA-L library)
   - Some sites use jerasure plugin instead
   - Plugin selection depends on hardware and configuration

5. Detection difficulty:
   - ASan not enabled in production builds
   - Silent corruption only appears during:
     * Degraded reads with recovery
     * Scrub operations
     * Deep-scrub verification
   - Corrupted data might not be immediately accessed

Fixes: https://tracker.ceph.com/issues/74382

Signed-off-by: Kefu Chai <k.chai@proxmox.com>
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.

1 participant