Fix groupArraySorted() for optimize_aggregation_in_order and WITH TOTALS#36815
Closed
azat wants to merge 1 commit intoClickHouse:masterfrom
Closed
Fix groupArraySorted() for optimize_aggregation_in_order and WITH TOTALS#36815azat wants to merge 1 commit intoClickHouse:masterfrom
azat wants to merge 1 commit intoClickHouse:masterfrom
Conversation
groupArraySorted() merge operation was incorrect, it does not moves data
to new arena, and hence triggers use-after-free.
ASan report [1]:
==103==ERROR: AddressSanitizer: heap-use-after-free on address 0x62100c12022c at pc 0x00000d49cbcc bp 0x7f345c320e40 sp 0x7f345c3205e8
READ of size 12 at 0x62100c12022c thread T244 (QueryPipelineEx)
7 0x143e19e4 in DB::AggregateFunctionGroupArraySortedDataBase<>::merge() build_docker/../src/AggregateFunctions/AggregateFunctionGroupArraySortedData.h:69:16
8 0x2c80296e in DB::ColumnAggregateFunction::insertMergeFrom() build_docker/../src/Columns/ColumnAggregateFunction.cpp:470:11
9 0x2ec61996 in DB::TotalsHavingTransform::addToTotals() build_docker/../src/Processors/Transforms/TotalsHavingTransform.cpp:283:35
10 0x2ec5f400 in DB::TotalsHavingTransform::transform() build_docker/../src/Processors/Transforms/TotalsHavingTransform.cpp:188:9
0x62100c12022c is located 300 bytes inside of 4096-byte region [0x62100c120100,0x62100c121100)
freed by thread T244 (QueryPipelineEx) here:
14 0x2c7f8ecd in DB::ColumnAggregateFunction::~ColumnAggregateFunction() build_docker/../src/Columns/ColumnAggregateFunction.cpp:85:1
26 0x2d23ffc3 in DB::Chunk::operator=(DB::Chunk&&) build_docker/../src/Processors/Chunk.h:53:17
27 0x2ec5f410 in DB::TotalsHavingTransform::transform(DB::Chunk&) build_docker/../src/Processors/Transforms/TotalsHavingTransform.cpp:189:15
previously allocated by thread T3 (TCPHandler) here:
18 0x2ed80019 in DB::AggregatingInOrderTransform::AggregatingInOrderTransform() build_docker/../src/Processors/Transforms/AggregatingInOrderTransform.cpp:20:9
SUMMARY: AddressSanitizer: heap-use-after-free crtstuff.c in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long)
[1]: https://s3.amazonaws.com/clickhouse-test-reports/35111/0ce44f30210d362c3436f03e926bf7893b034f06/fuzzer_astfuzzerasan,actions//report.html
Fixes: ClickHouse#34055 (cc @palegre-tiny @evillique)
Signed-off-by: Azat Khuzhin <[email protected]>
robot-clickhouse
added
the
pr-bugfix
Member
|
@azat Fuzzer has found many issues. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix use-after-free in
groupArraySorted()foroptimize_aggregation_in_orderandWITH TOTALSgroupArraySorted() merge operation was incorrect, it does not moves data
to new arena, and hence triggers use-after-free.
ASan report 1:
Fixes: #34055 (cc @palegre-tiny @evillique)
P.S. Serializing objects into StringRef looks odd