fix(storage): pin Arrow registry entries with shared_ptr to prevent use-after-free - #938
Merged
Conversation
…se-after-free getArrowData()/getArrowRelData() returned raw pointers into the process-wide registry after the lock_guard went out of scope, so a concurrent DROP TABLE / unregisterArrowData() (unordered_map::erase) left callers with dangling pointers (CWE-416/667). Store registry payloads as shared_ptr and return a shared_ptr copy taken under the mutex so callers pin the buffers past the lock scope. ArrowNodeTable/ArrowRelTable keep a pin plus shallow non-owning views; erasing the map entry now only drops the registry's reference. Fixes #933.
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
1 participant
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.
Fixes #933.
Problem
getArrowData()/getArrowRelData()insrc/storage/table/arrow_table_support.cppreturned raw pointers into the process-wide registry maps after thelock_guardwent out of scope. The mutex protected individual map operations, not the lifetime of data a caller already held a pointer into. A concurrentDROP TABLE→unregisterArrowData()→unordered_map::eraseleft the other thread with a dangling pointer (CWE-416 use-after-free, CWE-667 improper locking).Fix
shared_ptr<ArrowTableData>/shared_ptr<ArrowRelTableData>.getArrowData()/getArrowRelData()return ashared_ptrcopy taken under the mutex, so callers pin the buffers past the lock scope; erasing the map entry only drops the registry's reference.ArrowNodeTable/ArrowRelTableeach hold a pin plus shallow non-owning views into the pinned data; table destructors still callunregisterArrowData()for cleanup, now safe under concurrency.StorageManagercreation path uses the pinning getters, closing the get-then-copy TOCTOU.Testing
make releaseclean.build/release/test/api/api_test --gtest_filter="*Arrow*:*arrow*"→ 141 passed, 21 skipped (pre-existing skips).