linux-kernel - Re: [PATCH v8 4/6] rust: debugfs: Support arbitrary owned backing for File

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: <aGRCM1VRHI8EvZmd@pollux>
Date: Tue, 1 Jul 2025 22:16:51 +0200
From: Danilo Krummrich <dakr@...nel.org>
To: Benno Lossin <lossin@...nel.org>
Cc: Matthew Maurer <mmaurer@...gle.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.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>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Sami Tolvanen <samitolvanen@...gle.com>,
	Timur Tabi <ttabi@...dia.com>, linux-kernel@...r.kernel.org,
	rust-for-linux@...r.kernel.org,
	Dirk Behme <dirk.behme@...bosch.com>
Subject: Re: [PATCH v8 4/6] rust: debugfs: Support arbitrary owned backing
 for File

On Tue, Jul 01, 2025 at 10:09:10PM +0200, Benno Lossin wrote:
> On Tue Jul 1, 2025 at 10:03 PM CEST, Benno Lossin wrote:
> > On Tue Jul 1, 2025 at 9:58 PM CEST, Danilo Krummrich wrote:
> >> On 7/1/25 9:46 PM, Benno Lossin wrote:
> >>> On Tue Jul 1, 2025 at 9:21 PM CEST, Danilo Krummrich wrote:
> >>>> On Tue, Jul 01, 2025 at 11:11:13AM -0700, Matthew Maurer wrote:
> >>>>> If we implement *only* pinned files, we run into an additional problem
> >>>>> - you can't easily extend a pinned vector. This means that you cannot
> >>>>> have dynamically created devices unless you're willing to put every
> >>>>> new `File` into its own `Box`, because you aren't allowed to move any
> >>>>> of the previously allocated `File`s for a resize.
> >>>>>
> >>>>> Where previously you would have had
> >>>>>
> >>>>> ```
> >>>>> debug_files: Vec<File>
> >>>>> ```
> >>>>>
> >>>>> you would now have
> >>>>>
> >>>>> ```
> >>>>> debug_files: Vec<PinBox<File<T>>>
> >>>>> ```
> >>>>
> >>>> Stuffing single File instances into a Vec seems like the wrong thing to do.
> >>>>
> >>>> Instead you may have instances of some data structure that is created
> >>>> dynamically in your driver that you want to expose through debugfs.
> >>>>
> >>>> Let's say you have (userspace) clients that can be registered arbitrarily, then
> >>>> you want a Vec<Client>, which contains the client instances. In order to provide
> >>>> information about the Client in debugfs you then have the client embed things as
> >>>> discussed above.
> >>>>
> >>>> 	struct Client {
> >>>> 	   id: File<ClientId>,
> >>>> 	   data: File<ClientData>,
> >>>> 	   ...
> >>>> 	}
> >>>>
> >>>> I think that makes much more sense than keeping a Vec<Arc<Client>> *and* a
> >>>> Vec<File> separately. Also, note that with the above, your Client instances
> >>>> don't need to be reference counted anymore.
> >>>>
> >>>> I think this addresses the concerns below.
> >>> 
> >>> You still have the issue that `Client` now needs to be pinned and the
> >>> vector can't be resized. But if you know that it's bounded, then we
> >>> could just make `Pin<Vec<T>>` work as expected (not relocating the
> >>> underlying allocation by not exposing `push`, only
> >>> `push_within_capacity`).
> >>> 
> >>> We also could have a `SegmentedVec<T>` that doesn't move elements.
> >>> Essentially it is
> >>>      
> >>>      enum SegmentedVec<T> {
> >>>          Cons(Segment<T>, KBox<SegmentedVec<T>>)
> >>>          Nul,
> >>>      }
> >>> 
> >>>      struct Segment<T> {
> >>>          elements: [T; 16]
> >>>      }
> >>> 
> >>> or make the segments variable-sized and grow them accordingly.
> >>
> >> That sounds a lot like the perfect application for XArray. :)
> >
> > Haha I didn't know this already existed in the kernel :) Yeah then we
> > should make XArray work for this use-case.
> 
> Ah wait, I meant for `SegmentedVec<T>` to store multiple `T` in the same
> allocation, so it would only grow sometimes and amortize the allocations
> just like `Vec`. It seems to me that XArray only stores pointers, so you
> have to box everything (which we're trying to avoid IIUC).

Yes, that sounds good. And if the potential number of Client instances can get
pretty large Vec isn't a good choice anyways. If we really get a ton of clients,
they should be allocated with a kmem_cache and stored in an XArray, maple tree,
etc.