lkml.org 
[lkml]   [2025]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v5 4/4] rust: samples: Add debugfs sample
From
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

\
 
 \ /
  Last update: 2025-05-22 19:41    [from the cache]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and my Meterkast|Read the blog