Skip to content

fix: heap-buffer-overflow in sorted_buffer_gt::insert()#9

Merged
rschu1ze merged 4 commits intoClickHouse/v2.24.0from
fix/sorted-buffer-heap-overflow
Apr 3, 2026
Merged

fix: heap-buffer-overflow in sorted_buffer_gt::insert()#9
rschu1ze merged 4 commits intoClickHouse/v2.24.0from
fix/sorted-buffer-heap-overflow

Conversation

@dustinhealy
Copy link
Copy Markdown
Collaborator

Summary

  • Fixes heap-buffer-overflow detected by ASAN in ClickHouse's arm_asan_ubsan stress test (ClickHouse/ClickHouse#100411)
  • Root cause: search_to_insert_() and search_to_update_() used sorted_buffer_gt without calling reserve() first, and reserve() had an off-by-one (< vs <=) causing spurious reallocation

Fixes

  • reserve(): change < to <= to avoid spurious reallocation when new_capacity == capacity_ (both max_heap_gt and sorted_buffer_gt)
  • search_to_insert_(): add missing top.reserve(top_limit) and next.reserve(top_limit)
  • search_to_update_(): same missing reserve calls

Tests

  • 3 sorted_buffer_gt unit tests (reserve fix, fill and eviction, insert_reserved)
  • 1 max_heap_gt reserve test
  • Full index search path test with expansion values up to 512
  • 8-thread concurrent search stress test (400 queries under ASAN)

sorted_buffer_gt::insert(element, limit) trusted callers to have reserved
at least `limit` elements. When limit > capacity_, the shift-loop
(source[1] = source[0]) writes past the heap allocation, triggering ASAN.

Root cause: search_to_insert_() and search_to_update_() called
top.insert_reserved() and top.insert() without first calling
top.reserve(top_limit).

Fixes:
- reserve(): change < to <= to avoid spurious reallocation when
  new_capacity == capacity_
- insert_reserved(): guard against zero capacity / full buffer
- insert(): clamp limit to capacity to prevent out-of-bounds write
- search_to_insert_(): add missing top.reserve() and next.reserve()
- search_to_update_(): same missing reserve calls
Remove insert_reserved() zero-capacity guard and insert() limit clamping.
These masked bugs rather than surfacing them under ASAN. The real fix is
the missing reserve() calls in search_to_insert_() / search_to_update_()
and the <= off-by-one in reserve().
@alexey-milovidov alexey-milovidov self-assigned this Mar 24, 2026
@dustinhealy dustinhealy marked this pull request as ready for review March 24, 2026 21:46
@dustinhealy
Copy link
Copy Markdown
Collaborator Author

@shankar-iyer Thanks for your help with this - could you let me know when / if this is okay to merge?

@rschu1ze rschu1ze merged commit c5f2b22 into ClickHouse/v2.24.0 Apr 3, 2026
@rschu1ze
Copy link
Copy Markdown
Member

rschu1ze commented Apr 3, 2026

Port to upstream: unum-cloud#740

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants