diff --git a/issues/a-bars-mapping-outlives-the-claim-it-was-asked-on.md b/issues/a-bars-mapping-outlives-the-claim-it-was-asked-on.md new file mode 100644 index 00000000000..a3c7ce317b5 --- /dev/null +++ b/issues/a-bars-mapping-outlives-the-claim-it-was-asked-on.md @@ -0,0 +1,38 @@ +--- +status: open +kind: defect +opened: 2026-10-08 +--- + +# A BAR's mapping outlives the claim it was asked on + +`SYS_DEVICE_BAR_MAP` answers a `SharedMem` handle over a claimed function's +register window, carrying `MAP`, `DUP` and `TRANSFER` like any other +(`ops::initial_rights`). The object's life is its handles' and nothing ties it +to the claim: `pcidev::tear_down` (`kernel/src/pcidev/mod.rs`) stops the +function, resets it and takes its grants back, and touches no mapping of its +BARs. A window belongs to its BAR for the boot (`Machine::windows`), so the +next claim on the function is given the same address. The claim holds no +reference to an object `pcidev::bar_object` made for it, so the exit below +needs the claim to learn what it handed out. + +So a process that asked for a BAR, kept that handle — or sent it on — and let +the claim go still reads and writes the registers of a function its next +holder drives: it can aim that function's queues at any address the new +holder's grants map, which is the new holder's memory read out and written +through a device neither the unit nor the domain refuses. + +**Read from the code, not run.** A guest arm is the one `bar_map_again` +(`tests/toyos.rs`) boots: claim the virtio NIC, map a BAR, close the claim, +claim again from a second process, and read the first mapping. + +The tracker has ruled on taking a mapping back from a running process +(`issues/a-moved-handle-is-always-re-movable.md`, "Revocation is the other +answer"): forced reclaim is killing the holder. What that ruling means for a +window whose authority was a claim that has ended is not decided there. + +**Exit condition**: after a claim ends, no mapping made through it reaches +the function's registers once another claim holds the function, and a guest +test reads that. + +**Owner**: whoever holds `issues/every-driver-is-still-in-the-kernel.md`. diff --git a/issues/a-claims-bar-asked-for-again-after-its-handle-closed-panics-the-kernel.md b/issues/a-claims-bar-asked-for-again-after-its-handle-closed-panics-the-kernel.md deleted file mode 100644 index 15fc1e352a1..00000000000 --- a/issues/a-claims-bar-asked-for-again-after-its-handle-closed-panics-the-kernel.md +++ /dev/null @@ -1,44 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-08 ---- - -# A claim's BAR asked for again after its handle closed panics the kernel - -`pcidev::bar_object` (`kernel/src/pcidev/mod.rs`) makes one object per memory -BAR and keeps it in the claim's binding, to answer every later -`SYS_DEVICE_BAR_MAP` of that BAR with "the same object every time". The object -it keeps is counted by its handles like any other: when the one handle the -first answer installed closes, `HandleEntry`'s drop -(`kernel/src/object/handle.rs`) retires it. The binding still holds it, so the -next request for that BAR hands the retired object to `ops::install`, and -`HandleEntry::new` asserts against exactly that. The holder of a live claim -takes the machine down with three ordinary calls: ask, close, ask. - -The same object is retired without a close when the first install is refused -for a full handle table: the entry built for it is dropped. Read from the -code, not run. - -**Measured** at `06bbc236d`, on `tests/testcases` under QEMU with the virtio -NIC as the claimed function: `cargo test --test toyos-build -- bar_map_again` -exits 1 on - - PANIC: panicked at src/object/handle.rs:108:9: - a handle to a retired SharedMem (koid 197) - -after the job's line `bar_map_again: BAR 4 asked for and its handle closed; -asking again`. - -No metal row was run: the T14's one function a job can claim is its I219, and -a red here is the bench's machine in a panic. - -**Exit condition**: a BAR asked for again on a live claim, after every handle -to the first answer has gone, is answered with a handle that maps or with a -refusal, and `bar_map_again` is green; the arm where the first install is -refused for room is read by a test as well. - -**Owner**: whoever holds `issues/every-driver-is-still-in-the-kernel.md`. - -**Its test is deleted**: `a86cafa06` took `bar_map_again` out, and -`git revert a86cafa06` brings it back. diff --git a/kernel/src/object/mod.rs b/kernel/src/object/mod.rs index da60f23bcb1..d377a1408a8 100644 --- a/kernel/src/object/mod.rs +++ b/kernel/src/object/mod.rs @@ -2,6 +2,8 @@ //! //! Objects are plain `Arc`: no custom refcounting, no `Weak`, no `dyn` hierarchy. //! +//! An object ends with its last handle and is never handed out again: a holder that answers more than once keeps what an object is made over (`device::Screen`, a claim's BAR windows) and makes a fresh object for each answer. +//! //! `handle_count`, not the Arc strong count, is what userland-visible lifecycle rides: a syscall's `Arc` can be stranded on a killed thread's kernel stack, so release is deferred through [`ZERO_QUEUE`] — see `issues/deferred-release-outlives-its-syscall.md`. // `warn` here gates via CI's `-D warnings`; the rest of the kernel is not yet swept. diff --git a/kernel/src/pcidev/mod.rs b/kernel/src/pcidev/mod.rs index c0ab7cba7ad..3a0bdffbf78 100644 --- a/kernel/src/pcidev/mod.rs +++ b/kernel/src/pcidev/mod.rs @@ -238,9 +238,6 @@ struct Bound { /// advertises; 0 bytes is a slot with no BAR this claim may map. bar_at: [u64; BARS], bar_bytes: [u64; BARS], - /// Minted on the first map of that BAR, so a second answers the same - /// object rather than a second handle to one window. - bars: [Option>; BARS], grants: Vec, /// The [`RESIDUE`] this claim took over and has placed no grant at: mapped /// to nothing and counted against nothing, and where a grant of a range's @@ -819,7 +816,6 @@ fn bring_up(pci: PciDevice, id: PciId, slot: usize) -> Result { id, bar_at, bar_bytes, - bars: [const { None }; BARS], grants: Vec::new(), residue: Vec::new(), mastering: false, @@ -1509,26 +1505,25 @@ fn with_bound( f(bound) } -/// One memory BAR as an object to map; the same object every time. +/// One memory BAR as an object to map, a fresh one for every ask. +/// +/// **The claim keeps where the window is and never an object over it**: an +/// object ends with its last handle, which its holder closes whenever it +/// likes, and the claim outlives that. pub fn bar_object(slot: usize, index: u64) -> Result, SyscallError> { with_bound(slot, |bound| { let index = usize::try_from(index).map_err(|_| SyscallError::InvalidArgument)?; if index >= BARS || bound.bar_bytes[index] == 0 { return Err(SyscallError::InvalidArgument); } - if let Some(object) = bound.bars[index].as_ref() { - return Ok(Arc::clone(object)); - } - let object = SharedMemObject::over(Region { + Ok(SharedMemObject::over(Region { phys: DirectMap::from_phys(bound.bar_at[index]), size: align_2m(bound.bar_bytes[index] as usize) as u64, // Registers, and the memory type is not firmware's to decide. cache: CachePolicy::Uncacheable, // The kernel owns no pages here: this is a device's aperture. pages: None, - }); - bound.bars[index] = Some(Arc::clone(&object)); - Ok(object) + })) }) } diff --git a/tests/toyos-rust-tests/src/bin/bar_map_again.rs b/tests/toyos-rust-tests/src/bin/bar_map_again.rs new file mode 100644 index 00000000000..5d835f00daf --- /dev/null +++ b/tests/toyos-rust-tests/src/bin/bar_map_again.rs @@ -0,0 +1,105 @@ +//! One memory BAR of a claimed function, asked for more than once on the same +//! live claim: every answer is a handle to an object of its own, and the +//! function's registers are behind its mapping. Three times — while an earlier +//! answer is still held, where the two are two mappings and the later one +//! outlives the earlier; after a close; and after an ask a full handle table +//! refused, which is an object whose only handle went before its holder ever +//! had it. +//! +//! The function is QEMU's virtio NIC, which no program of `tests/testcases` +//! holds. + +use toyos::endow::Endowments; +use toyos::shm::SharedMemory; +use toyos::syscap::SysCap; +use toyos::{AsHandle, PciDev}; +use toyos_abi::handle::RawHandle; +use toyos_abi::syscall::{self, PciId, SyscallError, SYSCAP_LABEL}; + +const NIC: PciId = PciId { vendor: 0x1af4, device: 0x1041 }; + +/// `device_feature` of the common configuration QEMU puts at the base of the +/// NIC's one mappable BAR (virtio 1.2 §4.1.4.3): a dword that is the device's +/// own, never zero for a NIC, and that no read changes. +const DEVICE_FEATURE: usize = 4; + +/// What the function answers through `window`, a live mapping of its BAR. +fn read(window: &SharedMemory) -> u32 { + // SAFETY: `map_bar` answered a live mapping of the BAR, the dword is inside + // the sixteen bytes a memory BAR has at least, and a device register is read + // by a volatile load. + let answered = unsafe { window.as_ptr().add(DEVICE_FEATURE).cast::().read_volatile() }; + assert!( + answered != 0 && answered != u32::MAX, + "bar_map_again: the mapping answers {answered:#010x}, which is no device's register" + ); + answered +} + +/// What the function answers through a fresh ask of `bar`, with the handle +/// and the mapping let go again before this returns. +fn ask(nic: &PciDev, bar: u32, bytes: u64) -> u32 { + read(&nic.map_bar(bar, bytes).expect("bar_map_again: the BAR, asked for and mapped")) +} + +fn main() { + let cap: SysCap = Endowments::get() + .take(SYSCAP_LABEL) + .expect("the test estate is endowed a device-minting capability"); + let nic: PciDev = cap.claim_pci(NIC).expect("bar_map_again: the virtio NIC's claim"); + let info = nic.describe().expect("bar_map_again: the claim's description"); + let bar = info + .bar_bytes + .iter() + .position(|&bytes| bytes != 0) + .expect("bar_map_again: the function has a memory BAR its holder may map"); + let bytes = info.bar_bytes[bar]; + let bar = bar as u32; + + // Two answers held at once are two mappings, and letting the earlier one + // go takes nothing from the later. + let earlier = nic.map_bar(bar, bytes).expect("bar_map_again: the BAR, asked for and mapped"); + let later = nic.map_bar(bar, bytes).expect("bar_map_again: the BAR, asked for while held"); + assert_ne!( + earlier.as_ptr(), + later.as_ptr(), + "bar_map_again: two answers held at once are one mapping" + ); + let first = read(&earlier); + assert_eq!(read(&later), first, "bar_map_again: the two mappings are two windows"); + drop(earlier); + assert_eq!(read(&later), first, "bar_map_again: the later mapping changed with the earlier's end"); + drop(later); + println!("bar_map_again: two answers held at once map apart, and the later outlives the earlier"); + + println!("bar_map_again: BAR {bar} asked for and its handle closed; asking again"); + let again = ask(&nic, bar, bytes); + assert_eq!(again, first, "bar_map_again: the second ask's mapping is another window"); + println!("bar_map_again: answered with a handle that maps"); + + // No room for the handle: the ask is refused, and the object made for it + // went with the handle the table would not take. + // Standard output fills the table: a claim's own handle carries no `DUP`. + let mut filled = Vec::new(); + let full = loop { + match syscall::dup(RawHandle(1)) { + Ok(handle) => filled.push(handle), + Err(why) => break why, + } + }; + let refused = syscall::device_bar_map(nic.as_handle(), bar); + for handle in filled { + syscall::close(handle); + } + // Judged once the table has room again, for the report. + assert_eq!(full, SyscallError::ResourceExhausted, "bar_map_again: the table did not fill"); + assert_eq!( + refused, + Err(SyscallError::ResourceExhausted), + "bar_map_again: an ask from a full table was not refused for room" + ); + println!("bar_map_again: an ask from a full handle table refused; asking again"); + let after = ask(&nic, bar, bytes); + assert_eq!(after, first, "bar_map_again: the ask after the refusal maps another window"); + println!("bar_map_again: answered after the refusal with a handle that maps"); +} diff --git a/tests/toyos.rs b/tests/toyos.rs index 91c55f30b34..961a2064017 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -172,6 +172,9 @@ const RUST_SKIP: &[&str] = &[ "acpi_mediated", // It powers the machine off: `machine_shutdown_short_stop` runs it. "stop_short", + // It claims QEMU's virtio NIC, which the T14 has none of: `bar_map_again` + // runs it. + "bar_map_again", ]; /// Binaries a metal row or a guest test drives that the shared boot also runs @@ -275,6 +278,11 @@ const MACHINE_TESTS: &[&str] = &[ // ends the machine, so only one QEMU reports stopping can be asked, and // the T14 hands over in legacy mode, where no holder means no quieting. "machine_shutdown_short_stop", + // A claimable function with a mappable BAR that no program of the boot + // holds: QEMU's virtio NIC on `tests/testcases`. The T14's one such + // function is its I219, and a kernel that dies on this takes the bench's + // machine down with it. + "bar_map_again", ]; /// **The metal profile**: which registrations run on the ThinkPad T14, what @@ -2796,10 +2804,42 @@ fn run_machine_test(name: &str, test_config: &Path) -> Result<(), String> { "acpi_power_button" => power::acpi_power_button(test_config), "acpi_mediated_access" => acpi_mediated_access(), "machine_shutdown_short_stop" => power::machine_shutdown_short_stop(test_config), + "bar_map_again" => bar_map_again(test_config), other => Err(format!("unknown machine test {other}")), } } +/// A claim's memory BAR asked for again — while an earlier answer is held, and +/// once its handle is gone, closed or never installed for want of room: each +/// time the kernel answers a handle whose mapping reads the function's own +/// register, two held at once map apart, and the job that asked ends on its +/// own. +fn bar_map_again(test_config: &Path) -> Result<(), String> { + const JOB: &str = "bar_map_again"; + let bin = qemu::build_toyos_bin(qemu::SUITE_ARCH, &compile::repo_root().join("tests/toyos-rust-tests"), JOB); + let mut qemu = + QemuInstance::boot_with_options(test_config, &[], &[(JOB.to_string(), bin)], BootOptions::default()); + serial::Serial::boot(&qemu).must_be_clean()?; + let result = qemu.run_test("test_rs_bar_map_again", Duration::from_secs(60)); + if let Some(why) = &result.error { + return Err(format!("{why}\nthe job said:\n{}", result.stdout)); + } + if result.exit_code != Some(0) { + return Err(format!("the job ended {:?}:\n{}", result.exit_code, result.stdout)); + } + for answered in [ + "bar_map_again: two answers held at once map apart, and the later outlives the earlier", + "bar_map_again: answered with a handle that maps", + "bar_map_again: answered after the refusal with a handle that maps", + ] { + let Some(line) = result.stdout.lines().find(|l| l.contains(answered)) else { + return Err(format!("the job never said `{answered}`:\n{}", result.stdout)); + }; + eprintln!(" [claims] {}", line.trim()); + } + Ok(()) +} + /// Every device delivery is still cpu0's, and no CPU took a shootdown IPI the /// issuer did not count. /// diff --git a/toyos-abi/src/syscall.rs b/toyos-abi/src/syscall.rs index 4a829eb95af..a9f4424e4fb 100644 --- a/toyos-abi/src/syscall.rs +++ b/toyos-abi/src/syscall.rs @@ -2364,7 +2364,8 @@ pub fn pipe_map(handle: RawHandle) -> Result<*mut u8, SyscallError> { /// kernel's, and a process that could write it could aim the device's interrupt /// at any address the LAPIC decodes. /// -/// Idempotent per BAR: a second call answers the same object. +/// Every call answers an object of its own over the same window, so a BAR +/// whose handle was closed can be asked for again. /// /// A window alone drives nothing: the function masters the bus from its first /// [`device_dma_alloc`] and not before.