From f69d1e2dda42b5639faaea8c7a6f9e31677f010b Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:00:44 +0200 Subject: [PATCH] [ML] Rebuild CSandboxedProcessSpawner around an explicit, fault-injected lifecycle Replaces numeric-PID process control with an identity-bound spawner for sandboxed pytorch_inference children, because a sandboxed child's PID can be reused by an unrelated process while a stale monitor or a delayed terminate call is still in flight. CSandboxedProcessSpawner (new) drives every live child through an explicit state machine (Prepared -> Launched -> IdentityCaptured -> Registered -> Monitoring -> TerminationRequested -> CleanupRequired -> Reaped/Failed): - A non-throwing kill-and-reap guard is armed immediately once a sandboxed process is launched and its PID captured, before any operation that could throw (registry insertion, monitor-thread construction/detach). It holds its own owning reference to the sandbox handle so cleanup is safe regardless of the destruction order of other locals during unwinding, and disarms only after registry insertion and monitor handoff have both succeeded. - Four injectable seams (pidfd acquisition, registry allocation, monitor-thread launch, sandbox completion) allow every failure class to be exercised deterministically in tests, without waiting on real resource exhaustion. - pidfd-acquisition outcomes are classified explicitly: a kernel that lacks pidfd support (ENOSYS) is the only case that falls back to signalling the sandboxee through its owned monitor handle (SIGKILL, identity-safe, no numeric-PID lookup); every other acquisition failure (ESRCH, EMFILE, ENFILE, ...) is treated as a resource/identity error and fails registration outright rather than silently choosing a weaker termination mechanism. Numeric kill(pid, ...) does not appear anywhere in this file. - A one-shot CAS latch resolves the race between a timeout and the sandbox's own completion signal, replacing independent-boolean coordination with a single atomic decision that has exactly one winner. CSandboxedProcessSpawnerLifecycleTest_Linux.cc adds fault-injection coverage for every pidfd-acquisition outcome, allocation/resource failures during registration and monitor launch, protection against a stale monitor mutating a newer registration for a reused PID, protection against signalling the wrong process after PID reuse, the CAS race (deterministic interleavings, no wall-clock polling), descriptor-count cleanup after every case, and destructor latency with a live child still running. Reviewed through several fault-injection and use-after-free fix rounds this session; not yet compiled or executed against a real Linux/Sandbox2 toolchain. --- include/sandbox/CSandboxedProcessSpawner.h | 317 +++++ lib/sandbox/CMakeLists.txt | 1 + lib/sandbox/CSandboxedProcessSpawner_Linux.cc | 853 ++++++++++++ lib/sandbox/unittest/CMakeLists.txt | 30 + ...dboxedProcessSpawnerLifecycleTest_Linux.cc | 1188 +++++++++++++++++ .../payloads/lifecycle_signal_payload.cc | 72 + 6 files changed, 2461 insertions(+) create mode 100644 include/sandbox/CSandboxedProcessSpawner.h create mode 100644 lib/sandbox/CSandboxedProcessSpawner_Linux.cc create mode 100644 lib/sandbox/unittest/CSandboxedProcessSpawnerLifecycleTest_Linux.cc create mode 100644 lib/sandbox/unittest/payloads/lifecycle_signal_payload.cc diff --git a/include/sandbox/CSandboxedProcessSpawner.h b/include/sandbox/CSandboxedProcessSpawner.h new file mode 100644 index 000000000..138482618 --- /dev/null +++ b/include/sandbox/CSandboxedProcessSpawner.h @@ -0,0 +1,317 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the following additional limitation. Functionality enabled by the + * files subject to the Elastic License 2.0 may only be used in production when + * invoked by an Elasticsearch process with a license key installed that permits + * use of machine learning features. You may not use this file except in + * compliance with the Elastic License 2.0 and the foregoing additional + * limitation. + */ +#ifndef INCLUDED_ml_sandbox_CSandboxedProcessSpawner_h +#define INCLUDED_ml_sandbox_CSandboxedProcessSpawner_h + +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +// Sandbox2 headers are unavailable on non-Linux configure runs (see +// include/sandbox/CPytorchInferenceSandboxPolicy.h). Only a forward +// declaration is needed here: this header stores sandbox2::Sandbox2 solely +// behind a shared_ptr, never by value, so non-Linux builds never need the +// real type. +namespace sandbox2 { +class Sandbox2; +} + +#ifdef SANDBOX2_AVAILABLE +// The Sandbox2-completion injectable seam (TAwaitResultFn, below) names +// sandbox2::Result in a std::function signature, which needs the complete +// type - the forward declaration above is not enough for that one seam. +// Non-Linux/no-Sandbox2 configures never see this include, matching +// include/sandbox/CPytorchInferenceSandboxPolicy.h's pattern for the same +// reason. +#include +#endif + +namespace ml { +namespace sandbox { + +//! \brief +//! Spawn and own the lifecycle of processes inside a Sandbox2 isolation +//! boundary. +//! +//! DESCRIPTION:\n +//! Replaces numeric-PID process control (core::CDetachedProcessSpawner's +//! model) with identity-bound handles, because a sandboxed child's PID can +//! be reused by an unrelated process while a stale monitor or a delayed +//! terminateChild() call is still in flight. The lifecycle below is the +//! explicit state machine every live registry entry moves through: +//! Prepared -> Launched -> IdentityCaptured -> Registered -> Monitoring -> +//! Reaped is the full happy-path transition set, with +//! TerminationRequested/CleanupRequired/Failed as the additional states a +//! termination request or a failure path can move through. This header +//! declares the state shape and public API only - spawn()'s kill-and-reap +//! guard, injectable seams, and pidfd outcome classification are +//! implemented in CSandboxedProcessSpawner_Linux.cc. +class CSandboxedProcessSpawner { +public: + using TStrVec = std::vector; + + //! Explicit lifecycle states a registry entry moves through: Prepared -> + //! Launched -> IdentityCaptured -> Registered -> Monitoring -> Reaped. + //! E_IdentityCaptured marks the point where pid() has been captured but + //! the child is not yet registered - a distinct state from E_Launched + //! because the kill-and-reap guard must be armed as soon as pid() is + //! known, before registration, not folded into a coarser "Launched" + //! state. No state is skipped and no state is inferred from a + //! combination of booleans. + enum class EChildLifecycleState { + E_Prepared, //!< Launch spec validated; process not yet started. + E_Launched, //!< Sandbox2::RunAsync() succeeded; pid() not yet captured. + E_IdentityCaptured, //!< pid() captured; kill-and-reap guard armed so a failure from + //!< here on cannot leave a live, unowned child. + E_Registered, //!< Registry insertion succeeded. + E_Monitoring, //!< Monitor thread handoff succeeded; guard disarmed because the + //!< monitor now owns reaping the child. + E_TerminationRequested, //!< terminateChild() issued a request; child not yet confirmed exited. + E_CleanupRequired, //!< Sandbox2 completion observed; registry entry pending removal. + E_Reaped, //!< AwaitResult() returned; every descriptor closed exactly once. + E_Failed //!< spawn() failed at or after this state; no live unowned child remains. + }; + + //! One-shot outcome of the timeout-vs-completion race, replacing + //! independent-boolean coordination with a single atomic latch. + //! Exactly one of TimedOut/Completed wins via + //! compare_exchange_strong from Pending; the loser observes the + //! winner's value and must not perform cleanup. + enum class EOutcomeState { E_Pending, E_TimedOut, E_Completed }; + + //! \brief One-shot CAS latch: Pending -> TimedOut|Completed, never back. + //! + //! DESCRIPTION:\n + //! The only coordination mechanism between a timeout path and a + //! Sandbox2-completion path racing to decide who performs cleanup for + //! the same child. A single compare_exchange_strong call decides the + //! winner; the loser's compare_exchange_strong fails and returns the + //! value the winner set, so it can branch without a second flag. + class CCasOutcomeLatch { + public: + CCasOutcomeLatch() = default; + + CCasOutcomeLatch(const CCasOutcomeLatch&) = delete; + CCasOutcomeLatch& operator=(const CCasOutcomeLatch&) = delete; + + //! Attempt to move the latch from Pending to \p desired. Returns + //! true iff this call won the race (the latch was Pending and is + //! now \p desired); false means some call - possibly this one on a + //! retry, possibly a racing call - already set it to another value, + //! which is written back into \p desired for the caller to inspect. + bool tryResolve(EOutcomeState& desired) { + EOutcomeState expected{EOutcomeState::E_Pending}; + return m_State.compare_exchange_strong(expected, desired) + ? true + : (desired = expected, false); + } + + //! \return the latch's current value. For diagnostics only - never + //! branch cleanup logic on a load() result instead of tryResolve()'s + //! own return value, or the check-then-act gap reintroduces the + //! two-boolean race this latch replaces. + EOutcomeState load() const { return m_State.load(); } + + private: + std::atomic m_State{EOutcomeState::E_Pending}; + }; + + //! Placeholder outcome of the injectable pidfd-acquisition seam (Task 2 + //! scope only). A simple success/failure signal - Task 3 replaces this + //! with full ENOSYS/EMFILE/... classification and decides what a + //! classified failure does; nothing here selects a numeric-kill(pid) + //! fallback, and nothing should until Task 3 lands. + struct SPidFdAcquisitionResult { + int s_Fd{-1}; + int s_Errno{0}; + }; + + //! Explicit classification of a pidfd-acquisition attempt, replacing + //! the Task 2 placeholder's generic "negative fd" check with a + //! three-way outcome that decides both whether spawn() registers the + //! child at all, and - for a registered child - which of + //! terminateChild()'s two mechanisms applies. + //! + //! E_Acquired: s_Fd >= 0. terminateChild() sends a request via + //! pidfd_send_signal(SIGTERM) on the held pidfd. + //! + //! E_KernelUnsupported: s_Fd < 0 and s_Errno == ENOSYS - the running + //! kernel predates pidfd support entirely (pre-5.3). This is the *only* + //! classification for which terminateChild() falls back to + //! Sandbox2::Kill() (SIGKILL via the owned monitor, identity-safe, no + //! numeric-PID lookup). Recorded on the registry entry at registration + //! time - terminateChild() must use that recorded value, never + //! re-derive it by re-calling pidfd_open. + //! + //! E_Failed: s_Fd < 0 and s_Errno is anything else (ESRCH, EMFILE, + //! ENFILE, ...). This is a resource or identity error, not "no kernel + //! support" - it must never be treated the same + //! as E_KernelUnsupported. spawn() fails registration outright on this + //! outcome rather than registering a child whose termination would need + //! an undefined fallback. + enum class EPidFdOutcome { E_Acquired, E_KernelUnsupported, E_Failed }; + + //! Pure classification function for a pidfd-acquisition result: no + //! syscalls, no I/O, no side effects, so it is unit-testable in + //! isolation against synthetic SPidFdAcquisitionResult values (e.g. Task + //! 4's ENOSYS/EMFILE/ESRCH/success cases) without a real pidfd or + //! kernel. Implemented outside the SANDBOX2_AVAILABLE-gated block in the + //! .cc, so it compiles - and is testable - on every platform. + static EPidFdOutcome classifyPidFdOutcome(const SPidFdAcquisitionResult& result); + +public: + //! \brief A live sandboxed child and the handles needed to manage it + //! safely through every lifecycle state. + //! + //! DESCRIPTION:\n + //! Shape only in Task 1 - no lifecycle logic landed there. Carries the + //! explicit state, a monotonic generation (so a stale monitor cannot + //! erase or mutate a newer registration racing the same PID), the + //! Sandbox2 handle (co-owned with any monitor thread via shared_ptr, + //! since a monitor can outlive this spawner and must never hold a raw + //! pointer back into it), the pidfd used for identity-bound + //! termination when the kernel provides one, and the one-shot outcome + //! latch used to resolve a timeout-vs-completion race for this specific + //! child. Public (rather than Task 1's private placement) as + //! of Task 2: the registry-allocation seam (TRegistryInsertFn, below) + //! and its test-only overrides need to name this type, and a private + //! nested type cannot appear in a public alias's signature in a way + //! external test code could actually spell. + struct SSandboxedChild { + EChildLifecycleState s_State{EChildLifecycleState::E_Prepared}; + std::uint64_t s_Generation{0}; + std::shared_ptr s_Sandbox; + int s_PidFd{-1}; + //! Classification recorded at registration time (Task 3). Every + //! entry that actually reaches the registry has this set + //! to E_Acquired or E_KernelUnsupported - E_Failed never gets + //! registered (see EPidFdOutcome's comment) - but the default below + //! still resolves to the fail-closed value in case some future path + //! forgets to set it explicitly. + EPidFdOutcome s_PidFdOutcome{EPidFdOutcome::E_Failed}; + std::shared_ptr s_Outcome; + }; + + //! \brief The live sandboxed children, and the lock that guards them. + //! + //! DESCRIPTION:\n + //! Held behind a shared_ptr because a monitor thread that removes a + //! child outlives the spawn() call that started it, and can outlive + //! this object: the controller may tear the spawner down while a + //! sandboxed pytorch_inference is still running, and the monitor + //! only learns that the sandboxee exited some time later. A raw pointer + //! back to the spawner would be dangling by then, so the monitor + //! co-owns the registry instead, and the spawner's destructor needs no + //! synchronisation with in-flight monitors. + struct SPidRegistry { + mutable std::mutex s_Mutex; + std::uint64_t s_NextGeneration{0}; + std::map s_Children; + }; + using TPidRegistryPtr = std::shared_ptr; + + //! Injectable seams, introduced in Task 2. Each has a + //! production default, selected by passing an empty std::function to + //! the test-only constructor below (or by using the plain default + //! constructor, which never touches these types at all). + + //! pidfd-acquisition seam: wraps the pidfd_open syscall. See + //! SPidFdAcquisitionResult's comment - Task 3 replaces the placeholder + //! success/failure shape with full classification. + using TPidFdOpenFn = std::function; + + //! Registry-allocation seam: performs the locked map insertion + //! (replacing any stale entry for the same PID, mirroring the + //! production default) and returns the new entry's generation. The + //! production default never throws for ordinary insertion; a test + //! overriding this seam can throw std::bad_alloc, or return a + //! deliberately colliding generation, to exercise those failure and + //! collision paths deterministically without waiting on real resource + //! exhaustion. + using TRegistryInsertFn = + std::function; + + //! Monitor-thread creation/detach seam. Returns false - never throws - + //! if std::thread construction or detach() failed, so a test can force + //! that failure deterministically without depending on the OS + //! actually running out of threads. The production default constructs + //! std::thread(monitorBody) and detaches it, converting any + //! std::system_error from either step into a false return. + using TMonitorLaunchFn = std::function monitorBody)>; + +#ifdef SANDBOX2_AVAILABLE + //! Sandbox2-completion seam: wraps calling AwaitResult() on the live + //! sandbox handle, so a test controls exactly when/what result is + //! reported (for a deterministic timeout-vs-completion test, + //! Task 4 scope). Available only where sandbox2::Result is a complete + //! type; see the SANDBOX2_AVAILABLE include block above this class. + using TAwaitResultFn = std::function; +#endif + + CSandboxedProcessSpawner(); + + //! Test-only constructor injecting the four seams above. Each parameter + //! defaults to an empty std::function; spawn() + //! (CSandboxedProcessSpawner_Linux.cc) treats an empty seam as "use the + //! production behaviour", so production callers should keep using the + //! plain default constructor and never need to name these types. + CSandboxedProcessSpawner(TPidFdOpenFn pidFdOpenFn, + TRegistryInsertFn registryInsertFn, + TMonitorLaunchFn monitorLaunchFn +#ifdef SANDBOX2_AVAILABLE + , + TAwaitResultFn awaitResultFn +#endif + ); + + ~CSandboxedProcessSpawner(); + + //! Spawn a sandboxed process. Returns true only after registry + //! insertion and monitor handoff both succeed; on any other + //! outcome returns false with childPid left at 0 and no live unowned + //! child, no registry entry, and no leaked descriptor. + bool spawn(const std::string& processPath, const TStrVec& args, core::CProcess::TPid& childPid); + + //! Request termination of a sandboxed child previously started by this + //! object, targeting its identity-bound handle rather than a recycled + //! numeric PID. + bool terminateChild(core::CProcess::TPid pid); + + //! \return true if this object owns a sandboxed child with the given + //! PID that is still live (not yet Reaped or Failed). + bool hasChild(core::CProcess::TPid pid) const; + +private: + const TPidRegistryPtr m_PidRegistry{std::make_shared()}; + + //! Seam storage for the test-only constructor. Left empty (default + //! std::function) by the plain default constructor, which + //! CSandboxedProcessSpawner_Linux.cc reads as "use the production + //! behaviour" for every seam. + TPidFdOpenFn m_PidFdOpenFn; + TRegistryInsertFn m_RegistryInsertFn; + TMonitorLaunchFn m_MonitorLaunchFn; +#ifdef SANDBOX2_AVAILABLE + TAwaitResultFn m_AwaitResultFn; +#endif +}; + +} // namespace sandbox +} // namespace ml + +#endif // INCLUDED_ml_sandbox_CSandboxedProcessSpawner_h diff --git a/lib/sandbox/CMakeLists.txt b/lib/sandbox/CMakeLists.txt index 89170eb66..06a316469 100644 --- a/lib/sandbox/CMakeLists.txt +++ b/lib/sandbox/CMakeLists.txt @@ -25,6 +25,7 @@ set(ML_LINK_LIBRARIES set(SRCS CMlSandboxAvailability.cc CPytorchInferenceSandboxPolicy.cc + CSandboxedProcessSpawner_Linux.cc ) ml_add_library(MlSandbox STATIC ${SRCS}) diff --git a/lib/sandbox/CSandboxedProcessSpawner_Linux.cc b/lib/sandbox/CSandboxedProcessSpawner_Linux.cc new file mode 100644 index 000000000..f128be047 --- /dev/null +++ b/lib/sandbox/CSandboxedProcessSpawner_Linux.cc @@ -0,0 +1,853 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the following additional limitation. Functionality enabled by the + * files subject to the Elastic License 2.0 may only be used in production when + * invoked by an Elasticsearch process with a license key installed that permits + * use of machine learning features. You may not use this file except in + * compliance with the Elastic License 2.0 and the foregoing additional + * limitation. + */ +#include + +#include +#include + +#include +#include +#include +#include +#include + +// classifyPidFdOutcome is a pure function with no +// syscalls or Sandbox2 types in its signature, so - unlike the rest of this +// file - it is defined below outside the SANDBOX2_AVAILABLE-gated block: it +// must compile, and be unit-testable, on every platform, matching this TU's +// own "compiled unconditionally" contract (see the comment above the +// SANDBOX2_AVAILABLE block). (for ENOSYS) is therefore included +// unconditionally too, rather than inside that block alongside . + +// This translation unit is compiled unconditionally (see lib/sandbox/CMakeLists.txt +// - it is added to SRCS the same way lib/core/CMakeLists.txt unconditionally +// builds CDetachedProcessSpawner.cc), so every symbol outside the +// SANDBOX2_AVAILABLE-gated block below must compile with no Sandbox2/Linux +// headers available at all. The real spawn() logic - and everything that +// needs sandbox2:: types or Linux-only syscalls - lives inside that block; +// non-Linux/no-Sandbox2 configures fall through to the "not built with +// Sandbox2 support" stub path at the bottom of spawn(), matching +// CPytorchInferenceSandboxPolicy.cc's split. +#ifdef SANDBOX2_AVAILABLE + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include + +// environ is a global variable from the C runtime library. +extern char** environ; + +// The CentOS 7 based CI build image has kernel headers that predate pidfd, so +// __NR_pidfd_open may be undefined at build time even though the runtime +// kernel supports it. pidfd_open is syscall number 434 on every architecture +// ml-cpp builds for (x86_64 and aarch64); fall back to that literal so the +// spawner does not depend on the build image's header version. Task 3 owns +// classifying what a failed acquisition means (ENOSYS vs. a resource error); +// this task only needs the raw syscall wrapped behind the injectable seam. +#ifdef __NR_pidfd_open +#define ML_NR_pidfd_open __NR_pidfd_open +#else +#define ML_NR_pidfd_open 434 +#endif + +// Same rationale as ML_NR_pidfd_open above: pidfd_send_signal is syscall +// number 424 on every architecture ml-cpp builds for (x86_64 and aarch64), +// so fall back to that literal when the build image's kernel headers +// predate it. Used by terminateChild()'s E_Acquired path (SIGTERM request +// via the held pidfd) - the only place this file sends a signal to a +// sandboxee by identity-bound handle rather than by recycled numeric PID. +#ifdef __NR_pidfd_send_signal +#define ML_NR_pidfd_send_signal __NR_pidfd_send_signal +#else +#define ML_NR_pidfd_send_signal 424 +#endif + +#endif // SANDBOX2_AVAILABLE + +namespace ml { +namespace sandbox { + +// Defined outside the SANDBOX2_AVAILABLE-gated block below (unlike +// everything else in this file): a pure function with no syscalls, no +// Sandbox2 types, and no platform-specific behaviour, so it must compile - +// and be unit-testable - on every configure, matching this TU's +// "compiled unconditionally" contract (see the file-level comment above). +CSandboxedProcessSpawner::EPidFdOutcome CSandboxedProcessSpawner::classifyPidFdOutcome( + const CSandboxedProcessSpawner::SPidFdAcquisitionResult& result) { + if (result.s_Fd >= 0) { + return EPidFdOutcome::E_Acquired; + } + if (result.s_Errno == ENOSYS) { + return EPidFdOutcome::E_KernelUnsupported; + } + return EPidFdOutcome::E_Failed; +} + +#ifdef SANDBOX2_AVAILABLE + +namespace { + +//! RAII owner for a pidfd between acquisition and the registry insertion +//! that takes over its lifetime. Closes the descriptor on destruction unless +//! release() has handed ownership to the registry entry, so an exception +//! (e.g. std::bad_alloc from the map node allocation, injected via the +//! registry-allocation seam) thrown before registration cannot leak the fd. +class CScopedPidFd { +public: + explicit CScopedPidFd(int pidFd) : m_PidFd{pidFd} {} + ~CScopedPidFd() { + if (m_PidFd >= 0) { + ::close(m_PidFd); + } + } + CScopedPidFd(const CScopedPidFd&) = delete; + CScopedPidFd& operator=(const CScopedPidFd&) = delete; + int get() const { return m_PidFd; } + //! Relinquish ownership: the caller (the registry entry) is now + //! responsible for closing the descriptor. + void release() { m_PidFd = -1; } + +private: + int m_PidFd; +}; + +//! Close a pidfd that a registry entry owns, tolerating an already-released +//! (-1) value. +void closePidFdIfOpen(int pidFd) { + if (pidFd >= 0) { + ::close(pidFd); + } +} + +//! Non-throwing kill-and-reap guard, closing the gap between a successful +//! process launch and the point where ownership is fully handed off to the +//! registry and monitor thread. Armed immediately when RunAsync() succeeds +//! and pid() is captured (E_IdentityCaptured) - before any +//! potentially-throwing operation (registry insertion, monitor-thread +//! construction, detach()) - and disarmed only after registry insertion AND +//! monitor handoff both succeed (E_Monitoring). Every early return on the +//! path between those two points goes through this guard's destructor +//! rather than a hand-written duplicate cleanup block, so there is exactly +//! one cleanup owner for "launched but not yet fully handed off". +//! +//! Holds its own shared_ptr copy (not a raw, non-owning pointer) +//! so its lifetime is entirely self-sufficient: it does not matter what +//! order this guard is declared in relative to other shared_ptr-holding +//! locals in spawn() (e.g. `sandbox`, `child.s_Sandbox`), nor what order +//! those locals get destroyed in during stack unwinding on a failure path. +//! A raw pointer previously used here relied on some other local staying +//! alive for the guard's own destructor to run safely against; if that +//! local's declaration (and therefore destruction) order ever changed, or +//! if the object's last owning shared_ptr was destroyed before this guard +//! during unwinding, the guard's destructor would call Kill() on a dangling +//! pointer. Holding an owning copy makes that structurally impossible: this +//! guard is always one of the owners, so the object cannot be freed before +//! this guard's own destructor has run. +//! +//! The destructor must not throw: it runs during stack unwinding on the +//! failure paths this guard exists to cover, and a second exception there +//! would call std::terminate. Kill() and the AwaitResult seam are wrapped in +//! a catch-all for that reason; this task does not classify what Kill() +//! itself can fail with (Task 3 scope), only ensures a throw from it cannot +//! escape a destructor. +class CKillAndReapGuard { +public: + CKillAndReapGuard(std::shared_ptr sandbox, + CSandboxedProcessSpawner::TAwaitResultFn awaitResultFn) + : m_Sandbox{std::move(sandbox)}, m_AwaitResultFn{std::move(awaitResultFn)} {} + + ~CKillAndReapGuard() { + if (m_Armed && m_Sandbox) { + try { + m_Sandbox->Kill(); + if (m_AwaitResultFn) { + m_AwaitResultFn(*m_Sandbox); + } else { + m_Sandbox->AwaitResult(); + } + } catch (...) { + // Never let an exception escape a destructor; this guard's + // whole purpose is bounded, best-effort cleanup on a + // failure path that is already unwinding. + } + } + } + + CKillAndReapGuard(const CKillAndReapGuard&) = delete; + CKillAndReapGuard& operator=(const CKillAndReapGuard&) = delete; + + //! Called once registry insertion AND monitor handoff have both + //! succeeded (E_Monitoring). After this, the monitor thread owns + //! calling the (possibly injected) AwaitResult seam exactly once. + void disarm() { m_Armed = false; } + +private: + std::shared_ptr m_Sandbox; + CSandboxedProcessSpawner::TAwaitResultFn m_AwaitResultFn; + bool m_Armed{true}; +}; + +//! The sandboxee's environment: the caller's, with ML_SANDBOXED=1 set +//! exactly once so pytorch_inference skips its in-process seccomp filter and +//! relies on the Sandbox2 policy instead. +std::vector buildSandboxeeEnvironment() { + std::vector sandboxeeEnv; + bool markerSet{false}; + for (char** env = ::environ; *env != nullptr; ++env) { + std::string envVar{*env}; + if (envVar.find("ML_SANDBOXED=") == 0) { + sandboxeeEnv.push_back("ML_SANDBOXED=1"); + markerSet = true; + } else { + sandboxeeEnv.push_back(std::move(envVar)); + } + } + if (markerSet == false) { + sandboxeeEnv.push_back("ML_SANDBOXED=1"); + } + return sandboxeeEnv; +} + +//! An executor configured for a long-lived daemon sandboxee, matching the +//! frozen pre-rebuild reference's timeout/rlimit relaxations (a run-to- +//! completion default would kill a healthy, long-lived pytorch_inference). +std::unique_ptr +makeConfiguredExecutor(const std::string& absPath, + const std::vector& fullArgs, + const std::string& binDir) { + auto executor = std::make_unique( + absPath, fullArgs, buildSandboxeeEnvironment()); + executor->set_enable_sandbox_before_exec(true); + executor->set_cwd(binDir); + executor->limits()->set_walltime_limit(absl::ZeroDuration()); + executor->limits()->set_rlimit_cpu(RLIM64_INFINITY); + executor->limits()->set_rlimit_nofile(65536); + return executor; +} + +//! Production default for the pidfd-acquisition seam: the raw pidfd_open +//! syscall. classifyPidFdOutcome() (defined below, outside this +//! SANDBOX2_AVAILABLE block) turns this raw fd/errno pair into the +//! Acquired/KernelUnsupported/Failed classification spawn() acts on. No +//! numeric-kill(pid) fallback is introduced anywhere by this file. +CSandboxedProcessSpawner::SPidFdAcquisitionResult defaultPidFdOpen(core::CProcess::TPid pid) { + CSandboxedProcessSpawner::SPidFdAcquisitionResult result; + result.s_Fd = + static_cast(::syscall(ML_NR_pidfd_open, static_cast(pid), 0u)); + result.s_Errno = (result.s_Fd < 0) ? errno : 0; + return result; +} + +//! Production default for the monitor-thread creation/detach seam: +//! construct a std::thread running monitorBody and detach it, converting +//! any std::system_error from either step into a false return instead +//! of letting it propagate as an exception - the caller (spawn()) treats a +//! false return the same way regardless of which step failed. +bool defaultMonitorLaunch(std::function monitorBody) { + try { + std::thread monitor{std::move(monitorBody)}; + monitor.detach(); + return true; + } catch (const std::exception&) { return false; } +} + +//! Log how a sandboxed pytorch_inference terminated. Runs on the monitor +//! thread that owns the sandbox instance, so it deliberately takes no +//! spawner state - the caller does the registry bookkeeping under the lock. +//! +//! Review finding 5 (self-review round 2): every StatusEnum value gets its +//! own case rather than funnelling everything but SIGNALED into one opaque +//! LOG_ERROR. Two of those previously-generic cases matter operationally: +//! EXTERNAL_KILL is this file's OWN ENOSYS-fallback success path +//! (terminateChild()'s E_KernelUnsupported branch calls Sandbox2::Kill(), +//! which the monitor observes as EXTERNAL_KILL, not SIGNALED - see gate 1's +//! testTerminateChildFallsBackToKillWhenKernelUnsupportsPidfd) and must not +//! be logged as an abnormal termination; VIOLATION is the most +//! operationally important signal a sandbox can report and must never be +//! indistinguishable from an internal error. +void logSandboxeeTermination(core::CProcess::TPid sandboxPid, const sandbox2::Result& result) { + switch (result.final_status()) { + case sandbox2::Result::OK: + if (result.reason_code() == 0) { + LOG_DEBUG(<< "Sandboxed pytorch_inference (PID " << sandboxPid << ") has exited"); + } else { + LOG_WARN(<< "Sandboxed pytorch_inference (PID " << sandboxPid + << ") has exited with exit code " << result.reason_code()); + } + break; + case sandbox2::Result::SIGNALED: + LOG_INFO(<< "Sandboxed pytorch_inference (PID " << sandboxPid + << ") was terminated by signal " << result.reason_code()); + break; + case sandbox2::Result::EXTERNAL_KILL: + // Expected, successful termination - this is the ENOSYS-fallback + // path (Sandbox2::Kill() via terminateChild()'s E_KernelUnsupported + // branch), not a failure, so INFO rather than ERROR. + LOG_INFO(<< "Sandboxed pytorch_inference (PID " << sandboxPid + << ") was force-killed via Sandbox2::Kill()"); + break; + case sandbox2::Result::VIOLATION: + // reason_code() carries the violating syscall number for this + // status. Logged at ERROR with that detail so a seccomp policy + // violation is never mistaken for an opaque internal error. + LOG_ERROR(<< "Sandboxed pytorch_inference (PID " << sandboxPid << ") violated the sandbox policy (syscall " + << result.reason_code() << ')'); + break; + case sandbox2::Result::TIMEOUT: + LOG_ERROR(<< "Sandboxed pytorch_inference (PID " << sandboxPid + << ") exceeded its wall-time/CPU limit and was terminated"); + break; + case sandbox2::Result::SETUP_ERROR: + LOG_ERROR(<< "Sandboxed pytorch_inference (PID " << sandboxPid + << ") failed to set up the sandbox"); + break; + case sandbox2::Result::INTERNAL_ERROR: + LOG_ERROR(<< "Sandboxed pytorch_inference (PID " << sandboxPid + << ") hit an internal Sandbox2 error"); + break; + default: + // UNSET (and any future StatusEnum value this file does not yet + // know about) - AwaitResult() has already returned by the time this + // runs, so UNSET should be structurally unreachable, but keep a + // narrow default rather than silently dropping an unrecognized + // status. + LOG_ERROR(<< "Sandboxed pytorch_inference (PID " << sandboxPid + << ") terminated abnormally, final_status=" << result.final_status()); + break; + } +} + +//! Production default for the registry-allocation seam: lock, allocate the +//! next generation, replace any stale entry for the same PID (closing its +//! pidfd first), insert, and return the new generation. A test overriding +//! this seam can throw (e.g. std::bad_alloc) or return a colliding +//! generation to exercise those failure and collision paths deterministically. +std::uint64_t defaultRegistryInsert(CSandboxedProcessSpawner::SPidRegistry& registry, + core::CProcess::TPid pid, + CSandboxedProcessSpawner::SSandboxedChild child) { + std::lock_guard lock(registry.s_Mutex); + const std::uint64_t generation{++registry.s_NextGeneration}; + const auto existing = registry.s_Children.find(pid); + if (existing != registry.s_Children.end()) { + closePidFdIfOpen(existing->second.s_PidFd); + LOG_DEBUG(<< "Replacing stale registry entry for sandboxed pytorch_inference PID " + << pid << " before registering generation " << generation); + } + child.s_Generation = generation; + child.s_State = CSandboxedProcessSpawner::EChildLifecycleState::E_Registered; + registry.s_Children[pid] = std::move(child); + return generation; +} + +} // namespace + +#endif // SANDBOX2_AVAILABLE + +CSandboxedProcessSpawner::CSandboxedProcessSpawner() = default; + +CSandboxedProcessSpawner::CSandboxedProcessSpawner(TPidFdOpenFn pidFdOpenFn, + TRegistryInsertFn registryInsertFn, + TMonitorLaunchFn monitorLaunchFn +#ifdef SANDBOX2_AVAILABLE + , + TAwaitResultFn awaitResultFn +#endif + ) + : m_PidFdOpenFn{std::move(pidFdOpenFn)}, m_RegistryInsertFn{std::move(registryInsertFn)}, m_MonitorLaunchFn { + std::move(monitorLaunchFn) +} +#ifdef SANDBOX2_AVAILABLE +, m_AwaitResultFn { + std::move(awaitResultFn) +} +#endif +{} + +CSandboxedProcessSpawner::~CSandboxedProcessSpawner() = default; + +bool CSandboxedProcessSpawner::spawn(const std::string& processPath, + const TStrVec& args, + core::CProcess::TPid& childPid) { + childPid = 0; + +#ifdef SANDBOX2_AVAILABLE + + // Resolve to absolute path - Sandbox2 requires absolute paths. + char resolvedPath[PATH_MAX]; + if (::realpath(processPath.c_str(), resolvedPath) == nullptr) { + LOG_ERROR(<< "Cannot resolve path " << processPath << ": " << ::strerror(errno)); + return false; + } + const std::string absPath(resolvedPath); + + struct stat binaryStat; + if (::stat(absPath.c_str(), &binaryStat) != 0) { + LOG_ERROR(<< "Cannot stat " << absPath << ": " << ::strerror(errno)); + return false; + } + + TStrVec fullArgs; + fullArgs.reserve(args.size() + 1); + fullArgs.push_back(processPath); + for (const std::string& arg : args) { + fullArgs.push_back(arg); + } + + // Validate every path-bearing launch argument against the pinned + // child-root contract *before* a policy is ever constructed. s_Ok == + // false must fail the spawn outright - never fall back to a + // partially-built policy. + const char* tmpDirEnv{::getenv("TMPDIR")}; + const std::string trustedTmpDir{tmpDirEnv != nullptr ? tmpDirEnv : "/tmp"}; + const SChildIpcValidationResult validated{validateChildIpcLaunchSpec(trustedTmpDir, args)}; + if (validated.s_Ok == false) { + std::ostringstream rejected; + for (const SRejectedChildIpcPath& r : validated.s_Rejected) { + rejected << " [" << r.s_Arg + << ": reason=" << static_cast(r.s_Reason) << ']'; + } + LOG_ERROR(<< "Rejected pytorch_inference child-IPC launch spec for " + << processPath << ':' << rejected.str()); + return false; + } + + // Binary and library directories to bind-mount. libDir is the SIBLING of + // binDir, not a child of it: the ML distribution lays out + // /bin/pytorch_inference alongside /lib, so this + // strips "bin" off binDir before appending "lib" rather than appending + // to binDir. Derived from processPath rather than added as a + // CSandboxedProcessSpawner constructor parameter: spawn()'s signature is + // pinned by the plan and every known caller launches pytorch_inference + // from that fixed distribution layout, so there is nothing a caller- + // supplied binDir/libDir would let a test or caller express that + // deriving from absPath does not already cover. + const std::string binDir{absPath.substr(0, absPath.rfind('/'))}; + const std::string libDir{binDir.substr(0, binDir.rfind('/')) + "/lib"}; + + // A private, bounded tmpfs at /tmp inside the sandbox - never the host's + // shared /tmp. 16 MiB matches the size already exercised end-to-end by + // CPytorchInferenceSandboxPolicyMechanismTest_Linux.cc; revisit if a + // real pytorch_inference workload needs more scratch space. + const std::size_t tmpfsSizeBytes{16 * 1024 * 1024}; + + sandbox2::PolicyBuilder policyBuilder{buildPytorchInferenceFilesystemPolicy( + binDir, libDir, validated.s_Spec, tmpfsSizeBytes)}; + + auto policyResult = policyBuilder.TryBuild(); + if (!policyResult.ok()) { + LOG_ERROR(<< "Failed to build Sandbox2 policy for " << processPath); + return false; + } + + auto sandboxPtr = std::make_unique( + makeConfiguredExecutor(absPath, fullArgs, binDir), std::move(*policyResult)); + + // Take shared ownership immediately, before RunAsync() ever launches + // anything - not after pid() is captured. This conversion can itself + // throw (a shared_ptr control-block allocation failure), but nothing has been + // launched yet at this point, so sandboxPtr's own (plain) destructor is + // sufficient cleanup on that failure; no Kill()/AwaitResult() is needed + // for a sandboxee that was never started. Doing this early - rather + // than arming CKillAndReapGuard on a raw, non-owning pointer into the + // still-unique_ptr-owned object and converting to shared_ptr afterward + // - means the guard constructed below always holds a genuine owning + // shared_ptr copy, making its cleanup self-sufficient regardless of + // declaration/destruction order among the other shared_ptr-holding + // locals later in this function (`sandbox` itself, `child.s_Sandbox`). + std::shared_ptr sandbox; + try { + sandbox = std::shared_ptr(std::move(sandboxPtr)); + } catch (const std::exception& e) { + LOG_ERROR(<< "Failed to take shared ownership of a sandboxee for " + << processPath << ": " << e.what()); + return false; + } + + // E_Launched. + if (!sandbox->RunAsync()) { + sandbox->AwaitResult(); + LOG_ERROR(<< "Sandbox2 failed to start " << processPath); + return false; + } + + childPid = sandbox->pid(); + if (childPid <= 0) { + sandbox->AwaitResult(); + childPid = 0; + LOG_ERROR(<< "Sandbox2 returned an invalid PID for " << processPath); + return false; + } + + const core::CProcess::TPid sandboxPid{childPid}; + // I2: default the caller's out-parameter back to 0 for the entire span + // between capturing sandboxPid and confirmed success (the final `return + // true` below). Several calls in that span (e.g. + // std::make_shared() a few lines down) can throw + // std::bad_alloc *before* the try/catch blocks further down start, and + // an exception there propagates straight out of spawn() uncaught (the + // kill-and-reap guard's destructor still cleans up the sandboxee + // correctly during unwind). Without this, that throw-only exit would + // leave the caller's childPid at the live PID even though spawn() never + // returned true. Every explicit `return false` below already sets + // childPid = 0 too; this makes 0 the default regardless of whether a + // given exit is a return or an uncaught throw. + childPid = 0; + + // E_IdentityCaptured: arm the kill-and-reap guard now that the + // sandboxee is actually running. The guard takes its own shared_ptr + // copy of `sandbox` (see CKillAndReapGuard's comment), so it remains + // valid through every early return below - registry-insert throw, + // monitor-launch-span throw, monitor-launch-seam false - independent of + // when `sandbox`/`child.s_Sandbox` themselves get destroyed during + // stack unwinding. + CKillAndReapGuard killAndReapGuard{sandbox, m_AwaitResultFn}; + + const SPidFdAcquisitionResult pidFdResult{ + m_PidFdOpenFn ? m_PidFdOpenFn(sandboxPid) : defaultPidFdOpen(sandboxPid)}; + CScopedPidFd pidFdGuard{pidFdResult.s_Fd}; + const EPidFdOutcome pidFdOutcome{classifyPidFdOutcome(pidFdResult)}; + + // An errno other than ENOSYS (ESRCH, EMFILE, ENFILE, ...) is a + // resource/identity error, not "no kernel support" for pidfd - it must + // never be treated the same as E_KernelUnsupported. Fail registration + // outright rather than register a child whose termination would need an + // undefined fallback. pidFdGuard closes any fd this path somehow still + // holds; killAndReapGuard (still armed) Kill()s/awaits the sandboxee. + if (pidFdOutcome == EPidFdOutcome::E_Failed) { + LOG_ERROR(<< "pidfd_open failed for sandboxed process " << processPath + << " (PID " << sandboxPid << ") with errno " + << pidFdResult.s_Errno << " (" << ::strerror(pidFdResult.s_Errno) + << "); refusing to register a child with an undefined termination fallback"); + childPid = 0; + return false; // killAndReapGuard fires here; pidFdGuard closes any fd on unwind. + } + + SSandboxedChild child; + child.s_State = EChildLifecycleState::E_IdentityCaptured; + child.s_Sandbox = sandbox; + child.s_PidFd = pidFdGuard.get(); + child.s_PidFdOutcome = pidFdOutcome; + child.s_Outcome = std::make_shared(); + + std::uint64_t generation{0}; + try { + generation = m_RegistryInsertFn + ? m_RegistryInsertFn(*m_PidRegistry, sandboxPid, child) + : defaultRegistryInsert(*m_PidRegistry, sandboxPid, child); + } catch (const std::exception& e) { + LOG_ERROR(<< "Failed to register sandboxed process " << processPath + << " (PID " << sandboxPid << "): " << e.what()); + childPid = 0; + return false; // killAndReapGuard fires here; pidFdGuard still owns the fd. + } + // E_Registered. The registry entry now owns the pidfd; do not double- + // close it via pidFdGuard's destructor on this path. + pidFdGuard.release(); + + // Erase the registry entry this call just inserted, matching by + // generation (in case a racing call already replaced it). Shared by + // every failure path between a successful registry insertion and a + // successful monitor handoff, since no monitor thread exists on any of + // those paths to ever perform that erase itself. + const auto eraseRegistryEntry = [this, sandboxPid, generation]() { + std::lock_guard lock(m_PidRegistry->s_Mutex); + const auto it = m_PidRegistry->s_Children.find(sandboxPid); + if (it != m_PidRegistry->s_Children.end() && it->second.s_Generation == generation) { + closePidFdIfOpen(it->second.s_PidFd); + m_PidRegistry->s_Children.erase(it); + } + }; + + // The sandboxee is a child of the Sandbox2 forkserver rather than of the + // controller, so waitpid() never sees it. Own the sandbox instance on a + // dedicated monitor thread that keeps it alive for the lifetime of + // pytorch_inference, waits for its result (via the injectable + // AwaitResult seam), and removes the registry entry before logging + // termination. The thread co-owns the registry and the Sandbox2 + // shared_ptr rather than capturing this: it can still be waiting on a + // live sandboxee when the spawner is destroyed, and a raw pointer + // back to the spawner would be dangling by then. + // + // Everything from copying m_PidRegistry/m_AwaitResultFn through + // launching the monitor thread runs inside a try/catch: those copies + // and constructing monitorBody's capture list can themselves throw + // (e.g. std::bad_alloc copying a std::function), and left unguarded + // that exception would otherwise escape spawn() uncaught, leaking the + // just-inserted registry entry. Catching here ensures every throw in + // this span still erases the registry entry and returns false with + // childPid == 0; killAndReapGuard's destructor performs the + // Kill()/await half of cleanup on unwind either way. + bool monitorStarted{false}; + try { + const TPidRegistryPtr registry{m_PidRegistry}; + const TAwaitResultFn awaitResultFn{m_AwaitResultFn}; + auto monitorBody = [sandboxPid, registry, sandbox, generation, awaitResultFn]() { + const sandbox2::Result result{awaitResultFn ? awaitResultFn(*sandbox) + : sandbox->AwaitResult()}; + // This thread's completion and a (currently unwired - + // Task 3 scope stops at this call site; no external timeout + // caller exists yet) timeout path both race to decide who + // performs cleanup for the same child. Route that decision + // through exactly one tryResolve() call on the child's own CAS + // latch rather than an ad-hoc boolean - if a timeout caller + // resolves the latch to E_TimedOut first, this call loses the + // race and must not also erase the registry entry or log + // termination (the timeout path owns that instead). + bool completionWonRace{true}; + { + std::lock_guard lock(registry->s_Mutex); + const auto it = registry->s_Children.find(sandboxPid); + if (it != registry->s_Children.end() && it->second.s_Generation == generation) { + if (it->second.s_Outcome) { + EOutcomeState desired{EOutcomeState::E_Completed}; + completionWonRace = it->second.s_Outcome->tryResolve(desired); + } + if (completionWonRace) { + // I1: record E_Reaped immediately before erasing the + // entry, so a future accessor reading state via the + // lock during this brief window would see E_Reaped + // rather than a stale E_Monitoring. Defensive/ + // documentation-only today - nothing reads it before + // the erase below - but matches the state machine's + // declared intent. + it->second.s_State = EChildLifecycleState::E_Reaped; + closePidFdIfOpen(it->second.s_PidFd); + registry->s_Children.erase(it); + } + } + } + if (completionWonRace) { + logSandboxeeTermination(sandboxPid, result); + } + }; + + monitorStarted = m_MonitorLaunchFn + ? m_MonitorLaunchFn(std::move(monitorBody)) + : defaultMonitorLaunch(std::move(monitorBody)); + } catch (const std::exception& e) { + eraseRegistryEntry(); + LOG_ERROR(<< "Failed to launch monitor thread for sandboxed process " + << processPath << " (PID " << sandboxPid << "): " << e.what()); + childPid = 0; + return false; // killAndReapGuard fires here. + } + + if (monitorStarted == false) { + // Monitor handoff failed: no thread is running to ever erase + // this registry entry or call AwaitResult(), so this frame owns + // both. killAndReapGuard's destructor Kill()s/awaits the sandboxee. + eraseRegistryEntry(); + LOG_ERROR(<< "Failed to start monitor thread for sandboxed process " + << processPath << " (PID " << sandboxPid << ")"); + childPid = 0; + return false; // killAndReapGuard fires here. + } + + // E_Monitoring: registry insertion and monitor handoff both + // succeeded, so the monitor thread now owns calling AwaitResult() and + // removing the registry entry. Disarm - the guard must not also reap. + killAndReapGuard.disarm(); + + // I1: record the E_Monitoring transition explicitly, generation-matched + // and under the lock, now that handoff is confirmed. Without this, + // E_Monitoring was declared in the state machine but never actually + // assigned anywhere, so the "explicit state machine, no state skipped" + // claim was not true in the code, and a future timeout caller would + // have nothing correct to branch on. + { + std::lock_guard lock(m_PidRegistry->s_Mutex); + const auto it = m_PidRegistry->s_Children.find(sandboxPid); + // Only advance from E_Registered: a registry-scanning terminator + // (e.g. a future timeout caller) can race this window and + // already have set E_TerminationRequested on the same generation; + // an unconditional overwrite here would silently revert that marker. + if (it != m_PidRegistry->s_Children.end() && it->second.s_Generation == generation && + it->second.s_State == EChildLifecycleState::E_Registered) { + it->second.s_State = EChildLifecycleState::E_Monitoring; + } + } + + // Final-review fix: log the live PID, not the not-yet-restored out + // parameter (childPid is still 0 here per the I2 fix below). + LOG_INFO(<< "Spawned sandboxed process " << processPath << " with PID " << sandboxPid); + + // I2: only now, with registration and monitor handoff both confirmed, is + // it safe to hand the live PID back to the caller. + childPid = sandboxPid; + return true; + +#else // !SANDBOX2_AVAILABLE + + LOG_ERROR(<< "Cannot spawn " << processPath << ": ml-cpp was built without Sandbox2 support"); + return false; + +#endif // SANDBOX2_AVAILABLE +} + +#ifdef SANDBOX2_AVAILABLE + +bool CSandboxedProcessSpawner::terminateChild(core::CProcess::TPid pid) { + // Two mechanisms only, selected by the classification recorded on the + // registry entry at *registration* time (never re-derived here by + // re-calling pidfd_open, per the task brief): pidfd_send_signal(SIGTERM) + // - a graceful termination *request* - for E_Acquired, or Sandbox2::Kill() + // (SIGKILL via the owned monitor) for E_KernelUnsupported. No numeric + // kill(pid) fallback exists anywhere in this file. + std::shared_ptr sandboxToKill; + EChildLifecycleState previousState{EChildLifecycleState::E_Failed}; + std::uint64_t capturedGeneration{0}; + { + std::lock_guard lock(m_PidRegistry->s_Mutex); + const auto it = m_PidRegistry->s_Children.find(pid); + // Considered-and-dropped (self-review round 2): this guard does not + // exclude E_TerminationRequested, so a repeated terminateChild() + // call on an already-in-flight (or already E_KernelUnsupported- + // Kill()ed) child can reach Sandbox2::Kill() a second time. Confirmed + // harmless against the pinned sandboxed-api v20241008 tag: + // Sandbox2::Kill() is idempotent (sets a flag and issues a null-safe + // notify; no double-free/double-signal), so this is not a hazard. + if (it == m_PidRegistry->s_Children.end() || + it->second.s_State == EChildLifecycleState::E_Reaped || + it->second.s_State == EChildLifecycleState::E_Failed) { + return false; + } + SSandboxedChild& child{it->second}; + previousState = child.s_State; + // C1/I3: capture the generation now, under the same lock acquisition + // that decides the termination mechanism, so a failure below can + // roll back state only if it still identifies the SAME registration + // (not a newer one that reused this numeric PID after this entry + // was reaped and erased). + capturedGeneration = child.s_Generation; + switch (child.s_PidFdOutcome) { + case EPidFdOutcome::E_Acquired: { + if (child.s_PidFd < 0) { + // Logic error (should be structurally unreachable given + // spawn()'s fail-closed registration in this task): a + // registry entry classified E_Acquired must hold a real + // pidfd. Do not silently no-op - log loudly and refuse. + LOG_ERROR(<< "Logic error: sandboxed child PID " << pid + << " classified E_Acquired but holds no pidfd"); + return false; + } + // C1: send the request WHILE STILL HOLDING s_Mutex. + // pidfd_send_signal is a non-blocking syscall, so this is safe, + // and it is the only way to close the race against monitorBody's + // Sandbox2-completion handler, which also takes this same lock + // before closing this exact pidfd and erasing the registry entry + // (it does not check s_State). Previously the syscall ran + // outside the lock: a snapshot-then-signal window let + // monitorBody close the pidfd and the kernel recycle that + // descriptor number for an unrelated spawn() in between, so a + // delayed pidfd_send_signal here could hit the wrong process + // (an "identity, not recycled descriptor" hazard, one layer + // below the already-fixed numeric-PID case). + if (::syscall(ML_NR_pidfd_send_signal, child.s_PidFd, SIGTERM, nullptr, 0u) != 0) { + LOG_ERROR(<< "pidfd_send_signal(SIGTERM) failed for sandboxed child PID " + << pid << ": " << ::strerror(errno)); + // No state transition happened on this path (the state is + // only advanced below, on success), so there is nothing to + // roll back. + return false; + } + child.s_State = EChildLifecycleState::E_TerminationRequested; + return true; + } + case EPidFdOutcome::E_KernelUnsupported: + if (!child.s_Sandbox) { + // Same reasoning as above: E_KernelUnsupported without a + // Sandbox2 handle to Kill() is a logic error, not a + // silent no-op. + LOG_ERROR(<< "Logic error: sandboxed child PID " << pid + << " classified E_KernelUnsupported but holds no Sandbox2 handle"); + return false; + } + sandboxToKill = child.s_Sandbox; + child.s_State = EChildLifecycleState::E_TerminationRequested; + break; + case EPidFdOutcome::E_Failed: + default: + // Structurally unreachable: spawn() never registers an + // E_Failed child (see the pidFdOutcome check above it). Assert + // in debug builds and refuse rather than silently no-op if it + // somehow happened anyway. + LOG_ERROR(<< "Logic error: sandboxed child PID " << pid + << " registered with an undefined termination fallback (classification=" + << static_cast(child.s_PidFdOutcome) << ')'); + return false; + } + } + + // Only the E_KernelUnsupported/Sandbox2::Kill() path reaches here - the + // E_Acquired/pidfd path above already returned from inside the locked + // block (C1). sandboxToKill is identity-bound via the owned shared_ptr, + // so - unlike the pidfd branch - it remains safe to call Kill() outside + // s_Mutex, unchanged from before this fix wave. + + // Rolls the registry entry's s_State back to what it was before this + // call optimistically set it to E_TerminationRequested, but only if the + // entry still matches BOTH the captured generation AND the expected + // in-flight state (I3) - guards against a stale rollback clobbering a + // different (newer) registration that reused this numeric PID after the + // original entry was reaped and erased, and that newer registration + // happens to also currently be E_TerminationRequested. + const auto rollBackState = [this, pid, previousState, capturedGeneration]() { + std::lock_guard lock(m_PidRegistry->s_Mutex); + const auto it = m_PidRegistry->s_Children.find(pid); + if (it != m_PidRegistry->s_Children.end() && it->second.s_Generation == capturedGeneration && + it->second.s_State == EChildLifecycleState::E_TerminationRequested) { + it->second.s_State = previousState; + } + }; + + try { + // Locked design decision: MonitorBase::Kill() takes no + // signal parameter and hard-codes SIGKILL - this is the ENOSYS + // forced-kill fallback, never a SIGTERM-via-monitor path. + sandboxToKill->Kill(); + } catch (const std::exception& e) { + LOG_ERROR(<< "Sandbox2::Kill() failed for sandboxed child PID " << pid + << ": " << e.what()); + rollBackState(); + return false; + } + return true; +} + +#else // !SANDBOX2_AVAILABLE + +bool CSandboxedProcessSpawner::terminateChild(core::CProcess::TPid /* pid */) { + return false; +} + +#endif // SANDBOX2_AVAILABLE + +bool CSandboxedProcessSpawner::hasChild(core::CProcess::TPid pid) const { + std::lock_guard lock(m_PidRegistry->s_Mutex); + const auto it = m_PidRegistry->s_Children.find(pid); + return it != m_PidRegistry->s_Children.end() && + it->second.s_State != EChildLifecycleState::E_Reaped && + it->second.s_State != EChildLifecycleState::E_Failed; +} + +} // namespace sandbox +} // namespace ml diff --git a/lib/sandbox/unittest/CMakeLists.txt b/lib/sandbox/unittest/CMakeLists.txt index c0ad8e0a0..c29e1fc1e 100644 --- a/lib/sandbox/unittest/CMakeLists.txt +++ b/lib/sandbox/unittest/CMakeLists.txt @@ -43,6 +43,7 @@ if(TARGET sandbox2::sandbox2 AND CMAKE_SYSTEM_NAME STREQUAL "Linux") # are unavailable on non-Linux configure runs. list(APPEND SRCS CSandboxForkserverSmokeTest.cc) list(APPEND SRCS CPytorchInferenceSandboxPolicyMechanismTest_Linux.cc) + list(APPEND SRCS CSandboxedProcessSpawnerLifecycleTest_Linux.cc) list(APPEND ML_LINK_LIBRARIES sandbox2::sandbox2) # Deliberately-dependency-free sandboxee payload for the smoke test above. @@ -73,6 +74,28 @@ if(TARGET sandbox2::sandbox2 AND CMAKE_SYSTEM_NAME STREQUAL "Linux") POSITION_INDEPENDENT_CODE TRUE RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/payloads ) + + # Long-lived sandboxee for CSandboxedProcessSpawnerLifecycleTest_Linux + # (Task 4). Unlike the two payloads above, this one is launched through + # CSandboxedProcessSpawner::spawn() itself (not a hand-built Sandbox2 + # policy), which derives its filesystem policy's binDir/libDir from the + # payload's own resolved path: binDir is this payload's directory + # (payloads/) and libDir is binDir's *sibling* "lib" directory + # (${CMAKE_CURRENT_BINARY_DIR}/lib), matching the /bin + + # /lib pytorch_inference distribution layout spawn() assumes. + # That sibling directory does not otherwise exist in the unit test build + # tree; create it at configure time so PolicyBuilder::AddDirectory() never + # has to bind-mount a missing path. Empty is fine - the payload's actual + # shared-library dependencies (libc, libpthread, ld-linux) resolve via the + # fixed /lib, /lib64, /usr/lib, /usr/lib64 mounts the policy already adds. + file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/lib) + add_executable(lifecycle_signal_payload EXCLUDE_FROM_ALL + payloads/lifecycle_signal_payload.cc + ) + set_target_properties(lifecycle_signal_payload PROPERTIES + POSITION_INDEPENDENT_CODE TRUE + RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/payloads + ) endif() ml_add_test_executable(sandbox ${SRCS}) @@ -90,3 +113,10 @@ if(TARGET ml_sandbox_probe) ML_SANDBOX2_PROBE_PAYLOAD="$" ) endif() + +if(TARGET lifecycle_signal_payload) + add_dependencies(ml_test_sandbox lifecycle_signal_payload) + target_compile_definitions(ml_test_sandbox PRIVATE + ML_SANDBOX2_LIFECYCLE_PAYLOAD="$" + ) +endif() diff --git a/lib/sandbox/unittest/CSandboxedProcessSpawnerLifecycleTest_Linux.cc b/lib/sandbox/unittest/CSandboxedProcessSpawnerLifecycleTest_Linux.cc new file mode 100644 index 000000000..411f8f257 --- /dev/null +++ b/lib/sandbox/unittest/CSandboxedProcessSpawnerLifecycleTest_Linux.cc @@ -0,0 +1,1188 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the following additional limitation. Functionality enabled by the + * files subject to the Elastic License 2.0 may only be used in production when + * invoked by an Elasticsearch process with a license key installed that permits + * use of machine learning features. You may not use this file except in + * compliance with the Elastic License 2.0 and the foregoing additional + * limitation. + */ + +// Linux-only lifecycle test for CSandboxedProcessSpawner (Task 4). Drives the +// spawner's four injectable seams (TPidFdOpenFn, TRegistryInsertFn, +// TMonitorLaunchFn, TAwaitResultFn - see CSandboxedProcessSpawner.h) to +// exercise fault-injection and race scenarios deterministically, but there +// is no seam that bypasses Sandbox2::RunAsync() itself: every test case +// below performs one genuine spawn() of a real, minimal, dependency-free +// payload (lifecycle_signal_payload.cc) under the real filesystem policy +// spawn() builds. The registry-insert seam receives a mutable reference to +// the spawner's *actual* internal SPidRegistry (not a copy), which every +// test below uses after that one real spawn() to fabricate/mutate further +// registry state directly - this is the only externally reachable handle to +// that private registry, since the spawner has no accessor for it and no +// constructor overload accepts a caller-supplied one. +// +// No sleep()/wall-clock polling anywhere in this file. Timing-sensitive +// races are driven either by directly exercising +// CSandboxedProcessSpawner::CCasOutcomeLatch (a pure, thread-safe type, see +// gate 5), by manually invoking a *captured* monitor-body callable on the +// calling thread instead of ever starting a background thread for it, or - +// where a genuine background thread is required (gates 7 and 8) - by a +// std::promise/future gate the test controls explicitly. The one bounded +// wait that has no other synchronisation primitive available (observing +// "still alive" - the absence of an event) uses a single poll() call with a +// timeout, never a sleep-and-recheck loop. +// +// NOT YET RUN: like CPytorchInferenceSandboxPolicyMechanismTest_Linux, this +// file has not executed on a real Linux+Sandbox2 host in this session (the +// authoring host is macOS, and the Sandbox2 headers are fetched at CMake +// configure time, not vendored in this checkout). Open concern: the +// gate-7 orphan-cleanup half (whether an abandoned sandboxee is eventually +// reaped by something else in the system, once the controller exits) is +// deliberately out of scope for this file - see the ruling above gate 7's +// test case. + +#include + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#ifndef ML_SANDBOX2_LIFECYCLE_PAYLOAD +#error "ML_SANDBOX2_LIFECYCLE_PAYLOAD must be defined by lib/sandbox/unittest/CMakeLists.txt" +#endif + +#include "sandboxed_api/sandbox2/result.h" +#include "sandboxed_api/sandbox2/sandbox2.h" + +// Same rationale as CSandboxedProcessSpawner_Linux.cc's identical fallback: +// the CentOS 7 CI build image's kernel headers may predate pidfd_open, but +// this is the same syscall number (434) on every architecture ml-cpp +// builds for. Used here purely for *test-owned observation* pidfds (poll() +// for exit, never a signal) - never to signal a spawner-owned child. +#ifdef __NR_pidfd_open +#define ML_TEST_NR_pidfd_open __NR_pidfd_open +#else +#define ML_TEST_NR_pidfd_open 434 +#endif + +namespace { + +using ml::sandbox::CSandboxedProcessSpawner; +using TSpawner = CSandboxedProcessSpawner; +using TPid = ml::core::CProcess::TPid; + +// --------------------------------------------------------------------- +// Descriptor-count baseline helpers (gate 6). +// --------------------------------------------------------------------- + +//! Number of open file descriptors this process currently holds, via +//! /proc/self/fd (Linux-only, fine - this whole translation unit is +//! Linux-gated). Excludes "." and "..", includes the directory fd opendir() +//! itself just opened (consistently, on both the "before" and "after" +//! snapshot, so it cancels out). +std::size_t openFdCount() { + DIR* dir = ::opendir("/proc/self/fd"); + BOOST_TEST_REQUIRE(dir != nullptr); + std::size_t count{0}; + struct dirent* entry{nullptr}; + while ((entry = ::readdir(dir)) != nullptr) { + const std::string name{entry->d_name}; + if (name != "." && name != "..") { + ++count; + } + } + ::closedir(dir); + return count; +} + +//! Forward-declared here, defined further down once its own helpers +//! (makeChildIpcRoot, forcedPidFdOutcome, ...) are in scope. Starts the +//! lazily-created global Sandbox2 forkserver exactly once, no matter how +//! many test cases construct SFdBaselineFixture, so that its comms +//! descriptors are already open (and therefore already part of the +//! baseline) before the *first* test case's fd count is snapshotted. +void warmUpForkserverOnce(); + +//! Applied to the whole suite: every test case must leave the process with +//! exactly the descriptors it started with - no leaked pidfd, socketpair, or +//! Sandbox2 comms descriptor survives a spawn/terminate/cleanup cycle. +//! +//! Runs a one-time warm-up spawn (see warmUpForkserverOnce(), defined after +//! the helpers it needs) the first time this fixture is constructed, i.e. +//! for the very first test case that actually runs - never during Boost.Test's +//! own module/framework initialisation. An earlier version did this warm-up +//! via BOOST_GLOBAL_FIXTURE instead: that constructor runs before Boost.Test +//! has finished setting up its own test-tree/observer state, and forking a +//! real process that deep inside framework init corrupted that state (the +//! module reported "Incorrect setup: no test case executed" after every test +//! case had genuinely passed). Doing the warm-up lazily, inside the first +//! ordinary per-case fixture construction, avoids that entirely. +struct SFdBaselineFixture { + SFdBaselineFixture() + : s_Baseline((warmUpForkserverOnce(), openFdCount())) {} + ~SFdBaselineFixture() { BOOST_CHECK_EQUAL(openFdCount(), s_Baseline); } + std::size_t s_Baseline; +}; + +// --------------------------------------------------------------------- +// $TMPDIR / child-IPC-root scaffolding, matching the interlock +// (validateChildIpcLaunchSpec) spawn() enforces before building a policy - +// see CPytorchInferenceSandboxPolicyMechanismTest_Linux.cc for the same +// pattern used directly against the validation function. +// --------------------------------------------------------------------- + +int removeEntryBestEffort(const char* fpath, const struct stat*, int typeflag, struct FTW*) { + if (typeflag == FTW_DP) { + ::rmdir(fpath); + } else { + ::unlink(fpath); + } + return 0; +} + +void removeTreeBestEffort(const std::string& path) { + ::nftw(path.c_str(), removeEntryBestEffort, 16, FTW_DEPTH | FTW_PHYS); +} + +//! RAII: creates a fresh, private tmp directory and points $TMPDIR at it for +//! the lifetime of this object, so spawn()'s own trustedTmpDir +//! (getenv("TMPDIR") or "/tmp") matches exactly the root this test builds +//! its child-IPC directories under - giving each test case an isolated +//! ml-child-ipc root instead of colliding on a shared /tmp/ml-child-ipc. +class CScopedTmpDirEnv { +public: + CScopedTmpDirEnv() { + char tmpl[] = "/tmp/ml_sandbox_lifecycle_XXXXXX"; + char* dir = ::mkdtemp(tmpl); + BOOST_TEST_REQUIRE(dir != nullptr); + m_Dir = dir; + const char* previous = ::getenv("TMPDIR"); + if (previous != nullptr) { + m_PreviousTmpDir = previous; + m_HadPrevious = true; + } + ::setenv("TMPDIR", m_Dir.c_str(), 1); + } + ~CScopedTmpDirEnv() { + if (m_HadPrevious) { + ::setenv("TMPDIR", m_PreviousTmpDir.c_str(), 1); + } else { + ::unsetenv("TMPDIR"); + } + removeTreeBestEffort(m_Dir); + } + CScopedTmpDirEnv(const CScopedTmpDirEnv&) = delete; + CScopedTmpDirEnv& operator=(const CScopedTmpDirEnv&) = delete; + const std::string& dir() const { return m_Dir; } + +private: + std::string m_Dir; + std::string m_PreviousTmpDir; + bool m_HadPrevious{false}; +}; + +//! Creates $TMPDIR/ml-child-ipc/ (mode 0700), matching the layout +//! the native controller is responsible for creating, and returns its +//! path. +std::string makeChildIpcRoot(const std::string& trustedTmpDir, const std::string& childId) { + const std::string mlChildIpc{trustedTmpDir + "/ml-child-ipc"}; + ::mkdir(mlChildIpc.c_str(), 0700); // may already exist from an earlier case in this dir; ignore. + const std::string childRoot{mlChildIpc + "/" + childId}; + BOOST_TEST_REQUIRE(::mkdir(childRoot.c_str(), 0700) == 0); + return childRoot; +} + +//! One recognized path-bearing launch option is enough to satisfy +//! validateChildIpcLaunchSpec's s_Ok requirement (at least one present and +//! accepted) - the leaf file need not exist on disk (only its parent +//! directory is canonicalized). +std::vector childIpcArgs(const std::string& childRoot) { + return {"--input=" + childRoot + "/input.fifo"}; +} + +// --------------------------------------------------------------------- +// Test-owned pidfd observation (never signalling) - used only to answer +// "did this PID exit yet", never to terminate a spawner-owned child by +// numeric PID. +// --------------------------------------------------------------------- + +int testPidfdOpen(pid_t pid) { + return static_cast(::syscall(ML_TEST_NR_pidfd_open, pid, 0u)); +} + +//! Single bounded poll() call (not a sleep/recheck loop): returns true if +//! the pidfd became readable (the process exited) within timeoutMs, false +//! on timeout (process presumably still running). +bool pidfdReadableWithin(int pidfd, int timeoutMs) { + struct pollfd pfd {}; + pfd.fd = pidfd; + pfd.events = POLLIN; + const int rc = ::poll(&pfd, 1, timeoutMs); + return rc > 0 && (pfd.revents & POLLIN) != 0; +} + +// --------------------------------------------------------------------- +// Seam factories. Each mirrors just enough of the corresponding production +// default (see CSandboxedProcessSpawner_Linux.cc's defaultRegistryInsert +// etc.) to keep spawn() on its normal success path, while also handing the +// test a way to observe or control what happened. +// --------------------------------------------------------------------- + +//! Registry-insert seam that behaves like the production default (lock, +//! allocate the next generation, insert) and additionally captures a +//! non-owning pointer to the live SPidRegistry plus copies of the inserted +//! entry's pid/pidfd/Sandbox2 handle. Any output parameter may be nullptr +//! if the caller does not need it. The captured SPidRegistry* stays valid +//! for as long as something keeps the underlying shared_ptr +//! alive - normally the owning spawner, and after the owning spawner is +//! destroyed, only the monitor thread's own shared_ptr copy +//! (co-owned by design, since the monitor can outlive the spawner) - see +//! gate 8's test case comment for the one place this matters. +TSpawner::TRegistryInsertFn +capturingRegistryInsert(TSpawner::SPidRegistry** capturedRegistry, + TPid* capturedPid, + int* capturedPidFd, + std::shared_ptr* capturedSandbox) { + return [=](TSpawner::SPidRegistry& registry, TPid pid, + TSpawner::SSandboxedChild child) -> std::uint64_t { + if (capturedRegistry != nullptr) { + *capturedRegistry = ®istry; + } + if (capturedPid != nullptr) { + *capturedPid = pid; + } + if (capturedPidFd != nullptr) { + *capturedPidFd = child.s_PidFd; + } + if (capturedSandbox != nullptr) { + *capturedSandbox = child.s_Sandbox; + } + std::lock_guard lock(registry.s_Mutex); + const std::uint64_t generation{++registry.s_NextGeneration}; + child.s_Generation = generation; + child.s_State = TSpawner::EChildLifecycleState::E_Registered; + registry.s_Children[pid] = std::move(child); + return generation; + }; +} + +//! pidfd-acquisition seam that ignores the real pidfd_open syscall entirely +//! and returns a fixed, caller-chosen SPidFdAcquisitionResult - used to +//! force ENOSYS/ESRCH/EMFILE/ENFILE/"other" classifications deterministically, +//! independent of what the real, presumably-modern, CI kernel would +//! actually report. +TSpawner::TPidFdOpenFn forcedPidFdOutcome(TSpawner::SPidFdAcquisitionResult toReturn, + TPid* capturedPid = nullptr) { + return [=](TPid pid) -> TSpawner::SPidFdAcquisitionResult { + if (capturedPid != nullptr) { + *capturedPid = pid; + } + return toReturn; + }; +} + +//! Monitor-launch seam that never starts a thread: it just hands the real +//! monitorBody callable spawn() built (complete with its captured +//! registry/sandbox/generation/awaitResultFn closure) back to the test via +//! capturedBody, and reports success. The test then decides exactly when - +//! or whether - to invoke it, on whatever thread it chooses (usually the +//! test's own calling thread), which is what makes gates 1, 3, 4 and 6 +//! below fully deterministic without ever starting a background thread. +TSpawner::TMonitorLaunchFn captureMonitorBodyWithoutRunning(std::function* capturedBody) { + return [capturedBody](std::function body) -> bool { + *capturedBody = std::move(body); + return true; + }; +} + +//! Monitor-launch seam that DOES start a genuine background thread (like +//! the production default), but additionally signals donePromise once the +//! monitor body - including its registry cleanup - has fully returned, so +//! a test can block deterministically until that has happened without +//! polling or joining the (deliberately detached, since the monitor thread +//! must be able to outlive the spawner) thread itself. +//! +//! donePromise is heap-owned (shared_ptr), matching the precedent already +//! established for the ENOSYS case above (capturingAwaitResult's caller): +//! gates 7 and 8 both call spawn() before blocking on the returned future, +//! but if a BOOST_TEST_REQUIRE between spawn() and the wait throws, the +//! stack-local std::promise a raw pointer would have pointed at is +//! destroyed while this detached thread is still running and will later +//! call donePromise->set_value() on freed stack memory - self-review round +//! 2, finding 4. +//! +//! bodyKeepAliveOut is an optional extra output: when non-null, the seam +//! hands the caller its OWN shared_ptr> reference to the +//! monitor closure (self-review round 2, finding 2). Gate 8 needs this: it +//! destroys the spawner and then dereferences a raw SPidRegistry* obtained +//! from this same closure's registry-insert seam. That raw pointer is only +//! valid for as long as SOME shared_ptr copy - normally the +//! monitor closure's own - is still alive. Previously the detached thread +//! destroyed its only copy of the closure immediately after running it and +//! before signalling completion, so by the time gate 8's test thread woke +//! up and dereferenced the raw pointer, the registry had already been +//! freed (a deterministic UAF, not merely racy). Handing the test its own +//! extra reference here means the object survives regardless of when the +//! detached thread releases its own copy. This does not change the +//! fd-baseline story: gates 7/8 already keep the Sandbox2 handle alive via +//! their own `capturedSandbox` copy for the same span, so this reference +//! extends nothing that wasn't already being kept alive. +TSpawner::TMonitorLaunchFn realMonitorLaunchWithCompletionSignal( + std::shared_ptr> donePromise, + std::shared_ptr>* bodyKeepAliveOut = nullptr) { + return [donePromise, bodyKeepAliveOut](std::function body) -> bool { + auto bodyPtr = std::make_shared>(std::move(body)); + if (bodyKeepAliveOut != nullptr) { + *bodyKeepAliveOut = bodyPtr; + } + std::thread([bodyPtr, donePromise]() mutable { + (*bodyPtr)(); + bodyPtr.reset(); // release this thread's reference BEFORE signalling, so a waiter + // observing "done" is guaranteed this thread no longer holds the + // closure (and therefore the Sandbox2 handle it captured) - restores + // the ordering guarantee an earlier round's I5 fix established + // (self-review round 3, finding S1). + donePromise->set_value(); + }) + .detach(); + return true; + }; +} + +//! AwaitResult seam that always delegates to the real +//! sandbox2::Sandbox2::AwaitResult() (never fabricates a sandbox2::Result - +//! its constructor is not part of any header available in this checkout) +//! and additionally stashes a copy for the test to +//! inspect afterward, since production code only ever uses the result for +//! logging and never exposes it. +TSpawner::TAwaitResultFn capturingAwaitResult(std::shared_ptr* capturedResult) { + return [capturedResult](sandbox2::Sandbox2& sandbox) -> sandbox2::Result { + sandbox2::Result result{sandbox.AwaitResult()}; + if (capturedResult != nullptr) { + *capturedResult = std::make_shared(result); + } + return result; + }; +} + +// --------------------------------------------------------------------- +// I6: forkserver / fork() warm-up, run once before ANY per-case fixture. +// --------------------------------------------------------------------- + +//! Sandbox2's global forkserver is created lazily on the first RunAsync() +//! anywhere in this process, and holds its own comms descriptors for the +//! rest of the process's lifetime. SFdBaselineFixture (above) snapshots the +//! fd count before each case's first spawn(); if this test binary/suite +//! ever runs with this suite as the FIRST thing to spawn anything in the +//! whole process (e.g. via `--run_test=` filtering, or a future link-order +//! change), the first case's fd-baseline check would see the forkserver's +//! descriptors appear mid-case and spuriously fail. This is the same root +//! cause as the "gate 4 fork() implicit test-ordering dependency" concern +//! (gate 4 also forks - see testTerminateChildSignalsOnlyTheCurrentlyRegisteredIdentity +//! - and pays the same one-time lazy-init cost the first time anything in +//! this binary spawns or forks) - fixed once, here, for both. +//! +//! Runs once no matter how many test cases construct SFdBaselineFixture: +//! std::call_once guards the actual warm-up spawn behind a static flag, so +//! the real work happens only the first time a test case's fixture runs - +//! never during Boost.Test's own module/framework initialisation. An +//! earlier version did this warm-up in a BOOST_GLOBAL_FIXTURE constructor +//! instead: that constructor runs before Boost.Test has finished setting up +//! its own test-tree/observer state, and forking a real process that deep +//! inside framework init corrupted that state (the module reported +//! "Incorrect setup: no test case executed" after every test case had +//! genuinely passed, even though the run itself succeeded). Running the +//! warm-up lazily, from inside the first ordinary per-case fixture +//! construction, avoids that entirely while keeping the same guarantee: +//! the forkserver's comms descriptors are already open by the time any +//! case's own baseline is captured. +void warmUpForkserverOnce() { + static std::once_flag flag; + std::call_once(flag, []() { + CScopedTmpDirEnv tmpEnv; + const std::string childRoot{makeChildIpcRoot(tmpEnv.dir(), "forkserver-warmup")}; + + std::shared_ptr capturedResult; + std::function monitorBody; + // ENOSYS forces the Sandbox2::Kill() termination path below (rather + // than requiring a real pidfd_send_signal/SIGTERM round-trip), + // keeping this warm-up simple and unconditional regardless of what + // the real kernel supports. + TSpawner::TPidFdOpenFn pidFdOpen = forcedPidFdOutcome({-1, ENOSYS}); + TSpawner::TMonitorLaunchFn monitorLaunch = + captureMonitorBodyWithoutRunning(&monitorBody); + TSpawner::TAwaitResultFn awaitResultFn = capturingAwaitResult(&capturedResult); + + TSpawner spawner{pidFdOpen, TSpawner::TRegistryInsertFn{}, monitorLaunch, awaitResultFn}; + TPid childPid{0}; + // Best-effort: if this somehow fails, every real test case's own + // spawn() will surface the underlying problem on its own merits - + // this warm-up only exists to make the FIRST case's fd baseline + // deterministic, not to assert anything itself. + if (spawner.spawn(ML_SANDBOX2_LIFECYCLE_PAYLOAD, childIpcArgs(childRoot), childPid) && + childPid > 0) { + // Only await completion if termination was actually requested + // successfully: if Sandbox2::Kill() threw and terminateChild() + // returned false, the sandboxee may still be running, and an + // unconditional monitorBody() call would block this call - + // and therefore the first test case that triggers it - inside + // AwaitResult() with no bound and no diagnostic (production's + // wall-time limit is unbounded). + if (spawner.terminateChild(childPid) && monitorBody) { + monitorBody(); // real cleanup path: closes the pidfd, erases the entry. + } + } + }); +} + +} // namespace + +BOOST_FIXTURE_TEST_SUITE(CSandboxedProcessSpawnerLifecycleTest_Linux, SFdBaselineFixture) + +// ===================================================================== +// Gate 1: every pidfd classification. +// ===================================================================== + +//! classifyPidFdOutcome() is pure and platform-independent (no syscalls, no +//! Sandbox2 types) - exercised exhaustively here with no spawn() at all, +//! covering every classification and a representative errno for each of +//! the two failure buckets, including one genuinely "other" errno (EPERM) +//! that is neither ENOSYS nor one of the two resource-exhaustion examples +//! the brief names (ESRCH/EMFILE/ENFILE all also asserted explicitly). +BOOST_AUTO_TEST_CASE(testClassifyPidFdOutcomeExhaustive) { + using EOutcome = TSpawner::EPidFdOutcome; + BOOST_CHECK(TSpawner::classifyPidFdOutcome({3, 0}) == EOutcome::E_Acquired); + BOOST_CHECK(TSpawner::classifyPidFdOutcome({0, 0}) == EOutcome::E_Acquired); + BOOST_CHECK(TSpawner::classifyPidFdOutcome({-1, ENOSYS}) == EOutcome::E_KernelUnsupported); + BOOST_CHECK(TSpawner::classifyPidFdOutcome({-1, ESRCH}) == EOutcome::E_Failed); + BOOST_CHECK(TSpawner::classifyPidFdOutcome({-1, EMFILE}) == EOutcome::E_Failed); + BOOST_CHECK(TSpawner::classifyPidFdOutcome({-1, ENFILE}) == EOutcome::E_Failed); + BOOST_CHECK(TSpawner::classifyPidFdOutcome({-1, EPERM}) == EOutcome::E_Failed); // "other" +} + +//! Every non-success, non-ENOSYS classification must fail spawn() outright +//! rather than register a child with an undefined termination +//! fallback. Runs each of ESRCH/EMFILE/ENFILE/EPERM through the real +//! spawn() path via the pidfd seam. +BOOST_AUTO_TEST_CASE(testSpawnFailsClosedOnEveryNonKernelUnsupportedPidfdFailure) { + const int errnosToTry[] = {ESRCH, EMFILE, ENFILE, EPERM}; + for (int forcedErrno : errnosToTry) { + CScopedTmpDirEnv tmpEnv; + const std::string childRoot{makeChildIpcRoot( + tmpEnv.dir(), std::string("case1-failed-") + std::to_string(forcedErrno))}; + + TPid capturedPid{0}; + TSpawner::TPidFdOpenFn pidFdOpen = forcedPidFdOutcome({-1, forcedErrno}, &capturedPid); + TSpawner spawner{pidFdOpen, TSpawner::TRegistryInsertFn{}, + TSpawner::TMonitorLaunchFn{}, TSpawner::TAwaitResultFn{}}; + + TPid childPid{0}; + const bool spawned = spawner.spawn(ML_SANDBOX2_LIFECYCLE_PAYLOAD, + childIpcArgs(childRoot), childPid); + + BOOST_TEST_REQUIRE(spawned == false); // negative assertion + BOOST_CHECK_EQUAL(childPid, 0); + BOOST_TEST_REQUIRE(capturedPid > 0); // reached marker: the seam was invoked with a real pid + BOOST_CHECK(spawner.hasChild(capturedPid) == false); + + // Mechanism assertion: no registry entry exists to terminate, so + // there is nothing to call terminateChild() against, and no pidfd + // seam was ever consulted a second time. Cleanup assertion: the + // kill-and-reap guard ran synchronously during spawn()'s stack + // unwind (before spawn() returned), so the real sandboxee should + // already be gone - confirm via a test-owned observer pidfd, + // never a signal. + // I4: a fresh pidfd_open() on a PID the kill-and-reap guard has + // already Kill()ed and AwaitResult()ed can legitimately fail with + // ESRCH (fully reaped already - the common case) rather than + // succeed, so accept both outcomes as proof of cleanup instead of + // requiring a live pidfd. + const int observerPidFd{testPidfdOpen(capturedPid)}; + if (observerPidFd < 0) { + BOOST_CHECK_EQUAL(errno, ESRCH); // already fully reaped - this IS proof of cleanup + } else { + BOOST_CHECK(pidfdReadableWithin(observerPidFd, 3000)); + ::close(observerPidFd); + } + } +} + +//! ENOSYS classification: terminateChild() must fall back to +//! Sandbox2::Kill() (hard-coded SIGKILL, uncatchable). There is no seam +//! around Kill() itself (unlike AwaitResult()), so this cannot be verified +//! via a spy on the call. Instead this asserts the +//! only externally observable effect Kill()/SIGKILL and +//! pidfd_send_signal()/SIGTERM can be told apart by: the payload installs a +//! SIGTERM handler that does nothing and keeps running, so only an +//! uncatchable signal can end it - if it dies, SIGKILL (via Kill()) must +//! have been what ended it. +BOOST_AUTO_TEST_CASE(testTerminateChildFallsBackToKillWhenKernelUnsupportsPidfd) { + CScopedTmpDirEnv tmpEnv; + const std::string childRoot{makeChildIpcRoot(tmpEnv.dir(), "case1-enosys")}; + + TSpawner::SPidRegistry* registry{nullptr}; + std::function monitorBody; + + // Heap-owned box for the captured sandbox2::Result, not a plain stack + // local: monitorBody() below is run with a bounded wait (fixing the + // review finding that a terminateChild() regression to a no-op would + // otherwise hang this call forever, since production AwaitResult() has + // no wall-clock bound of its own - see spawn()'s + // set_walltime_limit(absl::ZeroDuration()) in + // CSandboxedProcessSpawner_Linux.cc, and there is no seam to override it + // for just this test). If the wait times out, the still-running + // background thread is detached rather than joined (so this test case, + // and the whole suite, fails fast instead of hanging) - anything that + // thread can still touch after this function returns must therefore + // live on the heap, not on this stack frame. + auto capturedResult = std::make_shared>(); + + TSpawner::TPidFdOpenFn pidFdOpen = forcedPidFdOutcome({-1, ENOSYS}); + TSpawner::TRegistryInsertFn insertFn = + capturingRegistryInsert(®istry, nullptr, nullptr, nullptr); + TSpawner::TMonitorLaunchFn monitorLaunch = captureMonitorBodyWithoutRunning(&monitorBody); + TSpawner::TAwaitResultFn awaitResultFn = capturingAwaitResult(capturedResult.get()); + + TSpawner spawner{pidFdOpen, insertFn, monitorLaunch, awaitResultFn}; + TPid childPid{0}; + BOOST_TEST_REQUIRE(spawner.spawn(ML_SANDBOX2_LIFECYCLE_PAYLOAD, + childIpcArgs(childRoot), childPid)); + BOOST_TEST_REQUIRE(childPid > 0); + BOOST_CHECK(spawner.hasChild(childPid)); // positive control / reached marker + + BOOST_TEST_REQUIRE(spawner.terminateChild(childPid)); + + BOOST_TEST_REQUIRE(static_cast(monitorBody)); + + // Run the real cleanup path (calls the injected AwaitResult() exactly + // once) on a separate thread, bounded by a std::promise/future wait - + // same synchronization primitive gate 8 already uses in this file, just + // with a timeout instead of an unconditional wait(), since here nothing + // else in the test independently guarantees the payload will ever die. + auto monitorDonePromise = std::make_shared>(); + std::future monitorDoneFuture{monitorDonePromise->get_future()}; + std::thread monitorThread( + [ body = monitorBody, monitorDonePromise, capturedResult ]() mutable { + body(); + monitorDonePromise->set_value(); + }); + const std::future_status waitStatus{monitorDoneFuture.wait_for(std::chrono::seconds(5))}; + if (waitStatus == std::future_status::ready) { + monitorThread.join(); + } else { + // Regression path: terminateChild()'s E_KernelUnsupported branch + // apparently didn't actually end the payload (e.g. sent the wrong + // signal, or Kill() regressed to a no-op), so the injected + // AwaitResult() is still blocked with no bound of its own. Detach + // instead of join() so this test fails on the assertion below + // within a few seconds rather than hanging indefinitely - every + // object the thread can still reach (monitorDonePromise, and + // monitorBody's own closure, copied above) is heap-owned via + // shared_ptr/std::function-by-value. Critically, capturedResult + // (the outer shared_ptr) is ALSO captured by value into this + // lambda: capturingAwaitResult() only holds a raw pointer into the + // heap-allocated inner shared_ptr, baked into + // monitorBody/awaitResultFn's closure by value, so without a + // shared_ptr copy of capturedResult riding along in this thread's + // own capture list, BOOST_TEST_REQUIRE below failing/unwinding this + // stack frame would drop the last reference and free the object + // out from under the still-running detached thread - a + // use-after-free once the real AwaitResult() unblocks and writes + // through that raw pointer. Capturing capturedResult here keeps it + // alive for as long as the detached thread might still run, + // independent of this function's own lifetime. + monitorThread.detach(); + } + // Fails fast (instead of hanging) if terminateChild() regressed to + // never actually killing the payload: a timeout here IS the failure, + // not a hang. Plain BOOST_REQUIRE, not BOOST_TEST_REQUIRE: the latter + // tries to stream both operands for its failure message, and + // std::future_status has no operator<<. + BOOST_REQUIRE(waitStatus == std::future_status::ready); + + BOOST_TEST_REQUIRE(*capturedResult != nullptr); + // Self-review round 2, finding 1: Sandbox2::Kill() does not produce a + // WIFSIGNALED-style SIGNALED/SIGKILL result. It sets the monitor's + // external-kill flag, and the monitor's status classification (pinned + // sandboxed-api v20241008, monitor_ptrace.cc) checks that flag AHEAD of + // the WIFSIGNALED path, so a Kill()ed sandboxee is reported as + // EXTERNAL_KILL with reason_code() == 0, never SIGNALED/SIGKILL. + // EXTERNAL_KILL is actually the STRONGER discriminator here: it is only + // reachable via Sandbox2::Kill(), whereas SIGNALED could also be + // produced by an external SIGKILL unrelated to this mechanism. + BOOST_CHECK((*capturedResult)->final_status() == sandbox2::Result::EXTERNAL_KILL); // mechanism: Kill() + BOOST_CHECK((*capturedResult)->reason_code() == 0); + BOOST_CHECK(registry->s_Children.count(childPid) == 0); // cleanup assertion +} + +//! E_Acquired classification (the un-forced, real-kernel path on any modern +//! CI host): terminateChild() must use pidfd_send_signal(SIGTERM), which +//! the payload's handler catches and survives - the negative assertion +//! (never Kill()/SIGKILL) is that the process is demonstrably still alive +//! afterward. +BOOST_AUTO_TEST_CASE(testTerminateChildUsesPidfdSignalWhenAcquiredAndChildSurvives) { + CScopedTmpDirEnv tmpEnv; + const std::string childRoot{makeChildIpcRoot(tmpEnv.dir(), "case1-acquired")}; + + TSpawner::SPidRegistry* registry{nullptr}; + int capturedPidFd{-1}; + std::shared_ptr capturedSandbox; + std::shared_ptr capturedResult; + std::function monitorBody; + + TSpawner::TRegistryInsertFn insertFn = + capturingRegistryInsert(®istry, nullptr, &capturedPidFd, &capturedSandbox); + TSpawner::TMonitorLaunchFn monitorLaunch = captureMonitorBodyWithoutRunning(&monitorBody); + TSpawner::TAwaitResultFn awaitResultFn = capturingAwaitResult(&capturedResult); + + // Left as the default (empty) seam: the real kernel's pidfd_open() is + // expected to succeed (E_Acquired) on any CI host new enough to build + // Sandbox2 at all - this is the natural, un-forced positive control. + TSpawner spawner{TSpawner::TPidFdOpenFn{}, insertFn, monitorLaunch, awaitResultFn}; + TPid childPid{0}; + BOOST_TEST_REQUIRE(spawner.spawn(ML_SANDBOX2_LIFECYCLE_PAYLOAD, + childIpcArgs(childRoot), childPid)); + BOOST_TEST_REQUIRE(childPid > 0); + BOOST_TEST_REQUIRE(capturedPidFd >= 0); // confirms the real kernel classified E_Acquired + + BOOST_TEST_REQUIRE(spawner.terminateChild(childPid)); // positive control + + // Negative + mechanism assertion, single bounded poll(), not a + // sleep/recheck loop: the process must still be alive. + const int observerPidFd{testPidfdOpen(childPid)}; + BOOST_TEST_REQUIRE(observerPidFd >= 0); + BOOST_CHECK(pidfdReadableWithin(observerPidFd, 1500) == false); + ::close(observerPidFd); + + // Cleanup: the payload never exits on its own; reap it via the + // identity-bound Sandbox2 handle (never a numeric ::kill()) and run + // the real cleanup path. + BOOST_TEST_REQUIRE(capturedSandbox != nullptr); + capturedSandbox->Kill(); + BOOST_TEST_REQUIRE(static_cast(monitorBody)); + monitorBody(); + BOOST_TEST_REQUIRE(capturedResult != nullptr); + BOOST_CHECK(registry->s_Children.count(childPid) == 0); +} + +// ===================================================================== +// Gate 2: allocation/resource failure. +// ===================================================================== + +BOOST_AUTO_TEST_CASE(testRegistryInsertBadAllocKillsAndReapsCleanly) { + CScopedTmpDirEnv tmpEnv; + const std::string childRoot{makeChildIpcRoot(tmpEnv.dir(), "case2a")}; + + TPid capturedPid{0}; + TSpawner::TPidFdOpenFn pidFdOpen = forcedPidFdOutcome({-1, ENOSYS}, &capturedPid); + TSpawner::TRegistryInsertFn throwingInsert = + [](TSpawner::SPidRegistry&, TPid, TSpawner::SSandboxedChild) -> std::uint64_t { + throw std::bad_alloc(); + }; + + TSpawner spawner{pidFdOpen, throwingInsert, TSpawner::TMonitorLaunchFn{}, + TSpawner::TAwaitResultFn{}}; + TPid childPid{0}; + const bool spawned = spawner.spawn(ML_SANDBOX2_LIFECYCLE_PAYLOAD, + childIpcArgs(childRoot), childPid); + + BOOST_TEST_REQUIRE(spawned == false); + BOOST_CHECK_EQUAL(childPid, 0); // no live unowned child, no registry entry, no leaked descriptor + BOOST_TEST_REQUIRE(capturedPid > 0); + BOOST_CHECK(spawner.hasChild(capturedPid) == false); // no registry entry + + // I4: accept either ESRCH (already fully reaped) or a live-but-exited + // pidfd as proof the guard's Kill()+AwaitResult() already ran. + const int observerPidFd{testPidfdOpen(capturedPid)}; + if (observerPidFd < 0) { + BOOST_CHECK_EQUAL(errno, ESRCH); // already fully reaped - this IS proof of cleanup + } else { + BOOST_CHECK(pidfdReadableWithin(observerPidFd, 3000)); + ::close(observerPidFd); + } +} + +BOOST_AUTO_TEST_CASE(testMonitorLaunchFailureKillsAndReapsCleanly) { + CScopedTmpDirEnv tmpEnv; + const std::string childRoot{makeChildIpcRoot(tmpEnv.dir(), "case2b")}; + + TPid capturedPid{0}; + TSpawner::TPidFdOpenFn pidFdOpen = forcedPidFdOutcome({-1, ENOSYS}, &capturedPid); + TSpawner::TMonitorLaunchFn alwaysFail = [](std::function) { + return false; + }; + + // Registry insert left at the production default - it must succeed so + // this test isolates monitor-launch failure specifically (the other + // half of gate 2's failure coverage, alongside case 2a's registry-insert + // failure). + TSpawner spawner{pidFdOpen, TSpawner::TRegistryInsertFn{}, alwaysFail, + TSpawner::TAwaitResultFn{}}; + TPid childPid{0}; + const bool spawned = spawner.spawn(ML_SANDBOX2_LIFECYCLE_PAYLOAD, + childIpcArgs(childRoot), childPid); + + BOOST_TEST_REQUIRE(spawned == false); + BOOST_CHECK_EQUAL(childPid, 0); + BOOST_TEST_REQUIRE(capturedPid > 0); + BOOST_CHECK(spawner.hasChild(capturedPid) == false); // eraseRegistryEntry() ran + + // I4: accept either ESRCH (already fully reaped) or a live-but-exited + // pidfd as proof eraseRegistryEntry()/the guard's cleanup already ran. + const int observerPidFd{testPidfdOpen(capturedPid)}; + if (observerPidFd < 0) { + BOOST_CHECK_EQUAL(errno, ESRCH); // already fully reaped - this IS proof of cleanup + } else { + BOOST_CHECK(pidfdReadableWithin(observerPidFd, 3000)); + ::close(observerPidFd); + } +} + +// ===================================================================== +// Gate 3: stale generation must not erase/mutate a newer registration. +// ===================================================================== + +BOOST_AUTO_TEST_CASE(testStaleMonitorGenerationCannotEraseNewerRegistration) { + CScopedTmpDirEnv tmpEnv; + const std::string childRoot{makeChildIpcRoot(tmpEnv.dir(), "case3")}; + + TSpawner::SPidRegistry* registry{nullptr}; + std::shared_ptr capturedResult; + std::function monitorBody; // closes over the ORIGINAL (stale) generation. + + TSpawner::TRegistryInsertFn insertFn = + capturingRegistryInsert(®istry, nullptr, nullptr, nullptr); + TSpawner::TMonitorLaunchFn monitorLaunch = captureMonitorBodyWithoutRunning(&monitorBody); + TSpawner::TAwaitResultFn awaitResultFn = capturingAwaitResult(&capturedResult); + + TSpawner spawner{TSpawner::TPidFdOpenFn{}, insertFn, monitorLaunch, awaitResultFn}; + TPid childPid{0}; + BOOST_TEST_REQUIRE(spawner.spawn(ML_SANDBOX2_LIFECYCLE_PAYLOAD, + childIpcArgs(childRoot), childPid)); + BOOST_TEST_REQUIRE(childPid > 0); + BOOST_TEST_REQUIRE(registry != nullptr); + + std::uint64_t originalGeneration{0}; + std::uint64_t newerGeneration{0}; + std::shared_ptr sandboxHandle; + { + std::lock_guard lock(registry->s_Mutex); + const auto it = registry->s_Children.find(childPid); + BOOST_REQUIRE(it != registry->s_Children.end()); // BOOST_REQUIRE: map iterators aren't streamable + originalGeneration = it->second.s_Generation; + sandboxHandle = it->second.s_Sandbox; + // Simulate a second, newer registration reusing the same numeric + // PID racing this call's slow first monitor - exactly what + // defaultRegistryInsert would do for a fresh insert under the same + // key (bump generation, move to E_Monitoring). + newerGeneration = ++registry->s_NextGeneration; + it->second.s_Generation = newerGeneration; + it->second.s_State = TSpawner::EChildLifecycleState::E_Monitoring; + } + BOOST_TEST_REQUIRE(sandboxHandle != nullptr); + BOOST_TEST_REQUIRE(newerGeneration != originalGeneration); + + // End the real sandboxee so the stale monitor body's (real) + // AwaitResult() call returns instead of hanging. + sandboxHandle->Kill(); + + BOOST_TEST_REQUIRE(static_cast(monitorBody)); + monitorBody(); // the STALE monitor, still closed over originalGeneration. + + BOOST_TEST_REQUIRE(capturedResult != nullptr); // reached marker: AwaitResult() did run + + // Negative + cleanup assertion: the stale monitor must not have + // erased or mutated the newer entry. + std::lock_guard lock(registry->s_Mutex); + const auto it = registry->s_Children.find(childPid); + BOOST_REQUIRE(it != registry->s_Children.end()); // BOOST_REQUIRE: map iterators aren't streamable + BOOST_CHECK_EQUAL(it->second.s_Generation, newerGeneration); + BOOST_CHECK(it->second.s_State == TSpawner::EChildLifecycleState::E_Monitoring); + + // Self-review round 2, finding 3: this case uses the REAL pidfd path + // (empty TPidFdOpenFn{}), so the entry above still holds a genuine open + // pidfd. The stale monitor body correctly skipped closing it (generation + // mismatch - that skip is exactly what gate 3 asserts above), but that also + // means nothing else in this case ever closes it: production's + // defaultRegistryInsert only closes a stale entry's pidfd when a NEWER + // spawn() replaces it, which never happens in this fabricated scenario. + // Close it explicitly so SFdBaselineFixture's end-of-case descriptor + // count matches the suite-wide baseline instead of leaking one fd on + // every run. + ::close(it->second.s_PidFd); +} + +// ===================================================================== +// Gate 4: a stale/expired identity must never let terminateChild() +// signal whatever unrelated process now owns a reused numeric PID. +// ===================================================================== + +//! There is no seam to force the OS's PID allocator to reuse a specific +//! number deterministically, so this fabricates the reused-PID scenario +//! directly in the registry (the only way to make it deterministic) and +//! proves terminateChild() acts on the CURRENTLY-registered identity's own +//! pidfd - never a numeric kill(pid) - by making that identity a real, +//! test-owned (never spawner-owned) forked process and observing it +//! actually receive the signal via a normal blocking waitpid(), not a +//! numeric ::kill() call anywhere in this file. +BOOST_AUTO_TEST_CASE(testTerminateChildSignalsOnlyTheCurrentlyRegisteredIdentity) { + CScopedTmpDirEnv tmpEnv; + const std::string childRoot{makeChildIpcRoot(tmpEnv.dir(), "case4")}; + + TSpawner::SPidRegistry* registry{nullptr}; + std::shared_ptr capturedResultA; + std::function monitorBodyA; + + TSpawner::TRegistryInsertFn insertFn = + capturingRegistryInsert(®istry, nullptr, nullptr, nullptr); + TSpawner::TMonitorLaunchFn monitorLaunch = captureMonitorBodyWithoutRunning(&monitorBodyA); + TSpawner::TAwaitResultFn awaitResultFn = capturingAwaitResult(&capturedResultA); + + TSpawner spawner{TSpawner::TPidFdOpenFn{}, insertFn, monitorLaunch, awaitResultFn}; + TPid pidA{0}; + BOOST_TEST_REQUIRE(spawner.spawn(ML_SANDBOX2_LIFECYCLE_PAYLOAD, + childIpcArgs(childRoot), pidA)); + BOOST_TEST_REQUIRE(pidA > 0); + + // Reap A for real - end its life and run its own monitor cleanup - so + // the registry no longer has a live entry for pidA, simulating "the + // original sandboxee already exited and was reaped". + std::shared_ptr sandboxA; + { + std::lock_guard lock(registry->s_Mutex); + const auto it = registry->s_Children.find(pidA); + BOOST_REQUIRE(it != registry->s_Children.end()); // BOOST_REQUIRE: map iterators aren't streamable + sandboxA = it->second.s_Sandbox; + } + sandboxA->Kill(); + BOOST_TEST_REQUIRE(static_cast(monitorBodyA)); + monitorBodyA(); + BOOST_CHECK(registry->s_Children.count(pidA) == 0); + + // Fabricate "an unrelated process B now owns pidA's numeric PID": a + // real, test-owned, throwaway forked process - never spawner-owned, so + // this is test-fixture setup/teardown, not the thing the "no + // numeric ::kill() on a spawner-owned child" constraint is about. + const pid_t pidB{::fork()}; + BOOST_TEST_REQUIRE(pidB >= 0); + if (pidB == 0) { + // Plain test-fixture child: default SIGTERM disposition (terminate) + // is exactly what this test wants to observe. + for (;;) { + ::pause(); + } + } + const int pidFdB{testPidfdOpen(pidB)}; + BOOST_TEST_REQUIRE(pidFdB >= 0); + + { + std::lock_guard lock(registry->s_Mutex); + TSpawner::SSandboxedChild fabricated; + fabricated.s_State = TSpawner::EChildLifecycleState::E_Monitoring; + fabricated.s_Generation = ++registry->s_NextGeneration; + fabricated.s_PidFd = pidFdB; + fabricated.s_PidFdOutcome = TSpawner::EPidFdOutcome::E_Acquired; + fabricated.s_Outcome = std::make_shared(); + registry->s_Children[pidA] = std::move(fabricated); // same numeric key A used to own. + } + + // The call under test, addressed at the numeric PID that used to + // identify A. + BOOST_TEST_REQUIRE(spawner.terminateChild(pidA)); + + // Mechanism + negative assertion: this must have signalled B via B's + // OWN pidfd (captured at B's own registration), never a numeric + // ::kill(pidA, ...) - confirmed by actually observing B die of SIGTERM + // via a normal blocking waitpid() on the test's own direct child, not + // polling. + int status{0}; + BOOST_TEST_REQUIRE(::waitpid(pidB, &status, 0) == pidB); + BOOST_CHECK(WIFSIGNALED(status) != 0); + BOOST_CHECK_EQUAL(WTERMSIG(status), SIGTERM); + + ::close(pidFdB); +} + +// ===================================================================== +// Gate 5: timeout-vs-completion race, both interleavings, plus a +// genuine concurrent stress run - all against CCasOutcomeLatch directly (the +// sole coordination primitive this race is assigned to). No timeout +// caller exists anywhere in the codebase yet (an accepted, documented gap), +// so there is nothing on the spawn()/monitorBody +// integration side to additionally exercise for this gate. +// ===================================================================== + +BOOST_AUTO_TEST_CASE(testCasOutcomeLatchResolvesExactlyOnceBothOrderings) { + using TLatch = TSpawner::CCasOutcomeLatch; + using EState = TSpawner::EOutcomeState; + { + TLatch latch; + EState completed{EState::E_Completed}; + EState timedOut{EState::E_TimedOut}; + const bool completionWon{latch.tryResolve(completed)}; + const bool timeoutWon{latch.tryResolve(timedOut)}; + BOOST_CHECK(completionWon); + BOOST_CHECK(timeoutWon == false); + BOOST_CHECK(timedOut == EState::E_Completed); // loser observes the winner's value + BOOST_CHECK(latch.load() == EState::E_Completed); + } + { + TLatch latch; + EState timedOut{EState::E_TimedOut}; + EState completed{EState::E_Completed}; + const bool timeoutWon{latch.tryResolve(timedOut)}; + const bool completionWon{latch.tryResolve(completed)}; + BOOST_CHECK(timeoutWon); + BOOST_CHECK(completionWon == false); + BOOST_CHECK(completed == EState::E_TimedOut); + BOOST_CHECK(latch.load() == EState::E_TimedOut); + } +} + +BOOST_AUTO_TEST_CASE(testCasOutcomeLatchUnderRealConcurrencyResolvesExactlyOnce) { + using TLatch = TSpawner::CCasOutcomeLatch; + using EState = TSpawner::EOutcomeState; + for (int trial = 0; trial < 200; ++trial) { + TLatch latch; + std::promise startPromise; + std::shared_future start{startPromise.get_future()}; + std::atomic completedWins{0}; + std::atomic timedOutWins{0}; + + auto race = [&](EState desiredInitial, std::atomic& winCounter) { + start.wait(); // test-controlled synchronization point, never sleep(). + EState desired{desiredInitial}; + if (latch.tryResolve(desired)) { + ++winCounter; + } + }; + std::thread t1(race, EState::E_Completed, std::ref(completedWins)); + std::thread t2(race, EState::E_TimedOut, std::ref(timedOutWins)); + startPromise.set_value(); + t1.join(); + t2.join(); + + // Exactly one side ever wins, regardless of scheduling order - the + // property this latch exists to guarantee. + BOOST_CHECK_EQUAL(completedWins.load() + timedOutWins.load(), 1); + } +} + +// ===================================================================== +// Gate 6 (cleanup): descriptor baseline. SFdBaselineFixture (above) +// already asserts this after every case in this suite; this case names it +// explicitly against one concrete spawn/terminate/cleanup cycle. +// ===================================================================== + +BOOST_AUTO_TEST_CASE(testDescriptorCountReturnsToBaselineAfterSpawnTerminateCleanup) { + const std::size_t before{openFdCount()}; + + CScopedTmpDirEnv tmpEnv; + const std::string childRoot{makeChildIpcRoot(tmpEnv.dir(), "case6")}; + + std::shared_ptr capturedResult; + std::function monitorBody; + TSpawner::TPidFdOpenFn pidFdOpen = forcedPidFdOutcome({-1, ENOSYS}); // avoids the SIGTERM-survives hang. + TSpawner::TMonitorLaunchFn monitorLaunch = captureMonitorBodyWithoutRunning(&monitorBody); + TSpawner::TAwaitResultFn awaitResultFn = capturingAwaitResult(&capturedResult); + + TSpawner spawner{pidFdOpen, TSpawner::TRegistryInsertFn{}, monitorLaunch, awaitResultFn}; + TPid childPid{0}; + BOOST_TEST_REQUIRE(spawner.spawn(ML_SANDBOX2_LIFECYCLE_PAYLOAD, + childIpcArgs(childRoot), childPid)); + BOOST_TEST_REQUIRE(childPid > 0); + BOOST_TEST_REQUIRE(spawner.terminateChild(childPid)); + BOOST_TEST_REQUIRE(static_cast(monitorBody)); + monitorBody(); // real cleanup path: closes the pidfd, erases the entry. + + // monitorBody's closure (built by + // spawn()'s defaultMonitorLaunch path) captures `sandbox` - + // shared_ptr - BY VALUE and never releases it during + // execution; invoking the closure does not destroy the closure itself. + // This `monitorBody` local therefore still keeps the Sandbox2 instance + // (and its supervisor-side comms socketpair fd, only closed by + // ~Comms()/~Sandbox2()) alive until it goes out of scope. Release it + // explicitly here, BEFORE the fd-baseline check, so ~Sandbox2() (and the + // comms fd close) has already run when openFdCount() is taken. + monitorBody = nullptr; + + BOOST_CHECK_EQUAL(openFdCount(), before); +} + +// ===================================================================== +// Gate 7: controller-exit orphan behavior - spawner-side half ONLY. +// +// RULING (per the task brief): the orphan-CLEANUP half (does an abandoned +// sandboxee eventually get reaped by something else in the system) is out +// of scope for this unit test and is NOT claimed as covered here. It is an +// accepted, documented gap left for a future follow-up to close. +// ===================================================================== + +BOOST_AUTO_TEST_CASE(testDestructorDoesNotJoinAndReturnsUnderOneSecond) { + CScopedTmpDirEnv tmpEnv; + const std::string childRoot{makeChildIpcRoot(tmpEnv.dir(), "case7")}; + + std::shared_ptr capturedSandbox; + TSpawner::TRegistryInsertFn insertFn = + capturingRegistryInsert(nullptr, nullptr, nullptr, &capturedSandbox); + + // Real monitor launch (a genuine background thread, like the production + // default) AND real AwaitResult (left as the default, empty seam): a + // genuine background thread is blocked in the real AwaitResult() on a + // genuinely live, never-self-exiting child when the spawner below is + // destroyed - this is the monitor-outlives-spawner scenario gate 8 below + // exercises in full, not a simulation of it. + // Unlike the plain default monitor-launch seam, this variant also + // signals monitorDonePromise once that thread's cleanup has fully run, + // which this test needs afterward to deterministically avoid racing + // the suite-wide SFdBaselineFixture's end-of-case descriptor count + // (the real cleanup closes the child's pidfd on that same thread, + // asynchronously with respect to this test case's own control flow). + // Heap-owned (shared_ptr), not a stack local, so a BOOST_TEST_REQUIRE + // throwing before monitorDone.wait() below cannot free this out from + // under the still-running detached thread (finding 4). + auto monitorDonePromise = std::make_shared>(); + std::future monitorDone{monitorDonePromise->get_future()}; + TSpawner::TMonitorLaunchFn monitorLaunch = + realMonitorLaunchWithCompletionSignal(monitorDonePromise); + + auto spawner = std::make_unique(TSpawner::TPidFdOpenFn{}, insertFn, monitorLaunch, + TSpawner::TAwaitResultFn{}); + TPid childPid{0}; + BOOST_TEST_REQUIRE(spawner->spawn(ML_SANDBOX2_LIFECYCLE_PAYLOAD, + childIpcArgs(childRoot), childPid)); + BOOST_TEST_REQUIRE(childPid > 0); + BOOST_CHECK(spawner->hasChild(childPid)); // reached marker: genuinely running + + const auto start = std::chrono::steady_clock::now(); + spawner.reset(); // ~CSandboxedProcessSpawner() with a live child and a real + // monitor thread genuinely blocked in AwaitResult() on it. + const auto elapsed = std::chrono::steady_clock::now() - start; + + BOOST_CHECK(elapsed < std::chrono::seconds(1)); // destructor must not block on the live child + + // Test hygiene, not part of the gate 7 assertion itself: reap the + // still-running sandboxee via its identity-bound Sandbox2 handle + // (never a numeric ::kill()) so this test process doesn't leave a + // permanently-blocked monitor thread behind, then block (no polling) + // until that thread's own cleanup has fully finished, so the next + // test case's fd-baseline snapshot cannot race this one's cleanup. + BOOST_TEST_REQUIRE(capturedSandbox != nullptr); + capturedSandbox->Kill(); + monitorDone.wait(); +} + +// ===================================================================== +// Gate 8: monitor outlives spawner - destroy the spawner while a +// monitor thread is genuinely still running (blocked on a test-controlled +// gate), release the gate, assert its cleanup runs safely against the +// registry it co-owns via shared_ptr. +// ===================================================================== + +BOOST_AUTO_TEST_CASE(testMonitorCleanupRunsSafelyAfterSpawnerDestruction) { + CScopedTmpDirEnv tmpEnv; + const std::string childRoot{makeChildIpcRoot(tmpEnv.dir(), "case8")}; + + std::promise gatePromise; + std::shared_future gate{gatePromise.get_future()}; + // Heap-owned (shared_ptr), not a stack local, matching gate 7 (finding + // 4): a throw between spawn() and monitorDone.wait() below must not free + // this out from under the still-running detached thread. + auto monitorDonePromise = std::make_shared>(); + std::future monitorDone{monitorDonePromise->get_future()}; + + TSpawner::TAwaitResultFn awaitResultFn = + [gate](sandbox2::Sandbox2& sandbox) -> sandbox2::Result { + gate.wait(); // test-controlled synchronization point - never sleep(). + return sandbox.AwaitResult(); + }; + // Self-review round 2, finding 2: also request the seam's own extra + // shared_ptr> reference to the monitor closure + // (monitorBodyKeepAlive), held by this test until after the registryRaw + // dereference below. Previously the ONLY surviving + // shared_ptr once the spawner block below exits was the + // detached thread's own copy inside that closure, and the thread + // destroyed its copy immediately after running the body and BEFORE + // signalling monitorDone - so by the time this test woke up from + // monitorDone.wait() and dereferenced registryRaw, the registry had + // already been freed (a deterministic use-after-free, not merely + // racy). Holding monitorBodyKeepAlive here keeps the same object alive + // regardless of when the detached thread releases its own copy. + std::shared_ptr> monitorBodyKeepAlive; + TSpawner::TMonitorLaunchFn monitorLaunch = + realMonitorLaunchWithCompletionSignal(monitorDonePromise, &monitorBodyKeepAlive); + + std::shared_ptr capturedSandbox; + TSpawner::SPidRegistry* registryRaw{nullptr}; + TSpawner::TRegistryInsertFn insertFn = + capturingRegistryInsert(®istryRaw, nullptr, nullptr, &capturedSandbox); + + TPid childPid{0}; + { + TSpawner spawner{TSpawner::TPidFdOpenFn{}, insertFn, monitorLaunch, awaitResultFn}; + BOOST_TEST_REQUIRE(spawner.spawn(ML_SANDBOX2_LIFECYCLE_PAYLOAD, + childIpcArgs(childRoot), childPid)); + BOOST_TEST_REQUIRE(childPid > 0); + BOOST_CHECK(spawner.hasChild(childPid)); // reached marker + } // spawner destroyed here; the monitor thread is still genuinely blocked on `gate`. + + // registryRaw is safe to dereference below because monitorBodyKeepAlive + // (captured just above) holds its own shared_ptr reference + // via the monitor closure - independent of whatever the detached monitor + // thread's own copy of that same closure does or does not still hold by + // this point. The spawner's own shared_ptr, which is what made this + // pointer valid originally, is gone; this test-owned reference is what + // now keeps the object alive, exercising the same underlying + // co-ownership property the monitor thread relies on in production. + + // Release the gate and make the sandboxee actually exit, so the + // now-unblocked real AwaitResult() call inside the monitor thread can + // return - identity-bound cleanup via the co-owned Sandbox2 handle, + // never a numeric ::kill(). + BOOST_TEST_REQUIRE(capturedSandbox != nullptr); + gatePromise.set_value(); + capturedSandbox->Kill(); + + // Block (no polling) until the monitor thread's entire body - including + // its registry cleanup - has fully returned. + monitorDone.wait(); + + // No-crash assertion: reaching this line at all, after the spawner + // is long gone, is the primary proof. The check below additionally + // confirms the monitor's registry erase actually ran. + BOOST_TEST_REQUIRE(registryRaw != nullptr); + std::lock_guard lock(registryRaw->s_Mutex); + BOOST_CHECK(registryRaw->s_Children.count(childPid) == 0); + // monitorBodyKeepAlive is not explicitly reset: it goes out of scope + // here, after every dereference of registryRaw above, which is all that + // matters for finding 2. Its (and capturedSandbox's) destruction here + // still runs on this thread, strictly before SFdBaselineFixture's + // end-of-case descriptor check, so this does not reintroduce the fd- + // baseline race the original early-destroy trick (I5) guarded against. +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/lib/sandbox/unittest/payloads/lifecycle_signal_payload.cc b/lib/sandbox/unittest/payloads/lifecycle_signal_payload.cc new file mode 100644 index 000000000..2f5ae2535 --- /dev/null +++ b/lib/sandbox/unittest/payloads/lifecycle_signal_payload.cc @@ -0,0 +1,72 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the following additional limitation. Functionality enabled by the + * files subject to the Elastic License 2.0 may only be used in production when + * invoked by an Elasticsearch process with a license key installed that permits + * use of machine learning features. You may not use this file except in + * compliance with the Elastic License 2.0 and the foregoing additional + * limitation. + */ + +// Deliberately dependency-free sandboxee for +// CSandboxedProcessSpawnerLifecycleTest_Linux (Task 4). Unlike +// sandbox_smoke_payload.cc (exits immediately) this payload stays alive +// indefinitely so the lifecycle test can drive CSandboxedProcessSpawner:: +// terminateChild() against a genuinely live child and distinguish its two +// termination mechanisms by observable effect: +// +// - pidfd_send_signal(SIGTERM) (the E_Acquired branch) is a *request*: this +// payload installs a SIGTERM handler that does nothing and returns, so +// the process stays alive and the test can observe "still running". +// - Sandbox2::Kill() (the E_KernelUnsupported branch) hard-codes SIGKILL, +// which cannot be caught or ignored, so the process actually exits. +// +// Only syscalls in seccomp::pytorch_inference::legacyBpfAllowedSyscalls() +// are available under the real spawn() policy - notably __NR_pause is NOT +// in that allowlist, so this cannot simply call pause() in a loop. Blocking +// on FUTEX_WAIT against a private, never-signalled word uses only +// __NR_futex (allowed) and is interrupted (EINTR) by the caught SIGTERM, +// after which the loop just re-enters the wait; the only way to actually +// terminate this process is an uncatchable signal (SIGKILL). +// +// No ml-cpp library dependencies, no policy of its own - same rationale as +// sandbox_smoke_payload.cc and ml_sandbox_probe.cc. + +#include +#include +#include +#include +#include + +namespace { + +std::atomic gFutexWord{0}; + +void ignoreSigterm(int /* signum */) { + // Deliberately empty: catching (rather than ignoring via SIG_IGN) means + // the blocking futex(2) call below observes EINTR and this handler + // itself is proof the process is still alive and processing signals + // normally - SIG_IGN would make that indistinguishable from "never + // received the signal at all". +} + +} // namespace + +int main() { + struct sigaction sa {}; + sa.sa_handler = ignoreSigterm; + ::sigemptyset(&sa.sa_mask); + sa.sa_flags = 0; + ::sigaction(SIGTERM, &sa, nullptr); + + for (;;) { + // FUTEX_WAIT (0): block while *reinterpret_cast(&gFutexWord) == + // 0, which it always is - nothing ever calls FUTEX_WAKE on this + // word. Returns on a spurious wake, a real wake (never happens + // here), or EINTR from the caught SIGTERM; any of those just loops + // back into another wait. + ::syscall(SYS_futex, reinterpret_cast(&gFutexWord), 0, 0, nullptr); + } + return 0; +}