linux-kernel - Re: [PATCH 1/2] rust: add initial scatterlist bindings

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20250530141419.GA292183@ziepe.ca>
Date: Fri, 30 May 2025 11:14:19 -0300
From: Jason Gunthorpe <jgg@...pe.ca>
To: Alexandre Courbot <acourbot@...dia.com>
Cc: Abdiel Janulgue <abdiel.janulgue@...il.com>, dakr@...nel.org,
	lyude@...hat.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>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	Valentin Obst <kernel@...entinobst.de>,
	open list <linux-kernel@...r.kernel.org>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Robin Murphy <robin.murphy@....com>, airlied@...hat.com,
	rust-for-linux@...r.kernel.org,
	"open list:DMA MAPPING HELPERS" <iommu@...ts.linux.dev>,
	Petr Tesarik <petr@...arici.cz>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Sui Jingfeng <sui.jingfeng@...ux.dev>,
	Randy Dunlap <rdunlap@...radead.org>,
	Michael Kelley <mhklinux@...look.com>
Subject: Re: [PATCH 1/2] rust: add initial scatterlist bindings

On Fri, May 30, 2025 at 11:02:02PM +0900, Alexandre Courbot wrote:
> You would have a trait for providing the pages and their range:
> 
>     /// Provides a list of pages that can be used to build a `SGTable`.
>     trait SGTablePages {
>         /// Returns an iterator to the pages providing the backing memory of `self`.
>         fn pages_iter<'a>(&'a self) -> impl Iterator<Item = &'a bindings::page>;
>         /// Returns the effective range of the mapping.
>         fn range(&self) -> Range<usize>;
>     }
>
> The `SGTable` becomes something like:
> 
>     struct SGTable<P: SGTablePages, T: MapState>
>     {
>         table: Opaque<bindings::sg_table>,
>         pages: P,
>         _s: PhantomData<T>,
>     }

At this point it isn't exactly a sgtable anymore, it is some rust
specific way to get a dma mapped scatterlist. Most of the actual ways
to use a sgtable's cpu side would become unavailable for safety
reasons.

That seems fine to me, and is what I was suggesting when I said not to
expose set_page at all.

But I would maybe lean into it a bit more, why have the type state at
all anymore if the flow is SGTablePages -> SgTable -> Dma Mapped?
There isn't really a reason to expose the CPU populated but not yet
mapped state to the user at all. They can't do anything with it. Just
directly create the DMA mapped scatterlist and only expose the DMA
list through the rust API in a single step.

So much simpler to understand and doesn't leak the bad decisions of
the scatterlist design.

Certainly the initial uses of scatterlist don't need to ever know
about or touch the CPU side of the scatterlist, and it would be great
if Rust could stay that way..

Jason