You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@orc.apache.org by lu...@apache.org on 2026/08/13 09:28:12 UTC
(orc) branch main updated: ORC-2215: [C++] Fix use-after-free bug in timezone cache
This is an automated email from the ASF dual-hosted git repository.
luffy-zh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/orc.git
The following commit(s) were added to refs/heads/main by this push:
new 7f1dd1821 ORC-2215: [C++] Fix use-after-free bug in timezone cache
7f1dd1821 is described below
commit 7f1dd1821bac79ccd017ca73d082bcaba1055b86
Author: Ben Hannel <be...@snowflake.com>
AuthorDate: Thu Aug 13 17:27:41 2026 +0800
ORC-2215: [C++] Fix use-after-free bug in timezone cache
`getTimezoneByFilename` overwrites 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 https://github.com/apache/orc/commit/481aadf4bbcaf2ebc36e862dafa6a628b94283d2 (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:
```
$ cmake /tmp/orc-tz-race \
-DBUILD_JAVA=OFF -DBUILD_TOOLS=OFF -DBUILD_CPP_TESTS=ON \
-DCMAKE_BUILD_TYPE=RelWithDebInfo \
-DCMAKE_CXX_FLAGS="-fsanitize=address" \
-DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address"
$ make -j$(nproc) orc-test
$ ASAN_OPTIONS=detect_leaks=0 /tmp/orc-tz-race-build-asan/c++/test/orc-test --gtest_filter='TimestampAliasCacheEviction*'
...
[ RUN ] TimestampAliasCacheEviction.readerSurvivesAliasCacheEviction
=================================================================
==917813==ERROR: AddressSanitizer: heap-use-after-free on address 0xffff94413b30 at pc 0x000000e65bf4 bp 0xfffff4e1ce60 sp 0xfffff4e1ce50
READ of size 8 at 0xffff94413b30 thread T0
#0 0xe65bf0 in orc::TimestampColumnReader::next(orc::ColumnVectorBatch&, unsigned long, char*) /tmp/orc-tz-race/c++/src/ColumnReader.cc:336
#1 0xe65208 in void orc::StructColumnReader::nextInternal<false>(orc::ColumnVectorBatch&, unsigned long, char*) /tmp/orc-tz-race/c++/src/ColumnReader.cc:868
#2 0xe65208 in orc::StructColumnReader::next(orc::ColumnVectorBatch&, unsigned long, char*) /tmp/orc-tz-race/c++/src/ColumnReader.cc:849
#3 0xc9a700 in orc::RowReaderImpl::next(orc::ColumnVectorBatch&) /tmp/orc-tz-race/c++/src/Reader.cc:1417
#4 0xc9a700 in orc::RowReaderImpl::next(orc::ColumnVectorBatch&) /tmp/orc-tz-race/c++/src/Reader.cc:1392
#5 0xd28f0c in orc::TimestampAliasCacheEviction_readerSurvivesAliasCacheEviction_Test::TestBody() /tmp/orc-tz-race/c++/test/TestWriter.cc:2708
#6 0x14bdb20 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2664
#7 0x14bdb20 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2700
#8 0x14864b0 in testing::Test::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2739
#9 0x1486ff0 in testing::Test::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2730
#10 0x1486ff0 in testing::TestInfo::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2885
#11 0x1489cc0 in testing::TestSuite::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:3063
#12 0x14a2cc0 in testing::internal::UnitTestImpl::RunAllTests() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:6054
#13 0x1487464 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2664
#14 0x1487464 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2700
#15 0x1487464 in testing::UnitTest::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:5594
#16 0x44ee74 in RUN_ALL_TESTS() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/include/gtest/gtest.h:2334
#17 0x44ee74 in main /tmp/orc-tz-race/c++/test/TestDriver.cc:30
#18 0xffff9882453c in __libc_start_call_main (/lib64/libc.so.6+0x2753c)
#19 0xffff98824614 in __libc_start_main_alias_2 (/lib64/libc.so.6+0x27614)
#20 0x49562c in _start (/tmp/orc-tz-race-build-asan/c++/test/orc-test+0x49562c)
0xffff94413b30 is located 16 bytes inside of 72-byte region [0xffff94413b20,0xffff94413b68)
freed by thread T0 here:
#0 0xffff98d50ecc in operator delete(void*, unsigned long) (/lib64/libasan.so.6+0xafecc)
#1 0xf95444 in __gnu_cxx::new_allocator<std::_Sp_counted_ptr_inplace<orc::LazyTimezone, std::allocator<orc::LazyTimezone>, (__gnu_cxx::_Lock_policy)2> >::deallocate(std::_Sp_counted_ptr_inplace<orc::LazyTimezone, std::allocator<orc::LazyTimezone>, (__gnu_cxx::_Lock_policy)2>*, unsigned long) /usr/include/c++/11/ext/new_allocator.h:145
#2 0xf95444 in std::allocator_traits<std::allocator<std::_Sp_counted_ptr_inplace<orc::LazyTimezone, std::allocator<orc::LazyTimezone>, (__gnu_cxx::_Lock_policy)2> > >::deallocate(std::allocator<std::_Sp_counted_ptr_inplace<orc::LazyTimezone, std::allocator<orc::LazyTimezone>, (__gnu_cxx::_Lock_policy)2> >&, std::_Sp_counted_ptr_inplace<orc::LazyTimezone, std::allocator<orc::LazyTimezone>, (__gnu_cxx::_Lock_policy)2>*, unsigned long) /usr/include/c++/11/bits/alloc_traits.h:496
#3 0xf95444 in std::__allocated_ptr<std::allocator<std::_Sp_counted_ptr_inplace<orc::LazyTimezone, std::allocator<orc::LazyTimezone>, (__gnu_cxx::_Lock_policy)2> > >::~__allocated_ptr() /usr/include/c++/11/bits/allocated_ptr.h:74
#4 0xf95444 in std::_Sp_counted_ptr_inplace<orc::LazyTimezone, std::allocator<orc::LazyTimezone>, (__gnu_cxx::_Lock_policy)2>::_M_destroy() /usr/include/c++/11/bits/shared_ptr_base.h:538
#5 0xf9325c in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() /usr/include/c++/11/bits/shared_ptr_base.h:184
#6 0xf9325c in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() /usr/include/c++/11/bits/shared_ptr_base.h:161
#7 0xf9325c in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() /usr/include/c++/11/bits/shared_ptr_base.h:705
#8 0xf9325c in std::__shared_ptr<orc::Timezone, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() /usr/include/c++/11/bits/shared_ptr_base.h:1154
#9 0xf9325c in std::enable_if<std::__sp_compatible_with<orc::LazyTimezone*, orc::Timezone*>::value, std::__shared_ptr<orc::Timezone, (__gnu_cxx::_Lock_policy)2>&>::type std::__shared_ptr<orc::Timezone, (__gnu_cxx::_Lock_policy)2>::operator=<orc::LazyTimezone>(std::__shared_ptr<orc::LazyTimezone, (__gnu_cxx::_Lock_policy)2>&&) /usr/include/c++/11/bits/shared_ptr_base.h:1258
#10 0xf9325c in std::enable_if<std::is_assignable<std::__shared_ptr<orc::Timezone, (__gnu_cxx::_Lock_policy)2>&, std::shared_ptr<orc::LazyTimezone> >::value, std::shared_ptr<orc::Timezone>&>::type std::shared_ptr<orc::Timezone>::operator=<orc::LazyTimezone>(std::shared_ptr<orc::LazyTimezone>&&) /usr/include/c++/11/bits/shared_ptr.h:393
#11 0xf9325c in orc::getTimezoneByFilename(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /tmp/orc-tz-race/c++/src/Timezone.cc:776
#12 0xf94144 in orc::getTimezoneByName(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /tmp/orc-tz-race/c++/src/Timezone.cc:809
#13 0xd28e5c in orc::TimestampAliasCacheEviction_readerSurvivesAliasCacheEviction_Test::TestBody() /tmp/orc-tz-race/c++/test/TestWriter.cc:2705
#14 0x14bdb20 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2664
#15 0x14bdb20 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2700
#16 0x14864b0 in testing::Test::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2739
#17 0x1486ff0 in testing::Test::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2730
#18 0x1486ff0 in testing::TestInfo::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2885
#19 0x1489cc0 in testing::TestSuite::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:3063
#20 0x14a2cc0 in testing::internal::UnitTestImpl::RunAllTests() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:6054
#21 0x1487464 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2664
#22 0x1487464 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:2700
#23 0x1487464 in testing::UnitTest::Run() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/src/gtest.cc:5594
#24 0x44ee74 in RUN_ALL_TESTS() /tmp/orc-tz-race-build-asan/_deps/gtest-src/googletest/include/gtest/gtest.h:2334
#25 0x44ee74 in main /tmp/orc-tz-race/c++/test/TestDriver.cc:30
#26 0xffff9882453c in __libc_start_call_main (/lib64/libc.so.6+0x2753c)
#27 0xffff98824614 in __libc_start_main_alias_2 (/lib64/libc.so.6+0x27614)
#28 0x49562c in _start (/tmp/orc-tz-race-build-asan/c++/test/orc-test+0x49562c)
```
I cannot file a tracking ticket because the [ORC Jira instance](https://issues.apache.org/jira/projects/ORC/issues) does not allow public signup.
Generated-by: Claude Sonnet 4.6, but a human (me) reviewed everything throughly before putting up this pR.
Closes #2702 from sfc-gh-bhannel/bhannel-timezone-cache-race.
Authored-by: Ben Hannel <be...@snowflake.com>
Signed-off-by: shouzhi <sh...@alibaba-inc.com>
---
c++/src/Timezone.cc | 7 ++++++-
c++/test/TestWriter.cc | 41 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/c++/src/Timezone.cc b/c++/src/Timezone.cc
index 6c3f58c07..b9499bd6d 100644
--- a/c++/src/Timezone.cc
+++ b/c++/src/Timezone.cc
@@ -773,7 +773,12 @@ namespace orc {
std::string newfilename(dir);
newfilename += "/";
newfilename += it->second;
- timezoneCache[newfilename] = std::make_shared<LazyTimezone>(newfilename);
+ // Only insert if absent: overwriting an existing entry destroys the
+ // LazyTimezone while raw pointers in live TimestampColumnReaders still
+ // reference it, causing a use-after-free (SIGSEGV) on the next read.
+ if (timezoneCache.find(newfilename) == timezoneCache.end()) {
+ timezoneCache[newfilename] = std::make_shared<LazyTimezone>(newfilename);
+ }
timezoneCache[filename] = timezoneCache[newfilename];
}
return *timezoneCache[filename].get();
diff --git a/c++/test/TestWriter.cc b/c++/test/TestWriter.cc
index 5827bcdbc..764ead1c8 100644
--- a/c++/test/TestWriter.cc
+++ b/c++/test/TestWriter.cc
@@ -2648,6 +2648,47 @@ namespace orc {
}
}
+ TEST(TimestampAliasCacheEviction, readerSurvivesAliasCacheEviction) {
+ MemoryOutputStream memStream(DEFAULT_MEM_STREAM_SIZE);
+ MemoryPool* pool = getDefaultPool();
+ std::unique_ptr<Type> type(Type::buildTypeFromString("struct<ts:timestamp>"));
+
+ // Write 2000 rows so that reading with a 1024-row batch requires two next() calls.
+ const uint64_t rowCount = 2000;
+ std::unique_ptr<Writer> writer =
+ createWriter(64 * 1024 * 1024, 64 * 1024, 64 * 1024, CompressionKind_ZLIB, *type, pool,
+ &memStream, FileVersion::v_0_12(), 0, "America/New_York");
+ std::unique_ptr<ColumnVectorBatch> batch = writer->createRowBatch(rowCount);
+ StructVectorBatch* structBatch = dynamic_cast<StructVectorBatch*>(batch.get());
+ TimestampVectorBatch* tsBatch = dynamic_cast<TimestampVectorBatch*>(structBatch->fields[0]);
+ for (uint64_t i = 0; i < rowCount; ++i) {
+ tsBatch->data[i] = static_cast<int64_t>(i * 3600);
+ tsBatch->nanoseconds[i] = 0;
+ }
+ structBatch->numElements = rowCount;
+ tsBatch->numElements = rowCount;
+ writer->add(*batch);
+ writer->close();
+
+ auto inStream = std::make_unique<MemoryInputStream>(memStream.getData(), memStream.getLength());
+ std::unique_ptr<Reader> reader = createReader(pool, std::move(inStream));
+ std::unique_ptr<RowReader> rowReader = createRowReader(reader.get(), "GMT");
+ ASSERT_EQ(rowCount, reader->getNumberOfRows());
+
+ std::unique_ptr<ColumnVectorBatch> readBatch = rowReader->createRowBatch(1024);
+
+ // First next() opens the stripe, constructs TimestampColumnReader, and stores
+ // writerTimezone_ = &getTimezoneByName("America/New_York").
+ ASSERT_TRUE(rowReader->next(*readBatch));
+ ASSERT_EQ(1024u, readBatch->numElements);
+
+ // Populate aliases in the timezone cache
+ (void)getTimezoneByName("US/Eastern");
+
+ // Verify that writerTimezone_ is still a valid pointer
+ EXPECT_TRUE(rowReader->next(*readBatch));
+ }
+
std::vector<TestParams> testParams = {{FileVersion::v_0_11(), true},
{FileVersion::v_0_11(), false},
{FileVersion::v_0_12(), false},