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

[Alice Ryhl <aliceryhl@google.com>]

On Wed, May 21, 2025 at 09:57:29AM +0200, Danilo Krummrich wrote:
> On Tue, May 20, 2025 at 09:24:21PM +0000, Alice Ryhl wrote:
> > On Thu, May 15, 2025 at 01:43:09PM +0200, Greg Kroah-Hartman wrote:
> > > On Thu, May 15, 2025 at 10:59:44AM +0200, Benno Lossin wrote:
> > > > Is there no refcount that we can use instead of borrowing? I guess not,
> > > > since one can call `debugfs_remove`. What about a refcount on the rust
> > > > side? or is debugfs not used for "debugging" and needs to have the
> > > > performance of no refcount?
> > > 
> > > debugfs should never have any performance issues (i.e. you don't use it
> > > for performant things.)
> > > 
> > > So refcount away!  That should never be an issue.
> > 
> > What I imagine would be the ideal API for Rust is the following:
> > 
> > * For each file / dir you create, you get a Rust object that owns it.
> > 
> > * When you destroy one of these Rust objects, it disappears from the
> >   file system. I.e., dropping a directory removes things recursively.
> > 
> > * If you remove a directory before the removing objects inside it, then
> >   the Rust objects become "ghost" objects that are still usable, but not
> >   visible in the file system anywhere. I.e. calling methods on them
> >   succeed but are no-ops.
> 
> If we would want to keep an entry alive as long as there are more leaves, we'd
> obviously need to reference count things.
> 
> But what do we need reference counting for with this logic? Shouldn't this be
> also possible without?

Well, my understanding is that when you remove the parent directory, the
dentries for child directories and files are freed, so trying to use the
Rust objects that correspond to those child dentries would be a UAF. I
want to refcount those child entries so that they at least remain valid
memory even if they're no longer visible in the file system.

> > * Possibly we have a way to drop a Rust object without removing it from
> >   the file system. In this case, it can never be accessed from Rust
> >   again, and the only way to remove it is to drop its parent directory.
> 
> I'm not sure about this one.
> 
> IIUC, this basically brings back the "keep() logic", which I still think is a
> footgun and should be avoided.
> 
> Also, we only needed this, since due to the borrowing design we couldn't store
> parent and child nodes in the same structure. With reference counting (or the
> logic above) this goes away.
> 
> I wrote the following in a previous conversation [1].
> 
> --
> 
> What I see more likely to happen is a situation where the "root" directory
> (almost) lives forever, and hence subsequent calls, such as
> 
> 	root.subdir("foo").keep()
> 
> effectively are leaks.
> 
> One specific example for that would be usb_debug_root, which is created in the
> module scope of usb-common and is used by USB host / gadget / phy drivers.
> 
> The same is true for other cases 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 leak memory everytime a
> device is unplugged (or unbound in general).

Where is the leak? If the file is still visible in the file system, then
it's not a leak to still have the memory. If the file got removed by
removing its parent, then my intent is that this should free the memory
of the child object.

Alice