diff --git a/docs/bounds-track-plan.md b/docs/bounds-track-plan.md index 9c69f37..90d34c5 100644 --- a/docs/bounds-track-plan.md +++ b/docs/bounds-track-plan.md @@ -17,7 +17,12 @@ next one starts.* | L3 | xHCI: slot count from `HCSPARAMS1.MaxSlots`, not 8 | **done** — QEMU reports 64; the driver tracked 8 | | L4 | USB: configuration descriptor sized by `wTotalLength`, not 512 | **done** — QEMU tops out at 211 bytes, so the case catches the class, not the original trigger | | L5 | USB: interfaces from the descriptor, and the misattributed-endpoint bug | **done** — fix is by construction; no direct test, see open question 5 | -| L6 | xHCI: a failed `allocateDevice` stops leaking an enabled slot | not started | +| L6 | xHCI: a failed `allocateDevice` stops leaking an enabled slot | **done** — path forced and verified; no regression test, see open question 5 | + +**Run complete.** L1 stopped (the step was wrong), L2–L6 landed. Suite 115 → 116. +Allowlist 278 → 269. Two steps ship without a permanent regression test, both because +QEMU's USB devices are too small to reach the paths — see open question 5, which is the +audit's own lesson recurring: the test rig is smaller than a real machine. **Suite:** 115/115 at the start of the run. **Branch:** `claude/bounds-track`. diff --git a/system/drivers/usb-xhci-bus/usb-xhci-library.zig b/system/drivers/usb-xhci-bus/usb-xhci-library.zig index 7f3a9dc..9d0186b 100644 --- a/system/drivers/usb-xhci-bus/usb-xhci-library.zig +++ b/system/drivers/usb-xhci-bus/usb-xhci-library.zig @@ -943,7 +943,8 @@ pub const Controller = struct { return null; }; const device = self.allocateDevice() orelse { - std.log.info("port {d} setup: no free device slot", .{port}); + std.log.warn("port {d} setup: no free device slot", .{port}); + self.disableSlot(slot_id); // the controller already gave it to us return null; }; device.* = .{ @@ -1145,7 +1146,11 @@ pub const Controller = struct { std.log.info("hub slot {d} port {d}: Enable Slot failed", .{ hub.slot_id, port }); return null; }; - const device = self.allocateDevice() orelse return null; + const device = self.allocateDevice() orelse { + std.log.warn("hub slot {d} port {d}: no free device slot", .{ hub.slot_id, port }); + self.disableSlot(slot_id); // the controller already gave it to us + return null; + }; const child_speed = mapHubPortSpeed(speed); device.* = .{ .used = true, @@ -1747,15 +1752,27 @@ pub const Controller = struct { for (&self.subscriptions) |*subscription| { if (subscription.active and subscription.slot_id == device.slot_id) subscription.active = false; } + self.disableSlot(device.slot_id); + device.freeInterfaces(); + device.used = false; + } + + /// Hand a slot back to the controller and clear its context-array entry. + /// + /// Every path that has issued a successful Enable Slot owes this, including the + /// ones that then fail to bring the device up. A slot the driver forgets is one + /// the controller never reissues, so the loss is permanent for the boot: the two + /// setup paths used to return null straight after a failed `allocateDevice`, + /// leaking a slot per attempt — and the hub path did it without even a log line. + fn disableSlot(self: *Controller, slot_id: u8) void { const physical = self.submitCommand(.{ - .control = trbControl(.disable_slot, @as(u32, device.slot_id) << 24), + .control = trbControl(.disable_slot, @as(u32, slot_id) << 24), }); if (self.awaitCommand(physical)) |code| { if (code != @intFromEnum(CompletionCode.success)) - std.log.info("slot {d}: Disable Slot completion code {d}", .{ device.slot_id, code }); - } else std.log.info("slot {d}: Disable Slot timed out", .{device.slot_id}); + std.log.info("slot {d}: Disable Slot completion code {d}", .{ slot_id, code }); + } else std.log.info("slot {d}: Disable Slot timed out", .{slot_id}); const array: [*]volatile u64 = @ptrFromInt(self.device_context_array.virtual); - array[device.slot_id] = 0; - device.used = false; + array[slot_id] = 0; } };