From 93fa9eb1a6a5ae9f88abfd40d8ede5a71fa8d38b Mon Sep 17 00:00:00 2001 From: Leon Schuermann Date: Mon, 10 Aug 2026 10:54:22 -0700 Subject: [PATCH] litebox_shim_linux: fix race in prepare_for_exit child TID write This fixes a bug in `prepare_for_exit` in the LiteBox Linux shim. When a thread exits, it may indicate this by writing its child TID to a location in user memory. However, prior to this, `detach_from_process` is run, which decrements `nr_threads`. This is a signal to other tasks that this task no longer holds any references to process memory, and thus that the memory is ready to be deallocated. In the case of, e.g., an `exec` system call, this in turn introduces a "confused deputy" style write where the LiteBox kernel will either attempt to write to some unmapped page (caught by the exception handlers and silently turned into a no-op), or write into and corrupt the new process' memory. The Race Condition ------------------ +----+------------------------------+------------------------------+ | # | A (main / exec-ing thread) | B (worker / dying thread) | +----+------------------------------+------------------------------+ | 1 | pthread_create(), passing | | | | CLONE_CHILD_CLEARTID = | | | | &pd->tid | | +----+------------------------------+------------------------------+ | 2 | | starts running | | | | - clear_child_tid = addr | | | | inside B's stack | | | | - nr_threads -> 2 | +----+------------------------------+------------------------------+ | 3 | | recv(fd, buf, ...), blocks | +----+------------------------------+------------------------------+ | 4 | execve("/new/program") | blocked | +----+------------------------------+------------------------------+ | 5 | kill_other_threads() | blocked | | | - sets is_exiting, | | | | interrupts B | | | | - waits nr_threads == 1 | | +----+------------------------------+------------------------------+ | 6 | waiting | recv returns EINTR | | | | - drops Task -> | | | | prepare_for_exit() | +----+------------------------------+------------------------------+ | 7 | waiting | detach_from_process() | | | | - nr_threads 2 -> 1, | | | | wakes A | | | | - DECLARED DONE, but | | | | guest write pending | +----+------------------------------+------------------------------+ | 8 | kill_other_threads() | | | | RETURNS | | | | - process assumed | | | | quiesced | | +----+------------------------------+------------------------------+ | 9 | | descheduled | | | | - still owes write to | | | | clear_child_tid | +----+------------------------------+------------------------------+ | 10 | release_memory() UNMAP | descheduled | | | - destroys all old | | | | mappings, incl. B's | | | | stack | | | | - ranges immediately | | | | reusable, no | | | | quarantine | | +----+------------------------------+------------------------------+ | 11 | load_program() REMAP | descheduled | | | - new ELF/stack/heap | | | | from same pool | | | | - may land on the | | | | freed range | | +----+------------------------------+------------------------------+ | 12 | new program runs | clear_child_tid | | | | .write_at_offset(0, 0) | | | | - confused-deputy write | | | | into new image | +----+------------------------------+------------------------------+ | 13 | new program runs | sys_futex(Wake), | | | | wake_robust_list() | | | | - more accesses via | | | | old-image addresses | +----+------------------------------+------------------------------+ The Fix ------- Don't decrement the `nr_threads` counter prior to completing any writes to that process' address space. `memcpy_fallible` provides fault safety, but not lifetime safety. I used Claude Opus 5 to inspect the codebase for this class of bug, help me verify that it can be triggered, and validate the fix. --- litebox_shim_linux/src/syscalls/process.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/litebox_shim_linux/src/syscalls/process.rs b/litebox_shim_linux/src/syscalls/process.rs index bdcfa19b8a..ca831f89fa 100644 --- a/litebox_shim_linux/src/syscalls/process.rs +++ b/litebox_shim_linux/src/syscalls/process.rs @@ -499,8 +499,6 @@ fn wake_robust_list( impl Task { /// Called when the task is exiting. pub(crate) fn prepare_for_exit(&mut self) { - self.thread.detach_from_process(); - if let Some(clear_child_tid) = self.thread.clear_child_tid.take() { // Clear the child TID if requested // TODO: if we are the last thread, we don't need to clear it @@ -516,6 +514,14 @@ impl Task { if let Some(robust_list) = self.thread.robust_list.take() { let _ = wake_robust_list(robust_list); } + + // This must run last, after all of this task's remaining accesses to + // guest memory have completed. Otherwise, because this decrements the + // `nr_threads` counter, other tasks may assume that the memory of this + // task isn't referenced any more, and therefore can be de-allocated and + // re-used (e.g., for an `exec`d process). See + // https://github.com/microsoft/litebox/pull/1155. + self.thread.detach_from_process(); } pub(crate) fn sys_exit(&self, status: i32) {