diff --git a/docs/bounds-track-plan.md b/docs/bounds-track-plan.md index 43a7df5..b8286b9 100644 --- a/docs/bounds-track-plan.md +++ b/docs/bounds-track-plan.md @@ -44,7 +44,7 @@ that cannot safely run in user space.** | D3 | The manager claims the seeded devices at boot, before any driver is spawned | **merged into D4** — see below | | D4 | The manager claims + delegates on `hello`; `usb-xhci-bus` is the first driver converted | **done** — caught an IOMMU regression I introduced; see below | | D5 | The other four claimants converted: `pci-bus`, `ps2-bus`, `virtio-gpu`, `acpi` | **partial** — `pci-bus` done; the rest unblocked by D0 below | -| D0 | The grant rides `system_spawn` — atomic, so no driver need change to receive one | not started — **do first** | +| D0 | The grant rides `system_spawn` — atomic, so no driver need change to receive one | **done** — and caught a test-marker bug that made the D2 fixture unfailable | | D10 | Every driver hellos, on its own merits (liveness, one class of driver) | not started — optional, independent | | 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 | diff --git a/library/kernel/process.zig b/library/kernel/process.zig index c341189..c3c5129 100644 --- a/library/kernel/process.zig +++ b/library/kernel/process.zig @@ -182,6 +182,22 @@ pub fn spawnWithArguments(name: []const u8, arguments: []const []const u8) ?u32 /// `ipc.replyWait` as a child-exit badge (`ipc.Received.isChildExit`/`childProcessId`), so /// one endpoint can supervise many children. Returns the child's process id, or null. pub fn spawnSupervised(name: []const u8, arguments: []const []const u8, exit_endpoint: ?usize) ?u32 { + return spawnSupervisedWithDevice(name, arguments, exit_endpoint, abi.no_device); +} + +/// Spawn a supervised child **and give it a device you hold**, in one call. +/// +/// The device manager's path: it holds the hardware and the driver it starts must have +/// it. Fusing the handover into the spawn is what removes the window a separate +/// transfer would leave — the child cannot run without its device, because it does not +/// exist until it holds it (docs/os-development/device-authority.md). The kernel checks +/// only that the device is the caller's to give. +pub fn spawnSupervisedWithDevice( + name: []const u8, + arguments: []const []const u8, + exit_endpoint: ?usize, + device: u64, +) ?u32 { var blob: [256]u8 = undefined; var len: usize = 0; for (arguments, 0..) |argument, i| { @@ -194,7 +210,7 @@ pub fn spawnSupervised(name: []const u8, arguments: []const []const u8, exit_end @memcpy(blob[len..][0..argument.len], argument); len += argument.len; } - const r = sc.systemCall5(.system_spawn, @intFromPtr(name.ptr), name.len, if (len == 0) 0 else @intFromPtr(&blob), len, exit_endpoint orelse abi.no_cap); + const r = sc.systemCall6(.system_spawn, @intFromPtr(name.ptr), name.len, if (len == 0) 0 else @intFromPtr(&blob), len, exit_endpoint orelse abi.no_cap, device); if (r > ~@as(usize, 0) - 4095) return null; // a wrapped -errno return @intCast(r); } diff --git a/library/kernel/system-call.zig b/library/kernel/system-call.zig index 5406a90..244a575 100644 --- a/library/kernel/system-call.zig +++ b/library/kernel/system-call.zig @@ -65,3 +65,19 @@ pub inline fn systemCall5(n: SystemCall, a0: usize, a1: usize, a2: usize, a3: us [a4] "{r8}" (a4), : .{ .rcx = true, .r11 = true, .memory = true }); } + +/// The sixth and last argument register. `syscall` clobbers rcx, so r10 stands in for +/// it and r9 is the end of the line — a call needing a seventh would have to pass a +/// struct instead. +pub inline fn systemCall6(n: SystemCall, a0: usize, a1: usize, a2: usize, a3: usize, a4: usize, a5: usize) usize { + return asm volatile ("syscall" + : [ret] "={rax}" (-> usize), + : [n] "{rax}" (@intFromEnum(n)), + [a0] "{rdi}" (a0), + [a1] "{rsi}" (a1), + [a2] "{rdx}" (a2), + [a3] "{r10}" (a3), + [a4] "{r8}" (a4), + [a5] "{r9}" (a5), + : .{ .rcx = true, .r11 = true, .memory = true }); +} diff --git a/system/abi.zig b/system/abi.zig index 138fe2d..8b18754 100644 --- a/system/abi.zig +++ b/system/abi.zig @@ -48,7 +48,7 @@ pub const SystemCall = enum(u64) { irq_bind = 14, // irq_bind(id, resource_index, endpoint): deliver a device IRQ as an IPC notification irq_ack = 15, // irq_ack(id, resource_index): re-arm a bound IRQ after servicing it device_register = 16, // device_register(parent_id, descriptor) -> id/-errno: publish a child of a device you claimed (-ENOSPC table full, -ECHILDREN parent full, -ERANGE resource escapes the parent, -ENODEV/-EPERM bad parent, -E2BIG too many resources) - system_spawn = 17, // system_spawn(name_ptr, name_len, arguments_ptr, arguments_len, exit_endpoint) -> child process id: start a named initial-ramdisk binary as a new ring-3 process + system_spawn = 17, // system_spawn(name_ptr, name_len, arguments_ptr, arguments_len, exit_endpoint, device) -> child process id: start a named initial-ramdisk binary as a new ring-3 process. `device` (or `no_device`) is a device the caller holds and gives to the child, atomically — the child never runs without it dma_alloc = 18, // dma_alloc(len, flags) -> virtual_address (rax), physical_address (rdx): contiguous, pinned, uncacheable DMA memory dma_free = 19, // dma_free(virtual_address, len) -> 0: release a prior dma_alloc msi_bind = 20, // msi_bind(device_id, endpoint) -> address (rax), data (rdx): a per-device MSI vector for a claimed device @@ -127,6 +127,11 @@ pub const ECONFINE: i64 = 16; // the device could not be placed under IOMMU tran /// space, and the number to bump when adding one. pub const errno_maximum: i64 = 16; +/// `system_spawn`'s `device` argument when the child is given no device — every +/// caller but the device manager. Matches `device-manager-protocol.no_device`, which +/// is the same sentinel one layer up. +pub const no_device: u64 = ~@as(u64, 0); + /// `futex_wait` return codes (in rax). pub const futex_woken: u64 = 0; // woken by a futex_wake pub const futex_mismatch: u64 = 1; // *addr != expected on entry; the caller did not block diff --git a/system/kernel/process.zig b/system/kernel/process.zig index 5efecc8..756e783 100644 --- a/system/kernel/process.zig +++ b/system/kernel/process.zig @@ -1017,6 +1017,13 @@ fn systemSpawn(state: *architecture.CpuState) void { const arguments_ptr = architecture.systemCallArg(state, 2); const arguments_len = architecture.systemCallArg(state, 3); const exit_handle = architecture.systemCallArg(state, 4); + // A device the caller holds and gives to the child. Fused into the spawn rather + // than transferred after it, because a separate transfer leaves a window in which + // the child is running and does not yet hold its device — a race that would close + // on one machine and open on another, which is the failure shape this whole track + // exists to remove (docs/bounds-track-plan.md, "the grant rides system_spawn"). + // Here the child cannot observe the gap: it does not exist until it holds it. + const device_to_give = architecture.systemCallArg(state, 5); const t = scheduler.current(); if (len == 0 or len > scheduler.maximum_task_name or ptr >= user_half_end or ptr + len > user_half_end) return fail(state); if (arguments_len > maximum_argument_bytes) return fail(state); @@ -1061,7 +1068,26 @@ fn systemSpawn(state: *architecture.CpuState) void { } } + // Refuse before creating anything if the device is not the caller's to give — a + // spawn that half-succeeds would leave a child running without the hardware it was + // spawned for, which is worse than not spawning it. + if (device_to_give != abi.no_device and devices_broker.ownerOf(device_to_give) != t.id) + return failErr(state, ipc.EPERM); + const child = spawnProcessSupervised(item.blob, 4, argv[0..argc], t.id, exit_endpoint) catch return fail(state); + + if (device_to_give != abi.no_device) { + devices_broker.transfer(device_to_give, t.id, child) catch { + // Cannot happen — ownership was checked above and the lock has not been + // dropped — but a spawned child holding nothing is not something to guess + // about, so say so rather than leave it silent. + log.print("/system/kernel: WARNING spawn gave device {d} to task {d} and the transfer failed\n", .{ device_to_give, child }); + }; + if (devices_broker.pciAddressOf(device_to_give)) |_| { + iommu.reassign(device_to_give, child); + dmaBindOwnerRegionsInto(child, device_to_give); + } + } architecture.setSystemCallResult(state, child); } diff --git a/system/kernel/tests.zig b/system/kernel/tests.zig index 5b16627..5217221 100644 --- a/system/kernel/tests.zig +++ b/system/kernel/tests.zig @@ -4257,8 +4257,12 @@ fn deviceAuthorityTest(boot_information: *const BootInformation) void { process.setInitialRamdisk(image); check("device-authority-test spawned", spawnNamedWithArg(rd, "device-authority-test", "run")); - const pass_marker = "device-authority: ok"; - const fail_marker = "device-authority: FAIL"; + // The VERDICT prefix matters: the fixture prints one "device-authority: ok " + // line per assertion, so a marker of "device-authority: ok" matches the FIRST + // passing assertion and this loop exits before any later failure is printed — the + // case then passes with failures in it, which it did until this was caught. + const pass_marker = "device-authority: VERDICT ok"; + const fail_marker = "device-authority: VERDICT FAILED"; scheduler.setPriority(1); const deadline = architecture.millis() + 20000; var saw_pass = false; diff --git a/system/services/device-manager/device-manager.zig b/system/services/device-manager/device-manager.zig index f2aff2a..4bf7525 100644 --- a/system/services/device-manager/device-manager.zig +++ b/system/services/device-manager/device-manager.zig @@ -306,7 +306,14 @@ fn spawnDriver(driver: *Driver) void { arguments[0] = std.fmt.bufPrint(&id_text, "{d}", .{driver.device_id}) catch return; argument_count = 1; } - const child = process.spawnSupervised(driver.name(), arguments[0..argument_count], manager_endpoint) orelse { + // The device rides the spawn, so the driver holds it before its first instruction. + // A transfer *after* spawning would leave a window in which the child is running + // without its hardware — closed on one machine, open on another + // (docs/bounds-track-plan.md, "the grant rides system_spawn"). + const give = if (isDelegated(driver.name())) driver.device_id else device_manager_protocol.no_device; + if (give != device_manager_protocol.no_device) + std.log.info("delegated device {d} to {s}", .{ give, driver.name() }); + const child = process.spawnSupervisedWithDevice(driver.name(), arguments[0..argument_count], manager_endpoint, give) orelse { std.log.info("failed to spawn {s}", .{driver.name()}); driver.state = .failed; return; @@ -467,22 +474,6 @@ fn onHello(_: void, invocation: Invocation(device_manager_protocol.Hello), _: An }; driver.state = .running; - // **Delegation.** The manager holds this driver's device and hands it over here — - // what replaces first-come-first-served `device_claim` with policy - // (docs/os-development/device-authority.md). `invocation.sender` is the driver's - // task id stamped by the kernel, so the manager cannot be lied to about who is - // asking, and the transfer is a move: the manager stops holding it. - // - // Gated on the delegated set so an unconverted driver still claims for itself and - // its path is untouched; the set and `device_claim` both go at D6. - if (isDelegated(driver.name()) and driver.device_id != device_manager_protocol.no_device) { - device.transfer(driver.device_id, invocation.sender) catch |e| { - std.log.warn("could not delegate device {d} to {s}: {s}", .{ driver.device_id, driver.name(), @errorName(e) }); - return -envelope.EPERM; - }; - std.log.info("delegated device {d} to {s}", .{ driver.device_id, driver.name() }); - } - std.log.info("hello from {s} (device {d})", .{ driver.name(), invocation.target }); // Resilience drill (V6): once, kill the virtio-gpu driver a moment after it hellos, so // the normal restart policy respawns it — the compositor must survive and re-attach. 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 ee965b3..fea745b 100644 --- a/test/system/services/device-authority-test/device-authority-test.zig +++ b/test/system/services/device-authority-test/device-authority-test.zig @@ -43,6 +43,7 @@ fn line(comptime format: []const u8, arguments: anytype) void { } var failures: usize = 0; +var process_table: [64]process.ProcessDescriptor = undefined; fn check(name: []const u8, ok: bool) void { if (!ok) failures += 1; @@ -74,10 +75,22 @@ fn run() void { const absent = if (device.transfer(0xFFFF_FFFF, process.taskId())) |_| false else |e| e == error.NoSuchDevice; check("a device that does not exist is refused as absent", absent); + // 4. **The spawn is not a second way in.** A device now rides system_spawn, which + // would be a fine back door if the kernel checked ownership any less carefully + // there than it does in transfer: spawn a child, name someone else's device, and + // the child holds hardware nobody gave it. The refusal must happen before the + // child exists, so nothing is left running either. + if (seen != 0) { + const before = process.processes(&process_table); + const spawned = process.spawnSupervisedWithDevice("/test/system/services/device-authority-test", &.{}, null, table[0].id); + check("spawning with a device the caller does not hold is refused", spawned == null); + check("and no child was left behind by the refusal", process.processes(&process_table) == before); + } + if (failures == 0) { - line("device-authority: ok ({d} devices, none of them mine)\n", .{seen}); + line("device-authority: VERDICT ok ({d} devices, none of them mine)\n", .{seen}); } else { - line("device-authority: FAILED {d} assertion(s)\n", .{failures}); + line("device-authority: VERDICT FAILED {d} assertion(s)\n", .{failures}); } }