diff --git a/PATCHES.md b/PATCHES.md index 7c00947..d00d757 100644 --- a/PATCHES.md +++ b/PATCHES.md @@ -98,23 +98,28 @@ rebuild whenever a guest patch changes. Built locally, not in CI: the host frame checked continuation, keeping the loop responsive. Safe because the body has no `await` and `ManagedProcess` is `Sendable`. Marked `[Nucleic vendored patch]`. -### PLANNED guest patch (design recorded; NOT yet implemented) - -9. **Per-exec cgroups (memory/cpu/pids isolation).** Today the whole container shares ONE cgroup - (`/container/`): `vmexec run` places the init there via the OCI `cgroupsPath` + `applyResources` - (`RunCommand.swift`), and each exec joins it via `loadFromPid(init.pid).addProcess` in - `ManagedProcess.start`. So one session's runaway RSS trips the VM OOM-killer against a *random* - sibling. Target layout (cgroup v2): make `/container/` an intermediary (enable - `cgroup.subtree_control` — `Cgroup2Manager.toggleSubtreeControllers` already skips the leaf so this - composes), move init to a leaf `/container//init`, and place each exec in its own leaf - `/container//` with generous `memory.high`/`memory.max`/`cpu.max`/`pids.max` so a - runaway session is throttled/OOM-killed *within its own cgroup*, siblings untouched — WITHOUT - hard-partitioning RAM (soft limits preserve burst). This is CROSS-CUTTING, not a one-file patch: - the per-exec limits must be carried on the exec RPC (the `CreateProcess`/exec OCI spec has no - resources field today), which means a protobuf field (`SandboxContext`) + host-side plumbing - (`Vminitd.createProcess` / `ContainerEngine.exec`) in addition to the vminitd cgroup restructure - (`ManagedContainer`, `ManagedProcess`, `vmexec/RunCommand`). Sequence it after #8 lands via CI, and - validate in a real container (a wrong v2 hierarchy fails at runtime, not at compile). +9. **Per-exec cgroups (OOM/CPU/pids isolation).** Upstream puts the container init AND every exec in + ONE cgroup (`/container/`), so one session's runaway RSS trips the in-VM OOM-killer against a + *random* sibling, and a fork bomb / CPU hog hits the whole box. This patch makes `/container/` + an intermediary: the resource ceiling stays on it, `ManagedContainer.init` moves the init into its + own leaf (`/container//init`) which enables `cgroup.subtree_control` up the chain, and + `ManagedProcess.start` places each exec in its OWN child (`/container//`) with + `memory.oom.group=1` (a runaway session's OOM kills only *its* tree), a fair `cpu.weight`, and a + `pids.max` fork-bomb backstop. New `Cgroup2Manager` helpers: `setOomGroup`/`setCpuWeight`/ + `setPidsMax`/`remove`. **Best-effort with a graceful fallback**: if any step of the per-exec setup + fails it wipes the partial state and reverts to the flat layout, and `ManagedProcess` falls back to + the container cgroup per exec — so a cgroup hiccup degrades to today's behavior, never a failed + start. `ManagedContainer.execCgroupParent == nil` marks flat mode. NOTE: this delivers *scoped-OOM* + containment without host-configured limits; a hard per-exec `memory.max` (host-chosen, so a session + can't consume the whole box before its own OOM) still wants the exec-RPC resources field + (protobuf + `Vminitd.createProcess`/`ContainerEngine.exec` plumbing) — a follow-up. Marked + `[Nucleic vendored patch]` across `Cgroup2Manager.swift`, `ManagedContainer.swift`, + `ManagedProcess.swift`. **COMPILE-VERIFIED ONLY (musl cross-build); NOT yet runtime-validated** — a + wrong cgroup-v2 hierarchy fails at runtime, so boot a container with the new image and confirm + sessions start, `/sys/fs/cgroup/container//` exists per session, and a hog is contained, + before pointing a shipping build at it. `vmexec/RunCommand` is unchanged: it still applies + `linux.resources` at `linux.cgroupsPath`, which the patch repoints (init leaf) and clears + accordingly. ## Re-vendoring a newer upstream commit @@ -129,8 +134,9 @@ rebuild whenever a guest patch changes. Built locally, not in CI: the host frame `nucleicDrainNonBlocking` + the rewritten `readabilityHandler` blocks), and patch #6 (the atomic stdio-or-abort guard in `start()`), patch #7 (the bounded `deleteProcess` timeout in `Vminitd.swift`), and patch #8 (the `ManagedProcess.start` event-loop offload in `vminitd/`). Grep - for `[Nucleic vendored patch]` to find every site. Patch #9 (per-exec cgroups) is design-only so - far — see its entry. After re-applying any `vminitd/` patch, rebuild + publish the custom init image + for `[Nucleic vendored patch]` to find every site, and patch #9 (per-exec cgroups) across + `Cgroup2Manager.swift` / `ManagedContainer.swift` / `ManagedProcess.swift`. After re-applying any + `vminitd/` patch, rebuild + publish the custom init image with `make vminit-image` + `make vminit-image-push`, and bump `ContainerEngine.vminitReference`. 5. Update the commit hash above and in the root `Package.swift` comment. 6. `swift build` and run the balloon tests. diff --git a/vminitd/Sources/Cgroup/Cgroup2Manager.swift b/vminitd/Sources/Cgroup/Cgroup2Manager.swift index 62fa78c..02151f6 100644 --- a/vminitd/Sources/Cgroup/Cgroup2Manager.swift +++ b/vminitd/Sources/Cgroup/Cgroup2Manager.swift @@ -267,6 +267,33 @@ public struct Cgroup2Manager: Sendable { fileName: "memory.low") } + /// [Nucleic vendored patch] Make the kernel OOM-killer treat this cgroup as an atomic unit: when a + /// memory limit (this cgroup's or an ancestor's) forces an OOM, the whole cgroup's process tree is + /// killed together rather than one victim. Used to scope a runaway exec's OOM to that exec so + /// sibling execs in the same container survive. + package func setOomGroup(_ enabled: Bool) throws { + try Self.writeValue(path: self.path, value: enabled ? "1" : "0", fileName: "memory.oom.group") + } + + /// [Nucleic vendored patch] Relative CPU share under contention (cgroup v2 `cpu.weight`, 1…10000, + /// default 100). Equal weights give each exec a fair slice so one busy session can't starve + /// siblings of CPU. + package func setCpuWeight(_ weight: UInt64) throws { + try Self.writeValue(path: self.path, value: String(weight), fileName: "cpu.weight") + } + + /// [Nucleic vendored patch] Cap the pids in this cgroup (`pids.max`) — a fork-bomb backstop so one + /// exec can't exhaust the pid space and wedge its siblings. + package func setPidsMax(_ max: UInt64) throws { + try Self.writeValue(path: self.path, value: String(max), fileName: "pids.max") + } + + /// [Nucleic vendored patch] Remove this cgroup directory (rmdir). The cgroup must already be empty + /// of processes and child cgroups. Best-effort partial-setup cleanup for the per-exec layout. + package func remove() throws { + try FileManager.default.removeItem(at: self.path) + } + package func getMemoryEvents() throws -> MemoryEvents { let content = try readFileContent(fileName: "memory.events") let values = parseKeyValuePairs(content) diff --git a/vminitd/Sources/VminitdCore/ManagedContainer.swift b/vminitd/Sources/VminitdCore/ManagedContainer.swift index 545046a..4d02c0f 100644 --- a/vminitd/Sources/VminitdCore/ManagedContainer.swift +++ b/vminitd/Sources/VminitdCore/ManagedContainer.swift @@ -32,6 +32,10 @@ public actor ManagedContainer { private let bundle: ContainerizationOCI.Bundle private let needsCgroupCleanup: Bool private var execs: [String: any ContainerProcess] = [:] + // [Nucleic vendored patch] When per-exec cgroup isolation is active, the container cgroup that each + // exec gets its own child under (`/`). nil = the legacy flat layout (init + all + // execs share the container cgroup). + private let execCgroupParent: String? public var pid: Int32? { self.initProcess.pid @@ -44,11 +48,53 @@ public actor ManagedContainer { ociRuntimePath: String? = nil, log: Logger ) async throws { - var cgroupsPath: String - if let cgPath = spec.linux?.cgroupsPath { - cgroupsPath = cgPath + // [Nucleic vendored patch] `spec` is mutated below to relocate the init into its own leaf cgroup + // when per-exec isolation is set up. + var spec = spec + let containerCgroup: String = { + if let p = spec.linux?.cgroupsPath, !p.isEmpty { return p } + return "/container/\(id)" + }() + + let cgManager = Cgroup2Manager( + group: URL(filePath: containerCgroup), + logger: log + ) + try cgManager.create() + + // [Nucleic vendored patch] Per-exec cgroup isolation. Turn the container cgroup into an + // intermediary — resource ceiling on it, controllers delegated to children — and run the + // container init in its own leaf (`/init`), so each exec later gets its OWN child + // cgroup (see `ManagedProcess.start`): a runaway session's OOM/CPU/fork-bomb is then scoped to + // that session and can't take down its siblings in the shared container. Best-effort: on ANY + // failure, wipe the partial state and fall back to the upstream flat layout (init + all execs + // share the container cgroup). `execCgroupParent == nil` marks flat mode. + var execParent: String? = nil + if spec.linux != nil { + do { + let initCg = Cgroup2Manager(group: URL(filePath: containerCgroup + "/init"), logger: log) + try initCg.create() + // Enabling controllers from the init leaf sets cgroup.subtree_control on every ancestor + // (incl. the container cgroup), which is what lets sibling child cgroups get memory/cpu/pids. + try initCg.toggleAllAvailableControllers(enable: true) + if let resources = spec.linux?.resources { + try cgManager.applyResources(resources: resources) // ceiling stays on the parent + } + spec.linux?.cgroupsPath = containerCgroup + "/init" // vmexec places the init here + spec.linux?.resources = nil // don't re-apply the ceiling to the init leaf + execParent = containerCgroup + } catch { + log.error("per-exec cgroup setup failed; using flat layout: \(error)") + // Undo any partial per-exec state so the container cgroup can hold the init again. + let initCg = Cgroup2Manager(group: URL(filePath: containerCgroup + "/init"), logger: log) + try? initCg.toggleAllAvailableControllers(enable: false) + try? initCg.remove() + try cgManager.toggleAllAvailableControllers(enable: true) + spec.linux?.cgroupsPath = containerCgroup + execParent = nil + } } else { - cgroupsPath = "/container/\(id)" + try cgManager.toggleAllAvailableControllers(enable: true) } let bundle = try ContainerizationOCI.Bundle.create( @@ -57,15 +103,7 @@ public actor ManagedContainer { ) log.debug("created bundle with spec \(spec)") - let cgManager = Cgroup2Manager( - group: URL(filePath: cgroupsPath), - logger: log - ) - try cgManager.create() - do { - try cgManager.toggleAllAvailableControllers(enable: true) - let initProcess: any ContainerProcess if let runtimePath = ociRuntimePath { @@ -99,6 +137,7 @@ public actor ManagedContainer { } self.cgroupManager = cgManager + self.execCgroupParent = execParent self.initProcess = initProcess self.id = id self.bundle = bundle @@ -177,6 +216,7 @@ extension ManagedContainer { stdio: stdio, bundle: self.bundle, owningPid: self.initProcess.pid, + execCgroupParent: self.execCgroupParent, // [Nucleic vendored patch] per-exec cgroup log: self.log ) self.execs[id] = process diff --git a/vminitd/Sources/VminitdCore/ManagedProcess.swift b/vminitd/Sources/VminitdCore/ManagedProcess.swift index af6dc2e..e90560d 100644 --- a/vminitd/Sources/VminitdCore/ManagedProcess.swift +++ b/vminitd/Sources/VminitdCore/ManagedProcess.swift @@ -57,6 +57,9 @@ final class ManagedProcess: ContainerProcess, Sendable { private let command: Command private let state: Mutex private let owningPid: Int32? + // [Nucleic vendored patch] Parent cgroup for this exec's OWN per-exec child (`/`); + // nil means the legacy flat layout (join the container/init cgroup via `owningPid`). + private let execCgroupParent: String? private let ackPipe: Pipe private let syncPipe: Pipe private let errorPipe: Pipe @@ -74,6 +77,7 @@ final class ManagedProcess: ContainerProcess, Sendable { stdio: HostStdio, bundle: ContainerizationOCI.Bundle, owningPid: Int32? = nil, + execCgroupParent: String? = nil, // [Nucleic vendored patch] log: Logger ) throws { self.id = id @@ -81,6 +85,7 @@ final class ManagedProcess: ContainerProcess, Sendable { log[metadataKey: "id"] = "\(id)" self.log = log self.owningPid = owningPid + self.execCgroupParent = execCgroupParent let syncPipe = Pipe() try syncPipe.setCloexec() @@ -215,7 +220,27 @@ extension ManagedProcess { // This should probably happen in vmexec, but we don't need to set any cgroup // toggles so the problem is much simpler to just do it here. - if let owningPid { + if let parent = execCgroupParent { + // [Nucleic vendored patch] Per-exec cgroup: place this exec in its OWN child cgroup + // (`/`) with memory.oom.group (a runaway session's OOM kills only its + // tree), a fair cpu.weight, and a pids.max fork-bomb backstop — so one session can't + // OOM-kill / starve / fork-bomb its siblings in the shared container. Best-effort: + // on ANY error fall back to the container/init cgroup so a cgroup hiccup never blocks + // the exec from starting. + do { + let execCg = Cgroup2Manager(group: URL(filePath: parent + "/" + id), logger: log) + try execCg.create() + try? execCg.setOomGroup(true) + try? execCg.setCpuWeight(100) + try? execCg.setPidsMax(4096) + try execCg.addProcess(pid: pid) + } catch { + log.error("per-exec cgroup for \(id) failed; joining the container cgroup: \(error)") + if let owningPid { + try? Cgroup2Manager.loadFromPid(pid: owningPid).addProcess(pid: pid) + } + } + } else if let owningPid { let cgManager = try Cgroup2Manager.loadFromPid(pid: owningPid) try cgManager.addProcess(pid: pid) }