Messages in this thread |  | | Date | Thu, 22 May 2025 17:40:43 +0000 | | Subject | Re: [PATCH v5 4/4] rust: samples: Add debugfs sample | | From | Alice Ryhl <> |
| |
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
|  |