linux-kernel - Re: [PATCH v13 2/7] 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] [day] [month] [year] [list]

Message-ID: <Z927D6V4SqAZ_Hcg@cassiopeiae>
Date: Fri, 21 Mar 2025 20:16:31 +0100
From: Danilo Krummrich <dakr@...nel.org>
To: Jason Gunthorpe <jgg@...pe.ca>
Cc: Abdiel Janulgue <abdiel.janulgue@...il.com>,
	rust-for-linux@...r.kernel.org, daniel.almeida@...labora.com,
	robin.murphy@....com, aliceryhl@...gle.com,
	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 v13 2/7] rust: add dma coherent allocator abstraction.

On Fri, Mar 21, 2025 at 03:29:01PM -0300, Jason Gunthorpe wrote:
> On Fri, Mar 21, 2025 at 06:34:53PM +0100, Danilo Krummrich wrote:
> > On Fri, Mar 21, 2025 at 02:23:53PM -0300, Jason Gunthorpe wrote:
> > > On Fri, Mar 07, 2025 at 01:06:19PM +0200, Abdiel Janulgue wrote:
> > > 
> > > > +        // SAFETY: Device pointer is guaranteed as valid by the type invariant on `Device`.
> > > > +        let ret = unsafe {
> > > > +            bindings::dma_alloc_attrs(
> > > > +                dev.as_raw(),
> > > > +                size,
> > > > +                &mut dma_handle,
> > > > +                gfp_flags.as_raw(),
> > > > +                dma_attrs.as_raw(),
> > > > +            )
> > > 
> > > This is not the correct safety statement, the device must have a driver
> > > bound to call this function, a struct device reference is not
> > > sufficient.
> > > 
> > > I belive Danilo was suggesting to ignore this unsafety for now, but if
> > > so it should be documented correctly.
> > 
> > If just landed patches [1], which are the foundation of addressing this issue.
> 
> Those patches say:
> 
>  The context types can be extended as required, e.g. to limit availability  of
>  certain (bus) device functions to probe().
> 
> Which is not an appropriate limitation for dma_alloc_coherent, we
> expect it to be called outside probe in real drivers. Is there more to
> that story?

Yeah, we can also use them to derive specifically typed Device instances from
other entry points of the driver where we know for sure that at this point the
device must (still) be bound to the driver.

For instance, bus callbacks, subsystem callbacks, certain (but not all) IOCTLs,
IRQ handlers, etc.

All those cases can be covered by only the type system, without additional
locks. We could even use this as an optimization to bypass Devres'
try_access() calls when holding a corresponding device instance in those places.

> 
> Regardless, the safety comment should not be merged with incorrect
> information. :\

v15 did land in rust-next, so unfortunately this was overlooked. Since you
caught, mind sending a patch improving the comment?