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
This commit is contained in:
Peter Dillinger
2026-06-11 22:32:11 -07:00
committed by meta-codesync[bot]
parent 6d4a8144e0
commit 77d9ed7f63
6 changed files with 43 additions and 4 deletions
+23
View File
@@ -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 (`<unistd.h>`,
`<sys/*.h>`, `getpid`, `_exit`, ...) or GCC/Clang extensions
(`__attribute__`, `__builtin_*`, VLAs, `alloca`). Prefer the `port::`/`Env`
abstractions; otherwise guard with `#ifdef OS_WIN` (POSIX `<unistd.h>` ->
Windows `<process.h>`). 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
+4 -4
View File
@@ -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);
}
}
+2
View File
@@ -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;
+10
View File
@@ -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;
+2
View File
@@ -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
+2
View File
@@ -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);