Fix possible use-after-free for INSERT into MV with concurrent DROP

ASan founds [1]:

    ==553==
    ERROR: AddressSanitizer: heap-use-after-free on address 0x61e004694080 at pc 0x000029150af2 bp 0x7f70b3f8ef10 sp 0x7f70b3f8ef08
    READ of size 8 at 0x61e004694080 thread T477 (QueryPipelineEx)
        0 0x29150af1 in DB::MergeTreeDataWriter::writeTempPart() >
        1 0x293b8e43 in DB::MergeTreeSink::consume(DB::Chunk) obj-x86_64-linux-gnu/../src/Storages/MergeTree/MergeTreeSink.cpp:27:65
        2 0x29dac73b in DB::SinkToStorage::onConsume(DB::Chunk) obj-x86_64-linux-gnu/../src/Processors/Sinks/SinkToStorage.cpp:18:5
        3 0x29c72dd2 in DB::ExceptionKeepingTransform::work()::$_1::operator()() const obj-x86_64-linux-gnu/../src/Processors/Transforms/ExceptionKeepingTransform.cpp:151:51

    0x61e004694080 is located 2048 bytes inside of 2480-byte region [0x61e004693880,0x61e004694230)
    freed by thread T199 (BgSchPool) here:
        ...
        4 0x26220f20 in DB::DatabaseCatalog::TableMarkedAsDropped::~TableMarkedAsDropped() obj-x86_64-linux-gnu/../src/Interpreters/DatabaseCatalog.h:248:12
        5 0x26220f20 in DB::DatabaseCatalog::dropTableDataTask() obj-x86_64-linux-gnu/../src/Interpreters/DatabaseCatalog.cpp:908:1

      [1]: https://s3.amazonaws.com/clickhouse-test-reports/33201/4f04d6af61eabf4899eb8188150dc862aaab80fc/stress_test__address__actions_.html

There was a fix in #32572, but it was not complete (yes it reduced the
race window a lot, but not completely), since the inner table still can
go away after the INSERT chain was built, to fix this obtain the
reference earlier.

Follow-up for: #32572 (cc @tavplubix)
This commit is contained in:
Azat Khuzhin 2022-01-04 13:27:53 +03:00
parent b5ee8cb694
commit 5ed7440381

View File

@ -263,6 +263,10 @@ BlockIO InterpreterInsertQuery::execute()
QueryPipelineBuilder pipeline;
StoragePtr table = getTable(query);
StoragePtr inner_table;
if (const auto * mv = dynamic_cast<const StorageMaterializedView *>(table.get()))
inner_table = mv->getTargetTable();
if (query.partition_by && !table->supportsPartitionBy())
throw Exception(ErrorCodes::NOT_IMPLEMENTED, "PARTITION BY clause is not supported by storage");
@ -450,11 +454,8 @@ BlockIO InterpreterInsertQuery::execute()
}
res.pipeline.addStorageHolder(table);
if (const auto * mv = dynamic_cast<const StorageMaterializedView *>(table.get()))
{
if (auto inner_table = mv->tryGetTargetTable())
if (inner_table)
res.pipeline.addStorageHolder(inner_table);
}
return res;
}