linux-kernel - Re: [PATCH v12 2/3] rust: add dma coherent allocator abstraction.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAH5fLggU_sU56dEz6hQvMwjY=G2TuC9GYiVEBZFtE=Tk2QW4Fw@mail.gmail.com>
Date: Mon, 24 Feb 2025 14:21:47 +0100
From: Alice Ryhl <aliceryhl@...gle.com>
To: Abdiel Janulgue <abdiel.janulgue@...il.com>
Cc: dakr@...nel.org, robin.murphy@....com, daniel.almeida@...labora.com, 
	rust-for-linux@...r.kernel.org, Miguel Ojeda <ojeda@...nel.org>, 
	Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>, 
	Gary Guo <gary@...yguo.net>, Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Trevor Gross <tmgross@...ch.edu>, Valentin Obst <kernel@...entinobst.de>, 
	open list <linux-kernel@...r.kernel.org>, Christoph Hellwig <hch@....de>, 
	Marek Szyprowski <m.szyprowski@...sung.com>, airlied@...hat.com, 
	"open list:DMA MAPPING HELPERS" <iommu@...ts.linux.dev>
Subject: Re: [PATCH v12 2/3] rust: add dma coherent allocator abstraction.

On Mon, Feb 24, 2025 at 12:50 PM Abdiel Janulgue
<abdiel.janulgue@...il.com> wrote:
>
> Add a simple dma coherent allocator rust abstraction. Based on
> Andreas Hindborg's dma abstractions from the rnvme driver, which
> was also based on earlier work by Wedson Almeida Filho.
>
> Nacked-by: Christoph Hellwig <hch@....de>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@...il.com>

> +    /// Create a duplicate of the `CoherentAllocation` object but prevent it from being dropped.
> +    pub fn skip_drop(self) -> CoherentAllocation<T> {
> +        let me = core::mem::ManuallyDrop::new(self);
> +        Self {
> +            // SAFETY: The refcount of `dev` will not be decremented because this doesn't actually
> +            // duplicafe `ARef` and the use of `ManuallyDrop` forgets the originals.
> +            dev: unsafe { core::ptr::read(&me.dev) },
> +            dma_handle: me.dma_handle,
> +            count: me.count,
> +            cpu_addr: me.cpu_addr,
> +            dma_attrs: me.dma_attrs,
> +        }
> +    }

The skip_drop pattern requires the return value to use a different
struct with the same fields, because otherwise you don't really skip
the destructor. But I don't think you have the user for this method
anymore so maybe just drop it.

> +    /// Retrieve a single entry from the region with bounds checking. `offset` is in units of `T`,
> +    /// not the number of bytes.
> +    pub fn item_from_index(&self, offset: usize) -> Result<*mut T> {
> +        if offset >= self.count {
> +            return Err(EINVAL);
> +        }
> +        // SAFETY:
> +        // - The pointer is valid due to type invariant on `CoherentAllocation`
> +        // and we've just checked that the range and index is within bounds.
> +        // - `offset` can't overflow since it is smaller than `self.count` and we've checked
> +        // that `self.count` won't overflow early in the constructor.
> +        Ok(unsafe { &mut *self.cpu_addr.add(offset) })

The point of the dma_read/dma_write macros is to avoid creating
references to the dma memory, so don't create a reference here.

Ok(unsafe { self.cpu_addr.add(offset) })

Alice