From bed87375f90f44cd2f9fe804a469274127d38593 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 30 Aug 2026 17:40:34 +0200 Subject: rust: dma: tie Coherent and CoherentBox to the device's bound lifetime Add a lifetime parameter to Coherent and CoherentBox that ties the DMA allocation to the device's bound scope, ensuring it is freed before the device is unbound. DMA allocations carry device resources (e.g. IOMMU mappings) that must not outlive the device's bound lifetime. Without a lifetime parameter, there was no compile-time enforcement that a Coherent or CoherentBox is dropped before the device is unbound. Propagate the new lifetime parameter through all users. Reviewed-by: Gary Guo Link: https://patch.msgid.link/20260830193824.471089-5-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/dma.rs | 121 +++++++++++++++++++++++-------------------------- rust/kernel/uaccess.rs | 4 +- 2 files changed, 59 insertions(+), 66 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs index 79f453e9ec0b..4ce914b7d1da 100644 --- a/rust/kernel/dma.rs +++ b/rust/kernel/dma.rs @@ -24,7 +24,6 @@ use crate::{ }, prelude::*, ptr::KnownSize, - sync::aref::ARef, transmute::{ AsBytes, FromBytes, // @@ -223,7 +222,7 @@ impl DmaMask { /// /// # fn test(dev: &Device) -> Result { /// let attribs = DMA_ATTR_FORCE_CONTIGUOUS | DMA_ATTR_NO_WARN; -/// let c: Coherent<[u64]> = +/// let c: Coherent<'_, [u64]> = /// Coherent::zeroed_slice_with_attrs(dev, 4, GFP_KERNEL, attribs)?; /// # Ok::<(), Error>(()) } /// ``` @@ -390,9 +389,9 @@ impl From for bindings::dma_data_direction { /// }; /// /// # fn test(dev: &Device) -> Result { -/// let mut dmem: CoherentBox = CoherentBox::zeroed(dev, GFP_KERNEL)?; +/// let mut dmem: CoherentBox<'_, u64> = CoherentBox::zeroed(dev, GFP_KERNEL)?; /// *dmem = 42; -/// let dmem: Coherent = dmem.into(); +/// let dmem: Coherent<'_, u64> = dmem.into(); /// # Ok::<(), Error>(()) } /// ``` /// @@ -410,18 +409,18 @@ impl From for bindings::dma_data_direction { /// }; /// /// # fn test(dev: &Device) -> Result { -/// let mut dmem: CoherentBox<[u64]> = CoherentBox::zeroed_slice(dev, 4, GFP_KERNEL)?; +/// let mut dmem: CoherentBox<'_, [u64]> = CoherentBox::zeroed_slice(dev, 4, GFP_KERNEL)?; /// dmem.fill(42); -/// let dmem: Coherent<[u64]> = dmem.into(); +/// let dmem: Coherent<'_, [u64]> = dmem.into(); /// # Ok::<(), Error>(()) } /// ``` -pub struct CoherentBox(Coherent); +pub struct CoherentBox<'a, T: KnownSize + ?Sized>(Coherent<'a, T>); -impl CoherentBox<[T]> { +impl<'a, T: AsBytes + FromBytes> CoherentBox<'a, [T]> { /// [`CoherentBox`] variant of [`Coherent::zeroed_slice_with_attrs`]. #[inline] pub fn zeroed_slice_with_attrs( - dev: &device::Device, + dev: &'a device::Device, count: usize, gfp_flags: kernel::alloc::Flags, dma_attrs: Attrs, @@ -432,7 +431,7 @@ impl CoherentBox<[T]> { /// Same as [CoherentBox::zeroed_slice_with_attrs], but with `dma::Attrs(0)`. #[inline] pub fn zeroed_slice( - dev: &device::Device, + dev: &'a device::Device, count: usize, gfp_flags: kernel::alloc::Flags, ) -> Result { @@ -480,14 +479,14 @@ impl CoherentBox<[T]> { /// /// # fn test(dev: &Device) -> Result { /// let data = [0u8, 1u8, 2u8, 3u8]; - /// let c: CoherentBox<[u8]> = + /// let c: CoherentBox<'_, [u8]> = /// CoherentBox::from_slice_with_attrs(dev, &data, GFP_KERNEL, DMA_ATTR_NO_WARN)?; /// /// assert_eq!(c.deref(), &data); /// # Ok::<(), Error>(()) } /// ``` pub fn from_slice_with_attrs( - dev: &device::Device, + dev: &'a device::Device, data: &[T], gfp_flags: kernel::alloc::Flags, dma_attrs: Attrs, @@ -512,7 +511,7 @@ impl CoherentBox<[T]> { /// `dma_attrs` is 0 by default. #[inline] pub fn from_slice( - dev: &device::Device, + dev: &'a device::Device, data: &[T], gfp_flags: kernel::alloc::Flags, ) -> Result @@ -523,11 +522,11 @@ impl CoherentBox<[T]> { } } -impl CoherentBox { +impl<'a, T: AsBytes + FromBytes> CoherentBox<'a, T> { /// Same as [`CoherentBox::zeroed_slice_with_attrs`], but for a single element. #[inline] pub fn zeroed_with_attrs( - dev: &device::Device, + dev: &'a device::Device, gfp_flags: kernel::alloc::Flags, dma_attrs: Attrs, ) -> Result { @@ -536,12 +535,12 @@ impl CoherentBox { /// Same as [`CoherentBox::zeroed_slice`], but for a single element. #[inline] - pub fn zeroed(dev: &device::Device, gfp_flags: kernel::alloc::Flags) -> Result { + pub fn zeroed(dev: &'a device::Device, gfp_flags: kernel::alloc::Flags) -> Result { Self::zeroed_with_attrs(dev, gfp_flags, Attrs(0)) } } -impl Deref for CoherentBox { +impl Deref for CoherentBox<'_, T> { type Target = T; #[inline] @@ -554,7 +553,7 @@ impl Deref for CoherentBox { } } -impl DerefMut for CoherentBox { +impl DerefMut for CoherentBox<'_, T> { #[inline] fn deref_mut(&mut self) -> &mut Self::Target { // SAFETY: @@ -565,9 +564,9 @@ impl DerefMut for CoherentBox { } } -impl From> for Coherent { +impl<'a, T: AsBytes + FromBytes + KnownSize + ?Sized> From> for Coherent<'a, T> { #[inline] - fn from(value: CoherentBox) -> Self { + fn from(value: CoherentBox<'a, T>) -> Self { value.0 } } @@ -588,26 +587,20 @@ impl From> for Coher /// to an allocated region of coherent memory and `dma_addr` is the DMA address base of the /// region. /// - The size in bytes of the allocation is equal to size information via pointer. -// TODO // -// DMA allocations potentially carry device resources (e.g.IOMMU mappings), hence for soundness -// reasons DMA allocation would need to be embedded in a `Devres` container, in order to ensure -// that device resources can never survive device unbind. -// -// However, it is neither desirable nor necessary to protect the allocated memory of the DMA -// allocation from surviving device unbind; it would require RCU read side critical sections to -// access the memory, which may require subsequent unnecessary copies. -// -// Hence, find a way to revoke the device resources of a `Coherent`, but not the -// entire `Coherent` including the allocated memory itself. -pub struct Coherent { - dev: ARef, +// The lifetime parameter ties DMA allocations to the device's bound scope, ensuring they are freed +// before the device is unbound under normal circumstances. However, if a `Coherent` is leaked (e.g. +// via `mem::forget`), device resources such as IOMMU mappings will not be released. Making all +// constructors `unsafe` to prevent this is considered too restrictive for the common case; this +// soundness hole is accepted for now. +pub struct Coherent<'a, T: KnownSize + ?Sized> { + dev: &'a device::Device, dma_addr: DmaAddress, cpu_addr: NonNull, dma_attrs: Attrs, } -impl Coherent { +impl Coherent<'_, T> { /// Returns the size in bytes of this allocation. #[inline] pub fn size(&self) -> usize { @@ -663,10 +656,10 @@ impl Coherent { } } -impl Coherent { +impl<'a, T: AsBytes + FromBytes> Coherent<'a, T> { /// Allocates a region of `T` of coherent memory. fn alloc_with_attrs( - dev: &device::Device, + dev: &'a device::Device, gfp_flags: kernel::alloc::Flags, dma_attrs: Attrs, ) -> Result { @@ -692,9 +685,9 @@ impl Coherent { // INVARIANT: // - We just successfully allocated a coherent region which is adequately sized for `T`, // hence the cpu address is valid. - // - We also hold a refcounted reference to the device. + // - `dev` is a valid reference to a bound device that outlives this allocation. Ok(Self { - dev: dev.into(), + dev, dma_addr, cpu_addr, dma_attrs, @@ -716,13 +709,13 @@ impl Coherent { /// }; /// /// # fn test(dev: &Device) -> Result { - /// let c: Coherent<[u64; 4]> = + /// let c: Coherent<'_, [u64; 4]> = /// Coherent::zeroed_with_attrs(dev, GFP_KERNEL, DMA_ATTR_NO_WARN)?; /// # Ok::<(), Error>(()) } /// ``` #[inline] pub fn zeroed_with_attrs( - dev: &device::Device, + dev: &'a device::Device, gfp_flags: kernel::alloc::Flags, dma_attrs: Attrs, ) -> Result { @@ -732,14 +725,14 @@ impl Coherent { /// Performs the same functionality as [`Coherent::zeroed_with_attrs`], except the /// `dma_attrs` is 0 by default. #[inline] - pub fn zeroed(dev: &device::Device, gfp_flags: kernel::alloc::Flags) -> Result { + pub fn zeroed(dev: &'a device::Device, gfp_flags: kernel::alloc::Flags) -> Result { Self::zeroed_with_attrs(dev, gfp_flags, Attrs(0)) } /// Same as [`Coherent::zeroed_with_attrs`], but instead of a zero-initialization the memory is /// initialized with `init`. pub fn init_with_attrs( - dev: &device::Device, + dev: &'a device::Device, gfp_flags: kernel::alloc::Flags, dma_attrs: Attrs, init: impl Init, @@ -764,7 +757,7 @@ impl Coherent { /// with `init`. #[inline] pub fn init( - dev: &device::Device, + dev: &'a device::Device, gfp_flags: kernel::alloc::Flags, init: impl Init, ) -> Result @@ -776,11 +769,11 @@ impl Coherent { /// Allocates a region of `[T; len]` of coherent memory. fn alloc_slice_with_attrs( - dev: &device::Device, + dev: &'a device::Device, len: usize, gfp_flags: kernel::alloc::Flags, dma_attrs: Attrs, - ) -> Result> { + ) -> Result> { const { assert!( core::mem::size_of::() > 0, @@ -809,9 +802,9 @@ impl Coherent { // INVARIANT: // - We just successfully allocated a coherent region which is adequately sized for // `[T; len]`, hence the cpu address is valid. - // - We also hold a refcounted reference to the device. + // - `dev` is a valid reference to a bound device that outlives this allocation. Ok(Coherent { - dev: dev.into(), + dev, dma_addr, cpu_addr, dma_attrs, @@ -836,17 +829,17 @@ impl Coherent { /// }; /// /// # fn test(dev: &Device) -> Result { - /// let c: Coherent<[u64]> = + /// let c: Coherent<'_, [u64]> = /// Coherent::zeroed_slice_with_attrs(dev, 4, GFP_KERNEL, DMA_ATTR_NO_WARN)?; /// # Ok::<(), Error>(()) } /// ``` #[inline] pub fn zeroed_slice_with_attrs( - dev: &device::Device, + dev: &'a device::Device, len: usize, gfp_flags: kernel::alloc::Flags, dma_attrs: Attrs, - ) -> Result> { + ) -> Result> { Coherent::alloc_slice_with_attrs(dev, len, gfp_flags | __GFP_ZERO, dma_attrs) } @@ -854,10 +847,10 @@ impl Coherent { /// `dma_attrs` is 0 by default. #[inline] pub fn zeroed_slice( - dev: &device::Device, + dev: &'a device::Device, len: usize, gfp_flags: kernel::alloc::Flags, - ) -> Result> { + ) -> Result> { Self::zeroed_slice_with_attrs(dev, len, gfp_flags, Attrs(0)) } @@ -876,18 +869,18 @@ impl Coherent { /// # fn test(dev: &Device) -> Result { /// let data = [0u8, 1u8, 2u8, 3u8]; /// // `c` has the same content as `data`. - /// let c: Coherent<[u8]> = + /// let c: Coherent<'_, [u8]> = /// Coherent::from_slice_with_attrs(dev, &data, GFP_KERNEL, DMA_ATTR_NO_WARN)?; /// /// # Ok::<(), Error>(()) } /// ``` #[inline] pub fn from_slice_with_attrs( - dev: &device::Device, + dev: &'a device::Device, data: &[T], gfp_flags: kernel::alloc::Flags, dma_attrs: Attrs, - ) -> Result> + ) -> Result> where T: Copy, { @@ -898,10 +891,10 @@ impl Coherent { /// `dma_attrs` is 0 by default. #[inline] pub fn from_slice( - dev: &device::Device, + dev: &'a device::Device, data: &[T], gfp_flags: kernel::alloc::Flags, - ) -> Result> + ) -> Result> where T: Copy, { @@ -909,7 +902,7 @@ impl Coherent { } } -impl Coherent<[T]> { +impl Coherent<'_, [T]> { /// Returns the number of elements `T` in this allocation. /// /// Note that this is not the size of the allocation in bytes, which is provided by @@ -922,10 +915,10 @@ impl Coherent<[T]> { } /// Note that the device configured to do DMA must be halted before this object is dropped. -impl Drop for Coherent { +impl Drop for Coherent<'_, T> { fn drop(&mut self) { let size = T::size(self.cpu_addr.as_ptr()); - // SAFETY: Device pointer is guaranteed as valid by the type invariant on `Device`. + // SAFETY: Device pointer is guaranteed as valid by the lifetime of this `Coherent`. // The cpu address, and the dma address are valid due to the type invariants on // `Coherent`. unsafe { @@ -942,15 +935,15 @@ impl Drop for Coherent { // SAFETY: It is safe to send a `Coherent` to another thread if `T` // can be sent to another thread. -unsafe impl Send for Coherent {} +unsafe impl Send for Coherent<'_, T> {} // SAFETY: Sharing `&Coherent` across threads is safe if `T` is `Sync`, because all // methods that access the buffer contents (`field_read`, `field_write`, `as_slice`, // `as_slice_mut`) are `unsafe`, and callers are responsible for ensuring no data races occur. // The safe methods only return metadata or raw pointers whose use requires `unsafe`. -unsafe impl Sync for Coherent {} +unsafe impl Sync for Coherent<'_, T> {} -impl debugfs::BinaryWriter for Coherent { +impl debugfs::BinaryWriter for Coherent<'_, T> { fn write_to_slice( &self, writer: &mut UserSliceWriter, @@ -1236,7 +1229,7 @@ impl<'a, T: ?Sized + KnownSize> IoBase<'a> for CoherentView<'a, T> { } } -impl<'a, T: ?Sized + KnownSize> IoBase<'a> for &'a Coherent { +impl<'a, T: ?Sized + KnownSize> IoBase<'a> for &'a Coherent<'_, T> { type Backend = CoherentIoBackend; type Target = T; diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs index 5f6c4d7a1a51..f09078228c53 100644 --- a/rust/kernel/uaccess.rs +++ b/rust/kernel/uaccess.rs @@ -520,14 +520,14 @@ impl UserSliceWriter { /// /// fn copy_dma_to_user( /// mut writer: UserSliceWriter, - /// alloc: &Coherent<[u8]>, + /// alloc: &Coherent<'_, [u8]>, /// ) -> Result { /// writer.write_dma(alloc, 0, 256) /// } /// ``` pub fn write_dma( &mut self, - alloc: &Coherent, + alloc: &Coherent<'_, T>, offset: usize, count: usize, ) -> Result { -- cgit v1.2.3