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 <[email protected]> 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/[email protected]?part=15
