diff --git a/docs/bounds-track-plan.md b/docs/bounds-track-plan.md index 49e6968..c57192c 100644 --- a/docs/bounds-track-plan.md +++ b/docs/bounds-track-plan.md @@ -119,14 +119,22 @@ Two things fall out rather than being special-cased: display service can still claim it exactly as today. No exemption in the kernel, no mention of display anywhere in the rule. - **Restart needs no race.** A dying driver's device returns to the manager, which - re-delegates it on respawn. Today the kernel releases it to nobody and the manager - re-claims first-come, so every restart reopens the hole this run closes. + re-delegates it on respawn. Previously the kernel released it to nobody and the + manager re-claimed first-come, so every restart reopened the hole. + +**Found while implementing: E2 is the step that closes the hole, not E3.** A delegated +device is *held*, so an attempt to take it is refused as `AlreadyClaimed` long before +the giver is consulted — and once a borrower's death returns the device to its lender +(or clears both when the lender is gone), there is no state where a device is unheld and +still on loan. E3's check is therefore unreachable today. It stays as one comparison +that fails closed, guarding any future path that frees a device without clearing its +giver, and its comment says so rather than implying a protection it is not providing. | Step | What | |---|---| | E1 | Record a giver per device; `device_transfer` and the spawn grant set it — **done** | | E2 | On task death a device reverts to its giver if alive, else its claim clears — **done** | -| E3 | `device_claim` refuses a device that has a giver | +| E3 | `device_claim` refuses a device that has a giver — **done**, but unreachable: E2 already closed the window | | E4 | The manager claims every resource-bearing device at boot, so nothing is left takeable | | E5 | The attacker fixture gains the claim half it has been waiting for since D2 | | E6 | Delete the delegated-set scaffolding — every driver is delegated now | diff --git a/library/device/driver/driver.zig b/library/device/driver/driver.zig index 3a9633c..0b2ea4b 100644 --- a/library/device/driver/driver.zig +++ b/library/device/driver/driver.zig @@ -70,7 +70,7 @@ pub fn transfer(id: u64, to: u32) TransferError!void { /// re-enumerate, and `NotConfined` means the machine could not place the device under /// IOMMU translation — the claim was rolled back, and that one is a fault report, not /// a retry. `Refused` is an errno this library does not know a name for. -pub const ClaimError = error{ NoSuchDevice, AlreadyClaimed, NotConfined, Refused }; +pub const ClaimError = error{ NoSuchDevice, AlreadyClaimed, NotConfined, NotYours, Refused }; /// Take exclusive ownership of device `id`. pub fn claim(id: u64) ClaimError!void { @@ -80,6 +80,7 @@ pub fn claim(id: u64) ClaimError!void { abi.ENODEV => error.NoSuchDevice, abi.EBUSY => error.AlreadyClaimed, abi.ECONFINE => error.NotConfined, + abi.EPERM => error.NotYours, // delegated hardware: it must be handed to you else => error.Refused, }; } diff --git a/system/kernel/devices-broker.zig b/system/kernel/devices-broker.zig index 7b16b73..0d6b8a2 100644 --- a/system/kernel/devices-broker.zig +++ b/system/kernel/devices-broker.zig @@ -251,6 +251,22 @@ pub fn enumerateFrom(start: usize, out: []device_abi.DeviceDescriptor) usize { pub fn claim(id: u64, owner: u32) ClaimError!void { if (id >= count) return error.NoSuchDevice; if (claimed[@intCast(id)] != null) return error.AlreadyClaimed; + // **Delegated hardware may be handed on, never taken.** + // + // Belt and braces, and worth being honest about: with the loan rule above this is + // **currently unreachable**. A device that was given to someone is held, so it is + // refused as `AlreadyClaimed` before reaching here; and when the holder dies the + // device goes back to its lender (or, if the lender is gone, has its giver cleared + // with its claim), so there is no state where a device is unheld *and* still on + // loan. The window a stranger could have used simply stops existing. + // + // It stays because it is one comparison and it fails closed: any future path that + // frees a device without clearing its giver would otherwise hand delegated + // hardware to whoever asked first, which is exactly the hole this run closed. + // + // Note it leaves the loader's framebuffer alone without naming it: nobody delegates + // the framebuffer, so it has no giver, so the display service claims it as always. + if (giver[@intCast(id)] != null) return error.NotYours; claimed[@intCast(id)] = owner; } @@ -428,6 +444,7 @@ pub fn errnoOf(e: RegisterError) i64 { pub const ClaimError = error{ NoSuchDevice, // no device with that id AlreadyClaimed, // a live task already owns it + NotYours, // delegated hardware: it has a giver, so it must be handed on, not taken }; /// Why a `transfer` was refused. @@ -474,6 +491,7 @@ pub fn claimErrnoOf(e: ClaimError) i64 { return switch (e) { error.NoSuchDevice => abi.ENODEV, error.AlreadyClaimed => abi.EBUSY, + error.NotYours => abi.EPERM, }; } diff --git a/test/system/services/device-authority-test/device-authority-test.zig b/test/system/services/device-authority-test/device-authority-test.zig index fea745b..8dc10f5 100644 --- a/test/system/services/device-authority-test/device-authority-test.zig +++ b/test/system/services/device-authority-test/device-authority-test.zig @@ -25,12 +25,13 @@ //! is what cost a debugging session on the Ryzen, so the distinction is //! part of the contract and is tested as such. //! -//! **What this fixture cannot yet claim.** `device_claim` is still -//! first-come-first-served at this point in the run — that is the hole D6 -//! closes. So the claim half of the invariant ("a process holds what it was -//! handed and cannot name its way into holding more") is deliberately NOT -//! asserted here; it is added to this fixture at D6, when it becomes true. -//! Asserting it now would mean writing a test that documents the bug. +//! **Why there is no "cannot take a delegated device" assertion here.** The +//! hole this fixture was written for is closed, but not by a refusal it could +//! observe. A device that was given to someone is *held*, so an attempt to +//! take it is refused as `AlreadyClaimed` — the same answer as before. What +//! changed is what happens when the holder dies: the device returns to +//! whoever lent it instead of becoming free, so the window in which a +//! stranger could take it no longer exists. There is no moment to catch. const std = @import("std"); const device = @import("driver");