Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Safe constructors of `Coherent` and `CoherentBox` allow leaking DMA allocations (e.g., via `std::mem::forget`), causing device resources like IOMMU mappings to outlive the device's bound lifetime, which can lead to Undefined Behavior upon device unbind. --
commit 6e4d094e50aec8e58639ef61e8619143bb52d532 Author: Danilo Krummrich <[email protected]> Subject: rust: dma: tie Coherent and CoherentBox to the device's bound lifetime This commit adds a lifetime parameter to Coherent and CoherentBox that ties the DMA allocation to the device's bound scope. This helps ensure it is freed before the device is unbound. > diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs > index 79f453e9ec0b6..4ce914b7d1dad 100644 > --- a/rust/kernel/dma.rs > +++ b/rust/kernel/dma.rs [ ... ] > @@ -663,10 +656,10 @@ pub unsafe fn as_mut(&self) -> &mut T { > } > } > > -impl<T: AsBytes + FromBytes> Coherent<T> { > +impl<'a, T: AsBytes + FromBytes> Coherent<'a, T> { > /// Allocates a region of `T` of coherent memory. > fn alloc_with_attrs( > - dev: &device::Device<Bound>, > + dev: &'a device::Device<Bound>, > gfp_flags: kernel::alloc::Flags, > dma_attrs: Attrs, > ) -> Result<Self> { [Severity: Medium] Is it safe to expose constructors like alloc_with_attrs() as safe APIs given that the returned object can be deliberately leaked via safe code? The documentation on Coherent notes this is an accepted soundness hole, but allowing a DMA allocation to be leaked (e.g., via std::mem::forget) means device resources like IOMMU mappings can outlive the device's bound lifetime. If the device unbinds and its structures are torn down, could these leaked active DMA mappings lead to memory corruption or undefined behavior? Since exposing a safe API that can trigger undefined behavior through entirely safe code violates standard safety guarantees, would it make sense to mark alloc_with_attrs() and similar allocation functions as unsafe? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
