linux-kernel - Re: [PATCH v5 4/4] rust: samples: Add debugfs sample

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: <aCUWvtERff_tk_7o@pollux>
Date: Thu, 15 May 2025 00:18:38 +0200
From: Danilo Krummrich <dakr@...nel.org>
To: Matthew Maurer <mmaurer@...gle.com>
Cc: Benno Lossin <lossin@...nel.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>,
	Benno Lossin <benno.lossin@...ton.me>,
	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>, linux-kernel@...r.kernel.org,
	rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v5 4/4] rust: samples: Add debugfs sample

On Wed, May 14, 2025 at 02:55:02PM -0700, Matthew Maurer wrote:
> On Wed, May 14, 2025 at 2:07 AM Danilo Krummrich <dakr@...nel.org> wrote:
> >
> > On Wed, May 14, 2025 at 09:20:49AM +0200, Benno Lossin wrote:
> > > On Tue May 6, 2025 at 1:51 AM CEST, Matthew Maurer wrote:
> > > > +impl kernel::Module for RustDebugFs {
> > > > +    fn init(_this: &'static ThisModule) -> Result<Self> {
> > > > +        // Create a debugfs directory in the root of the filesystem called "sample_debugfs".
> > > > +        let debugfs = Dir::new(c_str!("sample_debugfs"));
> > > > +
> > > > +        {
> > > > +            // Create a subdirectory, so "sample_debugfs/subdir" now exists.
> > > > +            // We wrap it in `ManuallyDrop` so that the subdirectory is not automatically discarded
> > > > +            // at the end of the scope - it will be cleaned up when `debugfs` is.
> > > > +            let sub = ManuallyDrop::new(debugfs.subdir(c_str!("subdir")));
> > >
> > > I dislike the direct usage of `ManuallyDrop`. To me the usage of
> > > `ManuallyDrop` signifies that one has to opt out of `Drop` without the
> > > support of the wrapped type. But in this case, `Dir` is sometimes
> > > intended to not be dropped, so I'd rather have a `.keep()` function I
> > > saw mentioned somewhere.
> >
> > I agree, if we really want to "officially" support to forget() (sub-)directories
> > and files in order to rely on the recursive cleanup of the "root" directory, it
> > should be covered explicitly by the API. I.e. (sub-)directories and files should
> > have some kind of keep() and / or forget() method, to make it clear that this is
> > supported and by design and won't lead to any leaks.
> 
> OK, I can do this with `.keep()` just being equivalent to
> `ManuallyDrop::new`. Since we now only have "by default, things are
> dropped", rather than two states, we shouldn't need a `.destroy()`
> method.
> 
> >
> > Consequently, this would mean that we'd need something like your proposed const
> > generic on the Dir type, such that keep() / forget() cannot be called on the
> > "root" directory.
> 
> Just to ensure I understand correctly, your opinion is that on
> balance, additional complexity in the API types are worth making it
> less ergonomic to suppress drop on a root directory, keeping in mind
> that they still *can* suppress drop on that directory. You don't want
> the extra API variants to do anything else special other than change
> the ergonomics of `.keep()` (e.g. don't have subdirectories be default
> pre-`.keep()`'d).

Absolutely, yes! Having the keep() method available for "root" directories is
clearly a footgun, because it suggests that it is a valid thing to call for a
"root" directory, but actually is almost always a bug.

What do we have a powerful type system for if we don't take advantage of it to
prevent bugs? :-)