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: <aC9hm9D458C6LsRW@google.com>
Date: Thu, 22 May 2025 17:40:43 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Danilo Krummrich <dakr@...nel.org>, Benno Lossin <lossin@...nel.org>, 
	Matthew Maurer <mmaurer@...gle.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>, 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
Subject: Re: [PATCH v5 4/4] rust: samples: Add debugfs sample

On Thu, May 22, 2025 at 04:15:46PM +0200, Greg Kroah-Hartman wrote:
> > > Well, take the case I described above, where the debugfs "root" is created in
> > > the module scope, but subsequent entries are created by driver instances. If a
> > > driver would use keep() in such a case, we'd effectively the file / directory
> > > (and subsequently also the corresponding memory) everytime a device is unplugged
> > > (or unbound in general)."
> > > 
> > > If the module is built-in the directory from the module scope is *never*
> > > removed, but the entries the driver e.g. creates in probe() for a particular
> > > device with keep() will pile up endlessly, especially for hot-pluggable devices.
> > > 
> > > (It's getting even worse when there's data bound to such a leaked file, that
> > > might even contain a vtable that is entered from any of the fops of the file.)
> > > 
> > > That'd be clearly a bug, but for the driver author calling keep() seems like a
> > > valid thing to do -- to me that's clearly a built-in footgun.
> > 
> > I mean, for cases such as this, I could imagine that you use `keep()` on
> > the files stored inside of the driver directory, but don't use it on the
> > directory. That way, you only have to keep a single reference to an
> > entire directory around, which may be more convenient.
> 
> No, sorry, but debugfs files are "create and forget" type of things.
> The caller has NO reference back to the file at all in the C version,
> let's not add that functionality back to the rust side after I spent a
> long time removing it from the C code :)
> 
> If you really want to delete a debugfs file that you have created in the
> past, then look it up and delete it with the call that is present for
> that.
> 
> The only thing I think that might be worth "keeping" in some form, as an
> object reference as discussed, is a debugfs directory.

That could work if we don't have any Rust value for files at all. The
problem is that if we do have such values, then code like this:

let my_file = dir.create_file("my_file_name");
dir.delete_file("my_file_name");
my_file.do_something();

would be a UAF on the last line. We have to design the Rust API to avoid
such UAF, which is why I suggested the ghost objects; the delete_file()
call leaves my_file in a valid but useless state. And as a ghost object,
the .do_something() call becomes a no-op since the file is now missing
from the filesystem.

Alice