rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Danilo Krummrich <dakr@kernel.org>
To: Alice Ryhl <aliceryhl@google.com>
Cc: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Benno Lossin" <lossin@kernel.org>,
	"Matthew Maurer" <mmaurer@google.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <benno.lossin@proton.me>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Sami Tolvanen" <samitolvanen@google.com>,
	"Timur Tabi" <ttabi@nvidia.com>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org
Subject: Re: [PATCH v5 4/4] rust: samples: Add debugfs sample
Date: Sat, 24 May 2025 14:25:27 +0200	[thread overview]
Message-ID: <aDG6t-HdZSuWyPGo@pollux.localdomain> (raw)
In-Reply-To: <aDCrufEjtJeGjO6z@google.com>

On Fri, May 23, 2025 at 05:09:13PM +0000, Alice Ryhl wrote:
> On Fri, May 23, 2025 at 11:42:23AM +0200, Danilo Krummrich wrote:
> > The only thing I don't want to is to allow to leak files or directories, i.e.
> > 
> > 	File::new("bar_file", bar_dir).leak()
> > 
> > which keeps the file in the filesystem, but takes away the handle from the Rust
> > side, such that it won't be removed from the filesystem anymore when the handle
> > goes out of scope, which can cause nasty bugs. But I think there isn't any
> > benefit to allow this anyways and it isn't needed with reference counting.
> 
> Why not have leak() for files only? That way, the files still go away
> when you drop the directory, and you don't have to keep around a bunch
> of File objects in your Rust structs.

In a previous mail I said that there are more reasons why we don't want to have
files (or directories) in the filesystem without an object representation in
Rust.

One of them is when attaching private data to a file, like we do with
debugfs_create_file().

When we do this, in most of the cases we want to share data between a driver
structure and the debugfs file (e.g. a GPU's VM to dump the virtual address
space layout).

And most likely we do this by passing an Arc<T> to dir.file(), to make the file
own a reference to the corresponding reference counted object (in C we just hope
that no one frees the object while the debugfs file holds a pointer to it).

The question is, what happens to this reference if we would subsequently call
file.leak()? We can't drop the Arc, because otherwise we risk a UAF in the
filesystem callback, but we can't keep it either because otherwise we *really*
leak this reference, even if the parent directory is removed eventually.

--

Now, one could say, what about attaching the file's private data to the directory
instead? (And I think this has been proposed already elsewhere.)

But I really don't like this approach, because it would mean that when creating
the directory we would necessarily need to know all the files we will ever
create in this directory *and* have all the corresponding private data
available at this point of time. But that would be an insane requirement.

For instance, let's assume a driver creates a directory 'clients', which for
every connected userspace "client" wants to provide a file with some metadata
for this client.

Sure, you can work around this somehow, e.g. by using a mutex protected Vec as
private data or by always creating a new directory first for dynamically created
debugfs entires. But this sounds pretty horrible to me.

  reply	other threads:[~2025-05-24 12:25 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-05 23:51 [PATCH v5 0/4] rust: DebugFS Bindings Matthew Maurer
2025-05-05 23:51 ` [PATCH v5 1/4] rust: debugfs: Bind DebugFS directory creation Matthew Maurer
2025-05-07 18:46   ` Timur Tabi
2025-05-14 22:26     ` Matthew Maurer
2025-05-14  7:33   ` Benno Lossin
2025-05-14  8:49     ` Greg Kroah-Hartman
2025-05-14  9:38       ` Benno Lossin
2025-05-05 23:51 ` [PATCH v5 2/4] rust: debugfs: Bind file creation for long-lived Display Matthew Maurer
2025-05-07 19:04   ` Timur Tabi
2025-05-07 19:41   ` Timur Tabi
2025-05-09 12:56   ` Alice Ryhl
2025-05-12 20:51   ` Timur Tabi
2025-05-14  8:06   ` Benno Lossin
2025-05-05 23:51 ` [PATCH v5 3/4] rust: debugfs: Support format hooks Matthew Maurer
2025-05-05 23:51 ` [PATCH v5 4/4] rust: samples: Add debugfs sample Matthew Maurer
2025-05-14  7:20   ` Benno Lossin
2025-05-14  9:07     ` Danilo Krummrich
2025-05-14  9:54       ` Benno Lossin
2025-05-14 11:24         ` Danilo Krummrich
2025-05-14 12:21           ` Benno Lossin
2025-05-14 13:04             ` Danilo Krummrich
2025-05-14 22:14           ` Matthew Maurer
2025-05-14 22:08         ` Matthew Maurer
2025-05-14 22:14           ` Danilo Krummrich
2025-05-14 22:23             ` Matthew Maurer
2025-05-14 22:32               ` Matthew Maurer
2025-05-14 22:40                 ` Timur Tabi
2025-05-14 22:42                   ` Matthew Maurer
2025-05-15  7:43                     ` gregkh
2025-05-15  8:50           ` Benno Lossin
2025-05-14 21:55       ` Matthew Maurer
2025-05-14 22:18         ` Danilo Krummrich
2025-05-15  8:59         ` Benno Lossin
2025-05-15 11:43           ` Greg Kroah-Hartman
2025-05-15 12:37             ` Danilo Krummrich
2025-05-15 12:55               ` Benno Lossin
2025-05-20 21:24             ` Alice Ryhl
2025-05-21  4:47               ` Greg Kroah-Hartman
2025-05-21 22:40                 ` Alice Ryhl
2025-05-21  7:57               ` Danilo Krummrich
2025-05-21 22:43                 ` Alice Ryhl
2025-05-22  6:25                   ` Danilo Krummrich
2025-05-22  8:28                     ` Greg Kroah-Hartman
2025-05-22 14:01                     ` Alice Ryhl
2025-05-22 14:15                       ` Greg Kroah-Hartman
2025-05-22 17:40                         ` Alice Ryhl
2025-05-22 20:26                           ` Benno Lossin
2025-05-23  9:15                           ` Greg Kroah-Hartman
2025-05-22 17:53                         ` Danilo Krummrich
2025-05-23  9:14                           ` Greg Kroah-Hartman
2025-05-23  9:42                             ` Danilo Krummrich
2025-05-23 10:22                               ` Greg Kroah-Hartman
2025-05-23 17:09                               ` Alice Ryhl
2025-05-24 12:25                                 ` Danilo Krummrich [this message]
2025-05-27 11:38                                   ` Alice Ryhl
2025-05-27 11:50                                     ` Danilo Krummrich
2025-06-10 17:54                                       ` Matthew Maurer
2025-05-23 17:06                             ` Alice Ryhl
2025-05-07 16:49 ` [PATCH v5 0/4] rust: DebugFS Bindings Danilo Krummrich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aDG6t-HdZSuWyPGo@pollux.localdomain \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=mmaurer@google.com \
    --cc=ojeda@kernel.org \
    --cc=rafael@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=samitolvanen@google.com \
    --cc=tmgross@umich.edu \
    --cc=ttabi@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).