From 77d9ed7f63f642cbefc86ad9a01e9433382a2663 Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Thu, 11 Jun 2026 22:32:11 -0700 Subject: [PATCH] Use _exit(1) instead of exit(1) after DB is open to avoid UAF (#14850) Summary: When `FinishInitDb()` or `Open()` calls `exit(1)` after the DB has been opened, background compaction/flush threads are still running. `exit()` triggers static object destruction (including the `KillPoint` singleton and its `rocksdb_kill_exclude_prefixes` vector) while those threads are still accessing them via `TestKillRandom()`, causing a heap-use-after-free detected by ASAN. This became more likely to trigger after the multi-DB support commit (3d0d60101e7f) which runs `RunStressTestImpl` on worker threads, making the race window larger when one DB fails initialization while other DBs background threads are active. The fix replaces `exit(1)` in all error paths that fire after the DB has been opened with a wrapper around `_exit(1)`. `_exit()` terminates immediately without running atexit handlers or destroying static objects, avoiding the race with background threads. Also updated CLAUDE.md to help get cross-platform compatibility right the first time. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14850 Reviewed By: hx235 Differential Revision: D108298839 Pulled By: pdillinger fbshipit-source-id: 8e87fcb259c273e2be5eb26b4eaf6009f5b998f1 --- CLAUDE.md | 23 +++++++++++++++++++++++ db_stress_tool/db_stress_test_base.cc | 8 ++++---- port/port_posix.cc | 2 ++ port/port_posix.h | 10 ++++++++++ port/win/port_win.cc | 2 ++ port/win/port_win.h | 2 ++ 6 files changed, 43 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8fb02b182e..f2e83d3b43 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -279,6 +279,29 @@ phantom bug. `dynamic_cast` in debug builds, plain `static_cast` in release). * Unit tests (`*_test.cc`) are built in debug mode with RTTI enabled. +### Cross-platform / portability +Local `make` only exercises Linux with GCC/Clang, but CI +(`.github/workflows/pr-jobs.yml` and `nightly.yml`) gates on a much wider +matrix, so portability breaks are invisible locally until CI fails. Code must +build (and where noted, run tests) across: + +| Axis | Must support | +|------|--------------| +| OS | Linux (x86_64 + ARM), macOS, Windows | +| Compiler | GCC, Clang (libstdc++ **and** libc++), AppleClang, **MSVC (VS2022)**, MinGW (Linux cross-compile, build-only, no gflags) | +| Build system | Make, CMake, and BUCK (internal) -- keep all in sync (see "Build system" above) | +| Config | release (`-fno-rtti`), `ASSERT_STATUS_CHECKED`, ASAN/UBSAN/TSAN, folly, unity build, JNI/Java | + +Treat these as constraints to satisfy and infer the specifics from them before +adding any system header, libc call, or compiler-specific construct. The most +common trap: anything that compiles under GCC/Clang on Linux but not under +**MSVC/MinGW** -- e.g. unguarded POSIX-only headers/functions (``, +``, `getpid`, `_exit`, ...) or GCC/Clang extensions +(`__attribute__`, `__builtin_*`, VLAs, `alloca`). Prefer the `port::`/`Env` +abstractions; otherwise guard with `#ifdef OS_WIN` (POSIX `` -> +Windows ``). Because libc++ is also tested, include what you use +rather than relying on libstdc++ transitive includes. + ### Unit Test * After all of the unit tests are added, review them and try to extract common reusable utility functions to reduce code duplication due to copy past between diff --git a/db_stress_tool/db_stress_test_base.cc b/db_stress_tool/db_stress_test_base.cc index 102394d4e2..6bcf3db9c3 100644 --- a/db_stress_tool/db_stress_test_base.cc +++ b/db_stress_tool/db_stress_test_base.cc @@ -752,7 +752,7 @@ void StressTest::FinishInitDb(SharedState* shared) { if (!s.ok()) { fprintf(stderr, "Error restoring historical expected values: %s\n", s.ToString().c_str()); - exit(1); + port::ImmediateExit(1); } } if (FLAGS_use_txn && !FLAGS_use_optimistic_txn) { @@ -785,7 +785,7 @@ void StressTest::TrackExpectedState(SharedState* shared) { if (!s.ok()) { fprintf(stderr, "Error enabling history tracing: %s\n", s.ToString().c_str()); - exit(1); + port::ImmediateExit(1); } } } @@ -4543,7 +4543,7 @@ void StressTest::Open(SharedState* shared, bool reopen) { if (!s.ok()) { fprintf(stderr, "open error: %s\n", s.ToString().c_str()); - exit(1); + port::ImmediateExit(1); } if (db_->GetLatestSequenceNumber() < shared->GetPersistedSeqno()) { @@ -4552,7 +4552,7 @@ void StressTest::Open(SharedState* shared, bool reopen) { "did not recover to the persisted " "sequence number %" PRIu64 " from last DB session\n", db_->GetLatestSequenceNumber(), shared->GetPersistedSeqno()); - exit(1); + port::ImmediateExit(1); } } diff --git a/port/port_posix.cc b/port/port_posix.cc index 5fd07db991..e1fd592ebf 100644 --- a/port/port_posix.cc +++ b/port/port_posix.cc @@ -232,6 +232,8 @@ void Crash(const std::string& srcfile, int srcline) { kill(getpid(), SIGTERM); } +void ImmediateExit(int code) { _exit(code); } + int GetMaxOpenFiles() { #if defined(RLIMIT_NOFILE) struct rlimit no_files_limit; diff --git a/port/port_posix.h b/port/port_posix.h index 36bc186271..745af51745 100644 --- a/port/port_posix.h +++ b/port/port_posix.h @@ -230,6 +230,16 @@ void cacheline_aligned_free(void* memblock); void Crash(const std::string& srcfile, int srcline); +// Terminates the process immediately with the given exit code, bypassing the +// usual shutdown path: no atexit handlers run and no static/global destructors +// are invoked (POSIX _exit(); same on Windows). This is the safe way to abort +// from a process that still has background threads running (e.g. RocksDB's +// compaction/flush threads). A normal exit() would tear down static objects +// those threads are concurrently accessing, causing cross-thread +// use-after-free. Use this instead of exit() once a DB has been opened and +// background threads may be live. +[[noreturn]] void ImmediateExit(int code); + int GetMaxOpenFiles(); extern const size_t kPageSize; diff --git a/port/win/port_win.cc b/port/win/port_win.cc index fb4939a4ca..b59d6e5f97 100644 --- a/port/win/port_win.cc +++ b/port/win/port_win.cc @@ -266,6 +266,8 @@ void Crash(const std::string& srcfile, int srcline) { abort(); } +void ImmediateExit(int code) { _exit(code); } + int GetMaxOpenFiles() { return -1; } // Assume 4KB page size diff --git a/port/win/port_win.h b/port/win/port_win.h index 700959c4db..0b6f794a45 100644 --- a/port/win/port_win.h +++ b/port/win/port_win.h @@ -307,6 +307,8 @@ inline void* pthread_getspecific(pthread_key_t key) { int truncate(const char* path, int64_t length); int Truncate(std::string path, int64_t length); void Crash(const std::string& srcfile, int srcline); +// See ImmediateExit in port/port_posix.h for documentation. +[[noreturn]] void ImmediateExit(int code); int GetMaxOpenFiles(); std::string utf16_to_utf8(const std::wstring& utf16); std::wstring utf8_to_utf16(const std::string& utf8);