Re: [PATCH v5 06/20] rust: io: rename `Mmio` to `MmioOwned`

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] `IoMem::ioremap` unnecessarily rejects valid memory regions whose sizes are not multiples of 4 or whose base addresses are not perfectly 4-byte aligned.

Pre-existing issues:
- [High] Virtual address space leaked in `IoMem::ioremap` on error path.
--

commit c64b5401d29b1a7d49e9330767d6549fcf42f22c
Author: Gary Guo <gary@garyguo.net>

rust: io: rename Mmio to MmioOwned

This commit renames the owned MMIO instance from Mmio to MmioOwned.
This frees up the name Mmio to be used for the view type instead, making
it more intuitive for users who usually interact with the view.

> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index 12be266d7ed7e..f93be7f780694 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs

[Severity: High]
Does Region::ptr_try_from_raw_parts_mut() unnecessarily reject valid
memory regions?

When MmioRaw::new_region() is called, it checks the size and alignment:

rust/kernel/io.rs:Region::ptr_try_from_raw_parts_mut() {
    if size < SIZE || base.align_offset(4) != 0 || !size.is_multiple_of(4) {
        return Err(EINVAL);
    }
}

Does this cause driver probes to fail with EINVAL when trying to map valid
but small or unaligned hardware resources, such as 1-byte or 2-byte
registers?

[ ... ]

> diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs
> index 9e15bc8fde784..8f6c257c5b8e6 100644
> --- a/rust/kernel/io/mem.rs
> +++ b/rust/kernel/io/mem.rs

[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
IoMem::ioremap() leak the virtual address space mapping?

When a driver calls IoMem::ioremap():

rust/kernel/io/mem.rs:IoMem::ioremap() {
    let addr = if resource.flags().contains(io::resource::Flags::IORESOURCE_MEM_NONPOSTED) {
        unsafe { bindings::ioremap_np(res_start, size) }
    } else {
        unsafe { bindings::ioremap(res_start, size) }
    };

    if addr.is_null() {
        return Err(ENOMEM);
    }

    let io = MmioRaw::new_region(addr as usize, size)?;
    Ok(IoMem { dev, io })
}

If bindings::ioremap() succeeds but MmioRaw::new_region() fails (such as
from the size and alignment constraints mentioned above), the ? operator
returns immediately.

Does this bypass unmapping the memory, resulting in virtual address space
exhaustion, since the IoMem object is never constructed and its Drop
implementation never runs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-io_projection-v5-0-d0961471ae50@garyguo.net?part=6