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: <CAGSQo02hyJncD1oTpUMgiSZeX5UYYY2p-WZTyroQJJ6fMnOrCQ@mail.gmail.com>
Date: Mon, 30 Jun 2025 10:49:51 -0700
From: Matthew Maurer <mmaurer@...gle.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: 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>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	"Rafael J. Wysocki" <rafael@...nel.org>, Sami Tolvanen <samitolvanen@...gle.com>, 
	Timur Tabi <ttabi@...dia.com>, Benno Lossin <lossin@...nel.org>, 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 Mon, Jun 30, 2025 at 10:39 AM Danilo Krummrich <dakr@...nel.org> wrote:
>
> On 6/30/25 7:34 PM, Matthew Maurer wrote:
> > On Mon, Jun 30, 2025 at 10:30 AM Danilo Krummrich <dakr@...nel.org> wrote:
> >>
> >> On 6/28/25 1:18 AM, Matthew Maurer wrote:
> >>> +    fn create_file<D: ForeignOwnable>(&self, _name: &CStr, data: D) -> File
> >>> +    where
> >>> +        for<'a> D::Borrowed<'a>: Display,
> >>> +    {
> >>> +        File {
> >>> +            _foreign: ForeignHolder::new(data),
> >>> +        }
> >>>        }
> >>
> >> What's the motivation for the ForeignHolder abstraction? Why not just make it
> >> File<D> and store data directly?
> >
> > 1. A `File<D>` can't be held in collection data structures as easily
> > unless all your files contain the *same* backing type.
>
> That sounds reasonable.
>
> > 2. None of the APIs or potential APIs for `File` care about which type
> > it's wrapping, nor are they supposed to. If nothing you can do with a
> > `File` is different depending on the backing type, making it
> > polymorphic is just needlessly confusing.
>
> What if I want to access file.data() and do something with the data? Then I'd
> necessarily need to put my data in an Arc and reference count it to still be
> able to access it.
>
> That doesn't seem like a reasonable requirement to be able to access data
> exposed via debugfs.

`pub fn data(&self) -> D` would go against my understanding of Greg's
request for DebugFS files to not really support anything other than
delete. I was even considering making `D` not be retained in the
disabled debugfs case, but left it in for now for so that the
lifecycles wouldn't change.

If you want a `.data()` function, I can add it in, but I don't think
it'll improve flexibility in most cases. If you want to do something
with the data and it's not in an `Arc` / behind a handle of some kind,
you'll need something providing threadsafe interior mutability in the
data structure. If that's a lock, then I have a hard time believing
that `Arc<Mutex<T>>`(or if it's a global, a `&'static Mutex<T>`, which
is why I added that in the stack) is so much more expensive than
`Box<Mutex<T>>` that it's worth a more complex API. If it's an atomic,
e.g. `Arc<AtomicU8>`, then I can see the benefit to having
`Box<AtomicU8>` over that, but it still seems so slim that I think the
simpler "`File` is just a handle to how long the file stays alive, it
doesn't let you do anything else" API makes sense.