Re: [PATCH v6 15/20] rust: io: implement a view type for `Coherent`

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `CoherentView::as_mut` allows converting a shared reference `&Coherent<T>` into a mutable reference `&mut T` on another thread without requiring `T: Send`.
--

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

rust: io: implement a view type for Coherent

Implement a CoherentView type which is a view of Coherent. To be able
to give out DMA handles, the view type contains both CPU and DMA pointers,
and the projection method projects both at once. Delegate most of the Io
implementation to SysMemBackend.

> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index 200def84fb69e..1535bc6eec642 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs

[ ... ]

> +impl<'a, T: ?Sized> CoherentView<'a, T> {

[ ... ]

> +    /// Returns a mutable reference to the data in the region.
> +    ///
> +    /// # Safety
> +    ///
> +    /// * Callers must ensure that the device does not read/write to/from memory while the returned
> +    ///   reference is live.
> +    /// * Callers must ensure that this call does not race with a read (including call to `as_ref`)
> +    ///   or write (including call to `as_mut`) to the same region while the returned reference is
> +    ///   live.
> +    #[inline]
> +    pub unsafe fn as_mut(self) -> &'a mut T {
> +        // SAFETY: pointer is aligned and valid per type invariant. Aliasing rule is satisfied per
> +        // safety requirement.
> +        unsafe { &mut *self.cpu_addr.as_ptr() }
> +    }

[Severity: Medium]
Since as_mut() returns a mutable reference &'a mut T, could this allow a
type that is not thread-safe to be mutated on another thread?

If a custom type T is Sync but not Send, CoherentView could still be sent
across threads. Calling as_mut() on the second thread would then yield a
mutable reference to a !Send type, which seems to bypass Rust's thread
safety guarantees.

Does as_mut() need a T: Send bound to ensure soundness?

[ ... ]

> +impl<'a, T: ?Sized + KnownSize> IoBase<'a> for &'a Coherent<T> {
> +    type Backend = CoherentIoBackend;
> +    type Target = T;
> +
> +    #[inline]
> +    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_handle: self.dma_handle,
> +        }
> +    }
> +}

[Severity: Medium]
This isn't a bug in the trait implementation itself, but this is where
the shared reference &'a Coherent<T> gets converted into a CoherentView.

Combined with the as_mut() method above, does this create a path to gain
mutable access from a shared reference without a T: Send requirement?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-io_projection-v6-0-72cd5d055d54@garyguo.net?part=15