diff --git a/docs/bounds-track-plan.md b/docs/bounds-track-plan.md index 01cce41..747387c 100644 --- a/docs/bounds-track-plan.md +++ b/docs/bounds-track-plan.md @@ -47,7 +47,11 @@ that cannot safely run in user space.** | D6 | `device_claim` refuses a device the caller was not handed; the hole is closed | not started | | D7 | Zero-resource devices stop being kernel objects — inventory moves to the manager | **blocked** — nothing else mints their ids; see question 8 | | D8 | **`maximum_children_per_parent` deleted** — the authorisation it stood in for exists | **blocked on D6**, and now ordered after D9 | -| D9 | The device table becomes dynamic; **`maximum_devices` deleted**; per-holder quota declared | not started — **runs before D8** | +| D9 | The device table becomes dynamic; **`maximum_devices` deleted**; per-holder quota declared | **done** — one of the two invented numbers is gone | + +**Run 2 stops here.** D1, D2, D4, D5 (`pci-bus` only) and D9 landed; D6, D7, D8 and the +rest of D5 are blocked on questions 6, 7 and 8 below. Suite 118/118, and +`maximum_devices` no longer exists. Ordering is load-bearing. D1–D2 build and prove the mechanism with nothing depending on it. D4–D5 move each claimant across one at a time, so the suite stays green throughout diff --git a/system/kernel/devices-broker.zig b/system/kernel/devices-broker.zig index 6dc7bdc..286e370 100644 --- a/system/kernel/devices-broker.zig +++ b/system/kernel/devices-broker.zig @@ -22,16 +22,36 @@ const std = @import("std"); const abi = @import("abi"); const platform = @import("platform"); const device_abi = @import("device-abi"); +const heap = @import("heap.zig"); -/// bound: device nodes for the whole machine — firmware-discovered plus every child a -/// bus driver registers at runtime -/// decided-by: hardware -/// protects: nothing — this is a sizing guess about someone else's computer, which is -/// why an AMD Ryzen booted with a working display, no USB and no storage -/// at-limit: refuse — ENOSPC from device_register; `dropped` counts discovery losses +/// How many devices a single registrar may put in the table. +/// +/// **This is a runaway detector, not a security boundary**, and the difference matters. +/// It cannot stop a malicious driver — a quota generous enough never to bite a real +/// machine is still generous enough to be unpleasant — and it is not trying to. What +/// stops malice is that only a driver the manager handed a device can register children +/// under it (docs/os-development/device-authority.md). What this catches is a +/// *legitimate* driver in a loop, early, and attributably: the driver that did it is +/// refused, named in its own log, and restarted, while every other driver is untouched. +/// +/// The shared ceilings it replaces could not do that. `maximum_devices = 64` was a +/// guess about someone else's computer and one driver's enumeration starved every +/// other — which is how an AMD Ryzen came to boot with a working display, no USB and no +/// storage. A per-registrar allowance is the same protection charged to whoever caused +/// it, which is the microkernel property rather than a workaround for it. +/// +/// The number: a machine's whole PCI segment tops out at 65536 functions, and the +/// biggest real registrar seen is pci-bus at a few dozen. 4096 is far above anything a +/// real machine produces and around 1.4 MiB of descriptors, well under the kernel heap. +/// **Reaching it is a bug report, not a tuning request** — no correct driver gets near. +/// +/// bound: devices one task may register +/// decided-by: ours +/// protects: the kernel heap, against a driver looping device_register +/// at-limit: refuse — ECHILDREN to the registrar; every other driver is unaffected /// observed-by: the bus driver's line naming the reason (pci-bus reconciles functions -/// found against registered), and kernel.zig:203 for discovery drops -pub const maximum_devices = 64; +/// found against registered) +const maximum_devices_per_registrar = 4096; /// Cap on children a single parent may have. A zero-resource child (legal — a USB /// device is addressed through its controller, not by MMIO) sidesteps the containment @@ -50,18 +70,64 @@ pub const maximum_devices = 64; /// observed-by: pci-bus logs the reason per refused function, and warns at end of scan const maximum_children_per_parent = 16; -var devices: [maximum_devices]device_abi.DeviceDescriptor = undefined; -var claimed: [maximum_devices]?u32 = .{null} ** maximum_devices; // owner task id, or null +/// The table, grown on demand from the kernel heap. **There is no ceiling**: how many +/// devices a machine has is the machine's business, and no specification bounds it, so +/// nothing here should. `devices_broker.init` runs after `heap.init` (kernel.zig), so +/// there was never a reason for this to be static beyond it having been written that +/// way first. +var devices: []device_abi.DeviceDescriptor = &.{}; +/// Owner task id per device, or null. Parallel to `devices` and grown with it. +var claimed: []?u32 = &.{}; +/// The task that called `register` for each device, so the per-registrar allowance can +/// be charged to whoever caused the entry. Firmware-discovered nodes carry `no_registrar` +/// — they are the kernel's own, not anybody's doing. +var registrar: []u32 = &.{}; +const no_registrar: u32 = 0; var count: usize = 0; +/// Grow the three parallel arrays so at least one more device fits. False if the heap +/// cannot satisfy it, which the callers report rather than swallow. +fn reserve() bool { + if (count < devices.len) return true; + // Double from a deliberately SMALL first block. Sizing it for a typical machine + // would mean the growth path never ran on the hardware we test on, and only woke up + // on someone else's larger machine — which is the exact shape of the failure this + // whole track exists to stop. At 8, every boot grows the table several times, so + // the path is exercised constantly and the suite asserts it. + const wanted = if (devices.len == 0) 8 else devices.len * 2; + const allocator = heap.allocator(); + const grown_devices = allocator.realloc(devices, wanted) catch return false; + devices = grown_devices; + const grown_claimed = allocator.realloc(claimed, wanted) catch return false; + claimed = grown_claimed; + const grown_registrar = allocator.realloc(registrar, wanted) catch return false; + registrar = grown_registrar; + for (claimed[count..], registrar[count..]) |*slot, *who| { + slot.* = null; + who.* = no_registrar; + } + return true; +} + +/// How many devices `task` has registered — the allowance is charged per registrar, so +/// a driver in a loop exhausts its own and no one else's. +fn registeredBy(task: u32) usize { + var n: usize = 0; + for (registrar[0..count]) |who| { + if (who == task) n += 1; + } + return n; +} + /// The id of the seeded framebuffer node (`seedDisplay`), or null when the machine /// handed over no framebuffer. Lets the process layer recognise the display claim /// (to quiesce the bootstrap console) without threading the id through every caller. var display_device: ?u64 = null; -/// Devices discovery found but the table had no room for. Non-zero means the machine -/// is bigger than `maximum_devices` and some hardware is simply invisible to drivers — -/// which would otherwise be an entirely silent failure. Logged at boot. +/// Devices discovery found but could not record. The table grows on demand, so this is +/// no longer "the machine is bigger than our guess" — it means the kernel heap could not +/// satisfy the growth, which would otherwise be an entirely silent failure. Logged at +/// boot. pub var dropped: usize = 0; /// Snapshot the device tree into the flat table. Run once, right after discovery. @@ -69,7 +135,8 @@ pub fn init(device_tree: *const platform.DeviceTree) void { count = 0; dropped = 0; display_device = null; - for (&claimed) |*c| c.* = null; + for (claimed) |*c| c.* = null; + for (registrar) |*r| r.* = no_registrar; walk(device_tree.root, device_abi.no_parent); } @@ -81,7 +148,7 @@ pub fn init(device_tree: *const platform.DeviceTree) void { /// table is full. Idempotent-ish: only ever call once per boot. pub fn seedDisplay(base: u64, width: u32, height: u32, pitch: u32, format: u32, refresh_hz: u32) ?u64 { if (base == 0 or width == 0 or height == 0) return null; // headless - if (count >= maximum_devices) { + if (!reserve()) { dropped += 1; return null; } @@ -126,7 +193,7 @@ fn walk(node: *platform.Device, parent_id: u64) void { } fn record(node: *platform.Device, parent_id: u64) u64 { - if (count >= maximum_devices) { + if (!reserve()) { dropped += 1; return device_abi.no_parent; // children of a dropped node become roots, not orphans } @@ -431,7 +498,11 @@ pub fn register(parent_id: u64, owner: u32, descriptor: *const device_abi.Device if (existingChild(parent_id, descriptor)) |existing_id| return existing_id; if (childCount(parent_id) >= maximum_children_per_parent) return error.TooManyChildren; - if (count >= maximum_devices) return error.NoSpace; + // The allowance is charged to whoever is registering, so a driver in a loop + // exhausts its own and every other driver carries on. There is no machine-wide + // ceiling any more: the table grows. + if (registeredBy(owner) >= maximum_devices_per_registrar) return error.TooManyChildren; + if (!reserve()) return error.NoSpace; const parent = &devices[@intCast(parent_id)]; for (0..@intCast(descriptor.resource_count)) |i| { @@ -454,6 +525,7 @@ pub fn register(parent_id: u64, owner: u32, descriptor: *const device_abi.Device for (0..@intCast(descriptor.resource_count)) |i| d.resources[i] = descriptor.resources[i]; devices[count] = d; + registrar[count] = owner; count += 1; return d.id; } diff --git a/system/kernel/iommu.zig b/system/kernel/iommu.zig index 7166fc4..5afb203 100644 --- a/system/kernel/iommu.zig +++ b/system/kernel/iommu.zig @@ -34,22 +34,24 @@ const platform = @import("platform"); const architecture = @import("architecture"); const devices_broker = @import("devices-broker.zig"); const log = @import("log.zig"); +const heap = @import("heap.zig"); const page_size: u64 = abi.page_size; const page_mask: u64 = page_size - 1; const huge_page_size: u64 = 2 * 1024 * 1024; -/// One domain per claimed PCI function. Coupled to devices-broker's device cap — and -/// coupled *in code*, by the comptime assert beside `confined` below, because when -/// these two agreed only by this sentence the disagreement failed open. +/// The IOMMU's own translation-domain pool — one per claimed DMA-capable device. /// -/// Both VT-d and AMD-Vi report the number of domains they support in a capability -/// register. We should be reading it rather than choosing 64 — docs/bounds-track-plan.md -/// phase 4. +/// No longer coupled to the device count. It was, by a comment and then by a comptime +/// assert, only because `confined` (one slot per device id) was sized by this same +/// constant; those are two unrelated quantities and making the device table dynamic +/// separated them. This one is genuinely the hardware's: both VT-d and AMD-Vi report +/// how many domains they support in a capability register, so the honest fix is to read +/// it rather than choose 64 — bounds-track-plan.md phase 4. /// -/// bound: IOMMU translation domains, one per claimed DMA-capable device +/// bound: IOMMU translation domains the kernel can hold at once /// decided-by: hardware -/// protects: the statically sized domain and confinement tables +/// protects: the statically sized domain pool /// at-limit: refuse — ECONFINE; the claim is rolled back and the device is not driven, /// because a claim that cannot be confined must not stand /// observed-by: the claiming driver's own line naming ECONFINE @@ -107,18 +109,30 @@ pub fn init() void { /// Per-claimed-device record: its private domain, so a driver's death tears down /// exactly the domains it held. const Confined = struct { active: bool = false, owner: u32 = 0, bdf: u16 = 0, domain: u16 = invalid_domain }; -var confined: [maximum_domains]Confined = .{Confined{}} ** maximum_domains; -// `confined` is indexed by **device id**, so it must cover every id the broker can -// mint. These two numbers agreed only by a sentence in a comment above -// `maximum_domains` — and when they disagreed, `confineDevice` returned success for -// the ids it had no room for, leaving those devices unconfined DMA masters. Coupled -// bounds agree in code, not in prose (docs/os-development/bounds.md). -comptime { - if (maximum_domains < devices_broker.maximum_devices) - @compileError("iommu.confined is indexed by device id but is smaller than the " ++ - "broker's device table: ids past its end cannot be confined, and so cannot " ++ - "be claimed at all"); +/// Indexed by **device id**, so it must cover every id the broker can mint — and the +/// broker's table has no ceiling any more, so neither can this. It grows on demand. +/// +/// This used to be `[maximum_domains]`, sized by the *domain* constant purely because +/// device ids happened to stop at 64 as well. Two unrelated quantities sharing one +/// number: `domains` below is the IOMMU's own translation-domain pool, which the +/// hardware bounds and reports, while this is one slot per device the machine has. +/// A comptime assert held them together while both were fixed; making the device table +/// dynamic is what forced them apart, which is the assert having done its job. +var confined: []Confined = &.{}; + +/// Grow `confined` to cover `device_id`. False if the heap cannot — and the caller +/// treats that as a refusal to confine, never as permission. +fn reserveConfined(device_id: u64) bool { + if (device_id < confined.len) return true; + if (device_id >= std.math.maxInt(usize) / 2) return false; // absurd id; refuse rather than size to it + var wanted: usize = if (confined.len == 0) 64 else confined.len; + while (wanted <= device_id) wanted *= 2; + const grown = heap.allocator().realloc(confined, wanted) catch return false; + const previous = confined.len; + confined = grown; + for (confined[previous..]) |*record| record.* = .{}; + return true; } /// Place a just-claimed PCI function under IOMMU translation on behalf of `owner`: give @@ -139,7 +153,7 @@ pub fn confineDevice(device_id: u64, bdf: u16, owner: u32) bool { // at `confined.len`; moving the inventory out of the kernel and taking the domain // count from the hardware both change that, and either would have made a silent // unconfined DMA master out of every device past the 64th. - if (device_id >= confined.len) return false; + if (!reserveConfined(device_id)) return false; const domain = domainCreate(owner, bdf) orelse return false; // Firmware reserved region for this device, if any (real hardware; QEMU has none). @@ -181,7 +195,7 @@ pub fn unmapForDevice(device_id: u64, physical: u64, len: u64) void { /// own freshly-`dma_alloc`'d buffer into the devices it drives. pub fn mapRegionForOwner(owner: u32, physical: u64, len: u64) void { if (!active) return; - for (&confined) |*c| { + for (confined) |*c| { if (c.active and c.owner == owner) _ = map(c.domain, physical, len); } } @@ -192,7 +206,7 @@ pub fn mapRegionForOwner(owner: u32, physical: u64, len: u64) void { /// domain other than its owner's. pub fn unmapRegionEverywhere(physical: u64, len: u64) void { if (!active) return; - for (&confined) |*c| { + for (confined) |*c| { if (c.active) unmap(c.domain, physical, len); } } @@ -222,7 +236,7 @@ pub fn reassign(device_id: u64, owner: u32) void { pub fn releaseAllOwnedBy(owner: u32) void { if (!active) return; - for (&confined) |*c| { + for (confined) |*c| { if (c.active and c.owner == owner) { detachDevice(c.bdf); domainDestroy(c.domain); diff --git a/system/kernel/tests.zig b/system/kernel/tests.zig index d1849f0..5b16627 100644 --- a/system/kernel/tests.zig +++ b/system/kernel/tests.zig @@ -4020,6 +4020,17 @@ fn containmentTest() void { const still_capped = if (devices_broker.register(parent_id, me, &novel)) |_| false else |err| err == error.TooManyChildren; check("a full parent still refuses a new child", still_capped); + // The table itself has no ceiling: it grows. The old `maximum_devices = 64` was a + // guess about someone else's computer, and one driver's enumeration starved every + // other — which is how a Ryzen booted with no USB and no storage. What bounds a + // runaway now is an allowance charged to the registrar, so the damage stays with + // whoever caused it (docs/os-development/device-authority.md). + // The table grows: it starts at 8 entries and this boot holds well past that, so + // the growth path runs every time rather than lying dormant until someone else's + // larger machine finds it — which is how the old ceiling stayed invisible. + const held = devices_broker.enumerate(&buffer); + check("the table grew beyond its initial block", held > 8); + result(); }