Re: [PATCH v5 1/4] rust: io: require freedom from plain accesses for `SysMem`

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] rust: io: SysMem::new() no-plain-access invariant violated by
  callers
--

commit 2301a5fd31bd2665dfd4001d0e9beab25a81725b
Author: Andreas Hindborg <a.hindborg@kernel.org>

rust: io: require freedom from plain accesses for `SysMem`

This commit adds a type invariant to SysMem and a safety requirement on
SysMem::new() requiring that no plain accesses are performed to the memory
region for the lifetime of the view.

> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index a877df11a03e0..3bc4be35e3554 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
[ ... ]
> @@ -1570,8 +1582,10 @@ impl<'a, T: ?Sized> SysMem<'a, T> {
>      ///
>      /// # Safety
>      ///
> -    /// `ptr.addr() .. ptr.addr() + KnownSize::size(ptr)` must be valid and aligned kernel
> -    /// accessible memory region for the lifetime `'a`.
> +    /// - `ptr.addr() .. ptr.addr() + KnownSize::size(ptr)` must be valid and aligned kernel
> +    ///   accessible memory region for the lifetime `'a`.
> +    /// - No plain access (see [`SysMem`]) to that memory region may be performed for the lifetime
> +    ///   `'a`.
>      #[inline]
>      pub unsafe fn new(ptr: *mut T) -> Self {
>          // INVARIANT: Per safety requirement.

[Severity: Medium]
Does this new SysMem<'a, T> invariant conflict with the existing
SysMem::new() callers in Coherent and VMap?

In rust/kernel/dma.rs, <&'a Coherent<'_, T> as IoBase<'a>>::as_view() is a
safe method that wraps the buffer in SysMem<'a, T> inside CoherentView<'a, T>,
while CoherentView::as_ref() and CoherentView::as_mut() hand out &'a T and
&'a mut T references for plain accesses over the same lifetime 'a:

rust/kernel/dma.rs:CoherentView::as_ref() {
    pub unsafe fn as_ref(self) -> &'a T {
        // SAFETY: pointer is aligned and valid per type invariant. Aliasing rule is satisfied per
        // safety requirement.
        unsafe { &*self.cpu_addr.as_ptr() }
    }
}

rust/kernel/dma.rs:<&'a Coherent<'_, T> as IoBase<'a>>::as_view() {
    fn as_view(self) -> CoherentView<'a, Self::Target> {
        CoherentView {
            // SAFETY: `cpu_addr` is valid and aligned kernel accessible memory.
            cpu_addr: unsafe { SysMem::new(self.cpu_addr.as_ptr()) },
            dma_addr: self.dma_addr,
        }
    }
}

Any plain read or write through those references during 'a violates the
SysMem<'a, T> invariant even when no concurrent access occurs.  Similarly,
<&'a VMap<D, R, SIZE> as IoBase<'a>>::as_view() in
rust/kernel/drm/gem/shmem.rs calls SysMem::new() without VMap's invariants or
its SAFETY comment guaranteeing that no plain accesses occur during 'a.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-page-volatile-io-v5-0-a91ef252c24b@kernel.org?part=1