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: <3c928a0e-ccd4-4ba0-9f42-9f2bb0203e75@kernel.org>
Date: Tue, 1 Jul 2025 21:58:52 +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 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:
>>> On Tue, Jul 1, 2025 at 8:10 AM Danilo Krummrich <dakr@...nel.org> wrote:
>>>>          impl Firmware {
>>>>             pub fn new(&dir: debugfs::Dir, buffer: [u8]) -> impl PinInit<Self> {
>>>>                pin_init!(Self {
>>>>                   minor <- dir.create_file("minor", 1),
>>>>                   major <- dir.create_file("major", 2),
>>>>                   buffer <- dir.create_file("buffer", buffer),
>>>>                })
>>>>             }
>>>>          }
>>>>
>>>>          // This is the only allocation we need.
>>>>          let fw = KBox::pin_init(Firmware::new(...), GFP_KERNEL)?;
>>>>
>>>> With this everything is now in a single allocation and since we're using
>>>> pin-init, Dir::create_file() can safely store pointers of the corresponding data
>>>> in debugfs_create_file(), since this structure is guaranteed to be pinned in
>>>> memory.
>>>>
>>>> Actually, we can also implement *only this*, since with this my previous example
>>>> would just become this:
>>>
>>> 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. :)