ORC-2215: [C++] Fix use-after-free bug in timezone cache - #2702
Open
sfc-gh-bhannel wants to merge 2 commits into
Open
ORC-2215: [C++] Fix use-after-free bug in timezone cache#2702sfc-gh-bhannel wants to merge 2 commits into
sfc-gh-bhannel wants to merge 2 commits into
Conversation
…ntry
getTimezoneByName("US/Eastern") follows the alias link to "America/New_York"
and unconditionally assigns a new shared_ptr into timezoneCache["America/New_York"],
even when that entry already exists. The old shared_ptr refcount drops to zero
inside timezone_mutex, destroying the LazyTimezone while TimestampColumnReader
holds a raw Timezone* (writerTimezone_) that still references it.
Reproduce without the fix (single-threaded, no sanitizer required):
bazel test //c++/test:orc-test \
--test_filter='TimestampAliasCacheEviction*' \
--test_env=ASAN_OPTIONS=detect_leaks=0
# Expected under ASAN: heap-use-after-free in orc::TimestampColumnReader::next
# Expected without ASAN: SIGSEGV on second rowReader->next() call
Fix: guard the cache insertion with find() so alias lookups only populate
a missing canonical entry and never replace an existing one.
Adds TimestampAliasCacheEviction.readerSurvivesAliasCacheEviction in
TestWriter.cc: writes an ORC file with writer timezone "America/New_York",
reads the first batch (initializing writerTimezone_), then triggers the
"US/Eastern" alias lookup to evict the cache entry, then reads the second
batch — without the fix this dereferences freed memory.
sfc-gh-bhannel
changed the title
sfc-gh-bhannel
marked this pull request as ready for review
July 28, 2026 21:42
ffacs
changed the title
wgtmac
approved these changes
Jul 29, 2026
Member
|
Thanks for fixing this, @sfc-gh-bhannel. It seems that this PR is linked to an old closed JIRA issue. Could you please create a new one? |
Author
|
See the PR description - I don't have the necessary permissions to create a new jira issue. |
dongjoon-hyun
approved these changes
Jul 29, 2026
dongjoon-hyun
changed the title
dongjoon-hyun
changed the title
Member
|
I created ORC-2215 for @sfc-gh-bhannel . |
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.
getTimezoneByFilenameoverwrites existing entries, potentially causing causing callers which already took a pointer to that entry to end up with a dangling pointer. I think the best fix is to only insert for new keys, since if the key already exists, the value is correct - this is a cache for a pure function. I think the issue may have been introduced in 481aadf (ORC-2011).I wrote a new unit test for the issue,
TimestampAliasCacheEviction.readerSurvivesAliasCacheEviction. Without the fix, this test segfaults due to a dangling pointer. ASAN gives good diagnostics:I cannot file a tracking ticket because the ORC Jira instance does not allow public signup.
Generated-by: Claude Sonnet 4.6, but a human (me) reviewed everything throughly before putting up this pR.