Re: [PATCH v4 09/20] rust: io: use view types instead of addresses for `Io`

[Alexandre Courbot]

On Tue Jun 16, 2026 at 11:50 PM JST, Gary Guo wrote:
> On Tue Jun 16, 2026 at 3:05 PM BST, Alexandre Courbot wrote:
>> On Fri Jun 12, 2026 at 1:28 AM JST, Gary Guo wrote:
>>> Currently, `io_read` and `io_write` methods require the exact type of `Io`
>>> plus an address. This means that they need to be monomorphized for each
>>> different `Io` instance. This also means that multiple I/O implementors for
>>> the same I/O kind needs to duplicate implementation (e.g. `Mmio` and
>>> `MmioOwned`).
>>>
>>> Create a new `IoBackend` trait and define these operations on it instead.
>>> The operations are just going to receive a view type and operate on them.
>>> This has the additional advantage that the invariants can be moved from the
>>> trait (and guaranteed via `unsafe`) to type invariants on the canonical
>>> view types of the backends, so `io_read` and `io_write` can be safe.
>>>
>>> Note that view type is needed; addresses are insufficient in this
>>> designk, as they do not carry sufficient information. For example,
>>
>> typo: design
>>
>>> `ConfigSpace` needs `&pci::Device` in addition to the address.
>>>
>>> Signed-off-by: Gary Guo <gary@xxxxxxxxxxx>
>>> ---
>>>  rust/kernel/io.rs     | 345 ++++++++++++++++++++++++++------------------------
>>>  rust/kernel/pci/io.rs |  70 ++++++----
>>>  2 files changed, 224 insertions(+), 191 deletions(-)
>>>
>>> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
>>> index 3ac8b396f5a7..e422a5ff2a5e 100644
>>> --- a/rust/kernel/io.rs
>>> +++ b/rust/kernel/io.rs
>>> @@ -244,6 +244,38 @@ const fn offset_valid<U>(base: usize, offset: usize, size: usize) -> bool {
>>>      }
>>>  }
>>>  
>>> +/// I/O backends.
>>> +///
>>> +/// This is an abstract representation to be implemented by arbitrary I/O
>>> +/// backends (e.g. MMIO, PCI config space, etc.).
>>> +///
>>> +/// The base trait only defines the projection operations; which I/O methods are available depends
>>> +/// on which [`IoCapable<T>`] traits are implemented for the type. For example, for MMIO regions,
>>> +/// all widths (u8, u16, u32, and u64 on 64-bit systems) are typically supported. For PCI
>>> +/// configuration space, u8, u16, and u32 are supported but u64 is not.
>>> +///
>>> +/// This trait is separate from the `Io` trait as multiple different I/O types may share the same
>>> +/// operation.
>>> +pub trait IoBackend {
>>> +    /// View type for this I/O backend.
>>> +    type View<'a, T: ?Sized + KnownSize>: Io<'a, Backend = Self, Target = T>;
>>> +
>>> +    /// Convert a `view` to a raw pointer for projection.
>>> +    fn as_ptr<'a, T: ?Sized + KnownSize>(view: Self::View<'a, T>) -> *mut T;
>>
>> Same as the previous patch, this pointer is not necessarily
>> dereferencable (e.g. for `pci::ConfigSpace`). This should probably be
>> mentioned, or maybe we can use a newtype to prevent dereferencing?
>
> This needs to be a pointer for pointer projection to work. I think the fact
> they're raw pointers is a very good indication that they can realisticly be
> anything; but I can add a note like
>
>     /// Convert a `view` to a raw pointer for projection.
>     ///
>     /// The returned pointer is private implementation detail of the backend;
>     /// it is likely not valid. It should be used for projection only.

Sounds good, the limitation should indeed be taken for granted given
that this is I/O, but making it explicit is helpful nonetheless.

<...>
>> And potentially, a more serious issue: `io_addr_assert` and `io_addr`
>> remain part of `Io`, which is a public trait. They only verify size and
>> alignment for `U`, not whether a projection of `U` at `offset` is
>> structurally valid. AFAICT this remains that way by the end of the
>> series, so users are able to call `io_addr*` to create and use invalid
>> projections.
>
> I think this could be solved by adding `U: FromBytes + IntoBytes`? We're only
> using it form primitives anyway.
>
> Technically, even without the bound, nothing can cause any breakage anyway,
> given even after the full series to do things with the view you'd need to use
> `copy_read`/`copy_write` which themselves carry the `FromBytes`/`IntoBytes`
> bound.

I should have provided an example - here is a minimal one showing the
issue. I have put this module into nova-core and it builds just fine:

    #[allow(dead_code)]
    mod foo {
        use kernel::io::{Io, IoLoc, Mmio};
        use kernel::prelude::*;

        #[repr(C)]
        struct Regs {
            status: u64,
        }

        fn io_addr_abuse(regs: Mmio<'_, Regs>) -> Result<u32> {
            let high_half: Mmio<'_, u32> = regs.io_addr::<u32>(4)?;

            Ok(high_half.read_val())
        }
    }

As you can see, this lets a 32-bit access be done on the upper half of a
64-bit register, which sounds like it should not be allowed? Similarly
one could change register types, and so on. This might not be "unsafe"
in the sense that it is still aligned and in bounds, but it lets the
structure set by the type system be bypassed. It could also potentially
be a violation of the hardware contract if the access width is relevant
for this particular address.

The same thing can be achieved using `IoLoc`:

    struct HighHalfOfStatus;

    impl IoLoc<Regs, u32> for HighHalfOfStatus {
        type IoType = u32;

        fn offset(self) -> usize {
            4
        }
    }

    fn ioloc_abuse(regs: Mmio<'_, Regs>) -> Result<u32> {
        Ok(regs.read(HighHalfOfStatus))
    }