Skip to content

Commit 81534dd

Browse files
authored
[bugfix](memtable) arena is freed early and will cause use after free (apache#46997)
### What problem does this PR solve? Related PR: apache#40912 Problem Summary: Do not reset _arena in MemTable::to_block(), because it is still used in ~MemTable() when releasing agg places Fix the following use-after-free Use: ==3628099==ERROR: AddressSanitizer: heap-use-after-free on address 0x52100381be60 at pc 0x5648f30893f8 bp 0x7f8842433310 sp 0x7f8842433308 READ of size 8 at 0x52100381be60 thread T4767 (wg_flush_broker) #0 0x5648f30893f7 in phmap::priv::raw_hash_set<phmap::priv::FlatHashSetPolicy<unsigned long>, phmap::Hash<unsigned long>, phmap::EqualTo<unsigned long>, std::allocator<unsigned long>>::destroy_slots() doris/thirdparty/installed/include/parallel_hashmap/phmap.h:1992:14 #1 0x5648f30936f6 in phmap::priv::raw_hash_set<phmap::priv::FlatHashSetPolicy<unsigned long>, phmap::Hash<unsigned long>, phmap::EqualTo<unsigned long>, std::allocator<unsigned long>>::~raw_hash_set() doris/thirdparty/installed/include/parallel_hashmap/phmap.h:1236:23 #2 0x5648f3089276 in phmap::flat_hash_set<unsigned long, phmap::Hash<unsigned long>, phmap::EqualTo<unsigned long>, std::allocator<unsigned long>>::~flat_hash_set() doris/thirdparty/installed/include/parallel_hashmap/phmap.h:4577:7 #3 0x5648f308922a in doris::BitmapValue::~BitmapValue() doris/be/src/util/bitmap_value.h:824:7 #4 0x56490d319fa6 in doris::vectorized::AggregateFunctionBitmapData<doris::vectorized::AggregateFunctionBitmapUnionOp>::~AggregateFunctionBitmapData() doris/be/src/vec/aggregate_functions/aggregate_function_bitmap.h:127:8 #5 0x56490d49636a in doris::vectorized::IAggregateFunctionDataHelper<doris::vectorized::AggregateFunctionBitmapData<doris::vectorized::AggregateFunctionBitmapUnionOp>, doris::vectorized::AggregateFunctionBitmapOp<doris::vectorized::AggregateFunctionBitmapUnionOp>>::destroy(char*) const doris/be/src/vec/aggregate_functions/aggregate_function.h:563:92 #6 0x5648f68376e9 in doris::MemTable::~MemTable() doris/be/src/olap/memtable.cpp:159:27 Free: 0x52100381be60 is located 352 bytes inside of 4096-byte region [0x52100381bd00,0x52100381cd00) freed by thread T4767 (wg_flush_broker) here: #0 0x5648f2f3ee46 in free (doris/output/be/lib/doris_be+0x57418e46) (BuildId: 298b9c91a1ec8fe0) #1 0x5648f3080dfc in DefaultMemoryAllocator::free(void*) doris/be/src/vec/common/allocator.h:108:41 #2 0x5648f3080b3f in Allocator<false, false, false, DefaultMemoryAllocator>::free(void*, unsigned long) doris/be/src/vec/common/allocator.h:323:13 #3 0x5648f30b6dee in doris::vectorized::Arena::Chunk::~Chunk() doris/be/src/vec/common/arena.h:77:31 #4 0x5648f30b6d1f in doris::vectorized::Arena::~Arena() doris/be/src/vec/common/arena.h:151:16 #5 0x5648f30b695a in std::default_delete<doris::vectorized::Arena>::operator()(doris::vectorized::Arena*) const env/ldb_toolchain/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:99:2 #6 0x5648f30b67c8 in std::__uniq_ptr_impl<doris::vectorized::Arena, std::default_delete<doris::vectorized::Arena>>::reset(doris::vectorized::Arena*) env/ldb_toolchain/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:211:4 #7 0x5648f30b5d8c in std::unique_ptr<doris::vectorized::Arena, std::default_delete<doris::vectorized::Arena>>::reset(doris::vectorized::Arena*) env/ldb_toolchain/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:509:7 apache#8 0x5648f684253b in doris::MemTable::_to_block(std::unique_ptr<doris::vectorized::Block, std::default_delete<doris::vectorized::Block>>*) doris/be/src/olap/memtable.cpp:522:12 apache#9 0x5648f6842ac5 in doris::MemTable::to_block(std::unique_ptr<doris::vectorized::Block, std::default_delete<doris::vectorized::Block>>*) doris/be/src/olap/memtable.cpp:528:5 apache#10 0x5648f6907a72 in doris::FlushToken::_do_flush_memtable(doris::MemTable*, int, long*) doris/be/src/olap/memtable_flush_executor.cpp:144:9 apache#11 0x5648f690932c in doris::FlushToken::_flush_memtable(std::shared_ptr<doris::MemTable>, int, long) doris/be/src/olap/memtable_flush_executor.cpp:183:16 apache#12 0x5648f6915d18 in doris::MemtableFlushTask::run() doris/be/src/olap/memtable_flush_executor.cpp:60:20
1 parent a0f4c4f commit 81534dd

File tree

1 file changed

+28
-24
lines changed

1 file changed

+28
-24
lines changed

be/src/olap/memtable.cpp

+28-24
Original file line numberDiff line numberDiff line change
@@ -144,35 +144,41 @@ void MemTable::_init_agg_functions(const vectorized::Block* block) {
144144

145145
MemTable::~MemTable() {
146146
SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(_query_thread_context.query_mem_tracker);
147+
{
148+
SCOPED_CONSUME_MEM_TRACKER(_mem_tracker);
149+
g_memtable_cnt << -1;
150+
if (_keys_type != KeysType::DUP_KEYS) {
151+
for (auto it = _row_in_blocks.begin(); it != _row_in_blocks.end(); it++) {
152+
if (!(*it)->has_init_agg()) {
153+
continue;
154+
}
155+
// We should release agg_places here, because they are not released when a
156+
// load is canceled.
157+
for (size_t i = _tablet_schema->num_key_columns(); i < _num_columns; ++i) {
158+
auto function = _agg_functions[i];
159+
DCHECK(function != nullptr);
160+
function->destroy((*it)->agg_places(i));
161+
}
162+
}
163+
}
164+
std::for_each(_row_in_blocks.begin(), _row_in_blocks.end(),
165+
std::default_delete<RowInBlock>());
166+
// Arena has to be destroyed after agg state, because some agg state's memory may be
167+
// allocated in arena.
168+
_arena.reset();
169+
_vec_row_comparator.reset();
170+
_row_in_blocks.clear();
171+
_agg_functions.clear();
172+
_input_mutable_block.clear();
173+
_output_mutable_block.clear();
174+
}
147175
if (_is_flush_success) {
148176
// If the memtable is flush success, then its memtracker's consumption should be 0
149177
if (_mem_tracker->consumption() != 0 && config::crash_in_memory_tracker_inaccurate) {
150178
LOG(FATAL) << "memtable flush success but cosumption is not 0, it is "
151179
<< _mem_tracker->consumption();
152180
}
153181
}
154-
g_memtable_cnt << -1;
155-
if (_keys_type != KeysType::DUP_KEYS) {
156-
for (auto it = _row_in_blocks.begin(); it != _row_in_blocks.end(); it++) {
157-
if (!(*it)->has_init_agg()) {
158-
continue;
159-
}
160-
// We should release agg_places here, because they are not released when a
161-
// load is canceled.
162-
for (size_t i = _tablet_schema->num_key_columns(); i < _num_columns; ++i) {
163-
auto function = _agg_functions[i];
164-
DCHECK(function != nullptr);
165-
function->destroy((*it)->agg_places(i));
166-
}
167-
}
168-
}
169-
std::for_each(_row_in_blocks.begin(), _row_in_blocks.end(), std::default_delete<RowInBlock>());
170-
_arena.reset();
171-
_vec_row_comparator.reset();
172-
_row_in_blocks.clear();
173-
_agg_functions.clear();
174-
_input_mutable_block.clear();
175-
_output_mutable_block.clear();
176182
}
177183

178184
int RowInBlockComparator::operator()(const RowInBlock* left, const RowInBlock* right) const {
@@ -629,8 +635,6 @@ Status MemTable::_to_block(std::unique_ptr<vectorized::Block>* res) {
629635
RETURN_IF_ERROR(_sort_by_cluster_keys());
630636
}
631637
_input_mutable_block.clear();
632-
// After to block, all data in arena is saved in the block
633-
_arena.reset();
634638
*res = vectorized::Block::create_unique(_output_mutable_block.to_block());
635639
return Status::OK();
636640
}

0 commit comments

Comments
 (0)