Harden lock_configuration and enable_external_access - #5
Merged
leo-altertable merged 4 commits intoAug 28, 2026
Merged
leo-altertable merged 4 commits into
leo-altertable merged 4 commits into
Conversation
utay
approved these changes
Aug 28, 2026
redox
pushed a commit
that referenced
this pull request
Sep 29, 2026
<details>
<summary>WARNING: ThreadSanitizer: data race; Read of size 8</summary>
```c++
Filters: test/sql/show_select/summarize_subquery.test
[0/1] (0%): test/sql/show_select/summarize_subquery.test==================
WARNING: ThreadSanitizer: data race (pid=80064)
Read of size 8 at 0x00010708e758 by thread T10:
#0 duckdb::ArenaAllocator::AlignNext() <null> (libduckdb.dylib:arm64+0x2a37af0)
#1 std::__1::vector<double, duckdb::arena_stl_allocator<double>>::reserve(unsigned long) vector.h:1100 (libduckdb.dylib:arm64+0x3184418)
#2 duckdb_tdigest::TDigest::updateCumulative() t_digest.hpp:538 (libduckdb.dylib:arm64+0x31819cc)
#3 duckdb_tdigest::TDigest::process() t_digest.hpp:583 (libduckdb.dylib:arm64+0x31816d8)
#4 void duckdb::(anonymous namespace)::ApproxQuantileScalarOperation::Finalize<long long, duckdb::(anonymous namespace)::ApproxQuantileState>(duckdb::(ano nymous namespace)::ApproxQuantileState&, long long&, duckdb::AggregateFinalizeData&) approximate_quantile.cpp:162 (libduckdb.dylib:arm64+0x318d540)
#5 void duckdb::AggregateFunction::StateFinalize<duckdb::(anonymous namespace)::ApproxQuantileState, long long, duckdb::(anonymous namespace)::ApproxQuant ileScalarOperation>(duckdb::Vector&, duckdb::AggregateFinalizeInputData&, duckdb::Vector&, unsigned long long, unsigned long long) aggregate_function.hpp:822 (libduckdb.dylib:arm64+0x318ca64)
#6 duckdb::RowOperations::FinalizeStates(duckdb::RowOperationsState&, duckdb::TupleDataLayout&, duckdb::Vector&, duckdb::DataChunk&, unsigned long long) r ow_aggregate.cpp:182 (libduckdb.dylib:arm64+0x166f848)
#7 duckdb::RadixHTLocalSourceState::Scan(duckdb::RadixHTGlobalSinkState&, duckdb::RadixHTGlobalSourceState&, duckdb::DataChunk&) <null> (libduckdb.dylib:a rm64+0x2458dac)
#8 duckdb::RadixHTLocalSourceState::ExecuteTask(duckdb::RadixHTGlobalSinkState&, duckdb::RadixHTGlobalSourceState&, duckdb::DataChunk&) <null> (libduckdb. dylib:arm64+0x2457ea0)
duckdb#9 duckdb::RadixPartitionedHashTable::GetData(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::GlobalSinkState&, duckdb::OperatorSourceInput&) const <null> (libduckdb.dylib:arm64+0x2459624)
duckdb#10 duckdb::PhysicalHashAggregate::GetDataInternal(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::OperatorSourceInput&) const physical_hash_aggreg ate.cpp:978 (libduckdb.dylib:arm64+0x21305cc)
duckdb#11 duckdb::PhysicalOperator::GetData(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::OperatorSourceInput&) const <null> (libduckdb.dylib:arm64+0x2 448dac)
...
Previous write of size 8 at 0x00010708e758 by thread T7:
#0 std::__1::vector<double, duckdb::arena_stl_allocator<double>>::reserve(unsigned long) vector.h:1100 (libduckdb.dylib:arm64+0x31844a0)
#1 duckdb_tdigest::TDigest::updateCumulative() t_digest.hpp:538 (libduckdb.dylib:arm64+0x31819cc)
#2 duckdb_tdigest::TDigest::process() t_digest.hpp:583 (libduckdb.dylib:arm64+0x31816d8)
#3 void duckdb::(anonymous namespace)::ApproxQuantileScalarOperation::Finalize<long long, duckdb::(anonymous namespace)::ApproxQuantileState>(duckdb::(ano nymous namespace)::ApproxQuantileState&, long long&, duckdb::AggregateFinalizeData&) approximate_quantile.cpp:162 (libduckdb.dylib:arm64+0x318d540)
#4 void duckdb::AggregateFunction::StateFinalize<duckdb::(anonymous namespace)::ApproxQuantileState, long long, duckdb::(anonymous namespace)::ApproxQuant ileScalarOperation>(duckdb::Vector&, duckdb::AggregateFinalizeInputData&, duckdb::Vector&, unsigned long long, unsigned long long) aggregate_function.hpp:822 (libduckdb.dylib:arm64+0x318ca64)
#5 duckdb::RowOperations::FinalizeStates(duckdb::RowOperationsState&, duckdb::TupleDataLayout&, duckdb::Vector&, duckdb::DataChunk&, unsigned long long) r ow_aggregate.cpp:182 (libduckdb.dylib:arm64+0x166f848)
#6 duckdb::RadixHTLocalSourceState::Scan(duckdb::RadixHTGlobalSinkState&, duckdb::RadixHTGlobalSourceState&, duckdb::DataChunk&) <null> (libduckdb.dylib:a rm64+0x2458dac)
#7 duckdb::RadixHTLocalSourceState::ExecuteTask(duckdb::RadixHTGlobalSinkState&, duckdb::RadixHTGlobalSourceState&, duckdb::DataChunk&) <null> (libduckdb. dylib:arm64+0x2457ea0)
#8 duckdb::RadixPartitionedHashTable::GetData(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::GlobalSinkState&, duckdb::OperatorSourceInput&) const <null> (libduckdb.dylib:arm64+0x2459624)
duckdb#9 duckdb::PhysicalHashAggregate::GetDataInternal(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::OperatorSourceInput&) const physical_hash_aggrega te.cpp:978 (libduckdb.dylib:arm64+0x21305cc)
duckdb#10 duckdb::PhysicalOperator::GetData(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::OperatorSourceInput&) const <null> (libduckdb.dylib:arm64+0x2 448dac)
...
Location is heap block of size 56 at 0x00010708e740 allocated by main thread:
#0 operator new(unsigned long) <null> (libclang_rt.tsan_osx_dynamic.dylib:arm64e+0x91650)
#1 duckdb::ArenaAllocator::AllocateNewBlock(unsigned long long) <null> (libduckdb.dylib:arm64+0x2a37830)
#2 std::__1::vector<duckdb_tdigest::Centroid, duckdb::arena_stl_allocator<duckdb_tdigest::Centroid>>::reserve(unsigned long) vector.h:1100 (libduckdb.dyli b:arm64+0x3180da0)
#3 void duckdb::(anonymous namespace)::ApproxQuantileOperation::Operation<long long, duckdb::(anonymous namespace)::ApproxQuantileState, duckdb::(anonymou s namespace)::ApproxQuantileScalarOperation>(duckdb::(anonymous namespace)::ApproxQuantileState&, long long const&, duckdb::AggregateUnaryInput&) approximate_ quantile.cpp:122 (libduckdb.dylib:arm64+0x318ccd4)
#4 void duckdb::AggregateFunction::UnaryScatterUpdate<duckdb::(anonymous namespace)::ApproxQuantileState, long long, duckdb::(anonymous namespace)::Approx QuantileScalarOperation>(duckdb::Vector*, duckdb::AggregateInputData&, unsigned long long, duckdb::Vector&, unsigned long long) aggregate_function.hpp:782 (li bduckdb.dylib:arm64+0x318c454)
#5 duckdb::RowOperations::UpdateStates(duckdb::RowOperationsState&, duckdb::AggregateObject&, duckdb::Vector&, duckdb::DataChunk&, unsigned long long, duc kdb::optional_ptr<duckdb::ClusteredAggr const, true>) aggregate_function.hpp (libduckdb.dylib:arm64+0x166ed24)
#6 duckdb::GroupedAggregateHashTable::UpdateAggregates(duckdb::DataChunk&, duckdb::vector<unsigned long long, false, std::__1::allocator<unsigned long lon g>> const&, unsigned long long, bool) <null> (libduckdb.dylib:arm64+0x240d234)
#7 duckdb::GroupedAggregateHashTable::AddChunk(duckdb::DataChunk&, duckdb::Vector&, duckdb::DataChunk&, duckdb::vector<unsigned long long, false, std::__1 ::allocator<unsigned long long>> const&) <null> (libduckdb.dylib:arm64+0x240f000)
#8 duckdb::GroupedAggregateHashTable::AddChunk(duckdb::DataChunk&, duckdb::DataChunk&, duckdb::vector<unsigned long long, false, std::__1::allocator<unsig ned long long>> const&) <null> (libduckdb.dylib:arm64+0x240cb30)
duckdb#9 duckdb::RadixPartitionedHashTable::Sink(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::OperatorSinkInput&, duckdb::DataChunk&, duckdb::vector<u nsigned long long, false, std::__1::allocator<unsigned long long>> const&) const <null> (libduckdb.dylib:arm64+0x2455190)
duckdb#10 duckdb::PhysicalHashAggregate::Sink(duckdb::ExecutionContext&, duckdb::DataChunk&, duckdb::OperatorSinkInput&) const physical_hash_aggregate.cpp:466 ( libduckdb.dylib:arm64+0x212bafc)
SUMMARY: ThreadSanitizer: data race (libduckdb.dylib:arm64+0x2a37af0) in duckdb::ArenaAllocator::AlignNext()+0x2c
==================
```
</details>
`SUMMARIZE` calculates three `approx_quantile` aggregates (`q25`, `q50`,
`q75`). Each aggregate owns a TDigest whose vectors use the hash table’s
`ArenaAllocator`.
Two hash-aggregate worker threads concurrently finalized separate
TDigest states:
- Both called `TDigest::process()` → `updateCumulative()` →
`vector::reserve()`.
- The states shared the same non-thread-safe `ArenaAllocator`.
- One thread read allocator position metadata in
`ArenaAllocator::AlignNext()` while another wrote it.
- The shared arena had originally been allocated during
`approx_quantile` aggregation on the main thread.
Result: a race in arena allocation during parallel `approx_quantile`
finalization, reported at `ArenaAllocator::AlignNext()`.
It was intermittent because `reserve()` only occurs when vector capacity
must grow, and the worker calls had to overlap closely enough.
The fix reserves the cumulative buffer and sufficient centroid merge
capacity when the TDigest is constructed and its allocator is still used
by only one thread. Parallel finalization then operates entirely within
already allocated buffers and no longer mutates the shared arena.
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
3 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.
There are 2 changes that should help with hardening duckdb against malicious attacks:
Piggy pack on
lock_configurationwhen interacting withSECRET:lock_configurationnow also lock theCREATE/UPDATE/DROPof secrets.Be stricter on
allow_external_access: this is enforced inside duckdb itself but extensions have to do it themselves. This prevent any attach that is advertised asexternal_storagewhen theallow_external_accessis set tofalse. This shouldn't change much, considering extensions likepostgreswere already checking this and refusing to attach in that case. This does tighten for ducklake and snowflake that did not do it.