Allows creation of files for references that live forever and lack
metadata through the `Display` implementation.
The reference must live forever because we do not have a maximum
lifetime for the file we are creating.
The `Display` implementation is used because `seq_printf` needs to route
through `%pA`, which in turn routes through Arguments. A more generic
API is provided later in the series, implemented in terms of this one.
Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
rust/kernel/debugfs.rs | 134 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 134 insertions(+)
diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index b589c2d9a8d169bd66e98d2894261784e427230e..ef69ae8f550a9fe6b0afc1c51bebaad2fc087811 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -7,6 +7,7 @@
//! C header: [`include/linux/debugfs.h`](srctree/include/linux/debugfs.h)
use crate::str::CStr;
+use core::fmt::Display;
use core::mem::ManuallyDrop;
use core::ops::Deref;
@@ -118,6 +119,48 @@ unsafe fn from_ptr(ptr: *mut bindings::dentry) -> Self {
fn as_ptr(&self) -> *mut bindings::dentry {
self.0
}
+
+ /// Create a file in a DebugFS directory with the provided name, and contents from invoking
+ /// [`Display::fmt`] on the provided reference.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// # use kernel::c_str;
+ /// # use kernel::debugfs::Dir;
+ /// let dir = Dir::new(c_str!("my_debugfs_dir"));
+ /// dir.display_file(c_str!("foo"), &200);
+ /// // "my_debugfs_dir/foo" now contains the number 200.
+ /// ```
+ pub fn display_file<T: Display + Sized>(&self, name: &CStr, data: &'static T) -> File {
+ // SAFETY:
+ // * `name` is a NUL-terminated C string, living across the call, by `CStr` invariant.
+ // * `parent` is a live `dentry` since we have a reference to it.
+ // * `vtable` is all stock `seq_file` implementations except for `open`.
+ // `open`'s only requirement beyond what is provided to all open functions is that the
+ // inode's data pointer must point to a `T` that will outlive it, which we know because
+ // we have a static reference.
+ // * debugfs_create_file_full either returns an error code or a legal dentry pointer, so
+ // `Self::from_ptr` is safe to call here.
+ #[cfg(CONFIG_DEBUG_FS)]
+ let dir = unsafe {
+ Self::from_ptr(bindings::debugfs_create_file_full(
+ name.as_char_ptr(),
+ 0o444,
+ self.as_ptr(),
+ data as *const _ as *mut _,
+ core::ptr::null(),
+ &<T as DisplayFile>::VTABLE,
+ ))
+ };
+ #[cfg(not(CONFIG_DEBUG_FS))]
+ let dir = {
+ // Mark parameters used
+ let (_, _) = (name, data);
+ Self()
+ };
+ File(ManuallyDrop::new(dir))
+ }
}
impl Drop for Dir {
@@ -153,3 +196,94 @@ fn new(dir: Dir) -> Self {
Self(ManuallyDrop::new(dir))
}
}
+/// Handle to a DebugFS file.
+#[repr(transparent)]
+pub struct File(ManuallyDrop<Dir>);
+
+impl File {
+ /// Remove the file from DebugFS.
+ ///
+ /// # Examples
+ /// ```
+ /// # use kernel::c_str;
+ /// # use kernel::debugfs::Dir;
+ /// let dir = Dir::new(c_str!("foo"));
+ /// {
+ /// let file = dir.display_file(c_str!("bar"), &0);
+ /// // "foo/bar" is created.
+ /// }
+ /// // "foo/bar" still exists.
+ /// {
+ /// let file = dir.display_file(c_str!("baz"), &0);
+ /// // "foo/baz" is created.
+ /// file.remove();
+ /// // "foo/baz" is gone.
+ /// }
+ pub fn remove(self) {
+ drop(ManuallyDrop::into_inner(self.0))
+ }
+}
+
+#[cfg(CONFIG_DEBUG_FS)]
+mod helpers {
+ use crate::seq_file::SeqFile;
+ use crate::seq_print;
+ use core::fmt::Display;
+
+ /// Implements `open` for `file_operations` via `single_open` to fill out a `seq_file`.
+ ///
+ /// # Safety
+ ///
+ /// * `inode`'s private pointer must point to a value of type `T` which will outlive the `inode`
+ /// and will not be mutated during this call.
+ /// * `file` must point to a live, not-yet-initialized file object.
+ pub(crate) unsafe extern "C" fn display_open<T: Display>(
+ inode: *mut bindings::inode,
+ file: *mut bindings::file,
+ ) -> i32 {
+ // SAFETY:
+ // * `file` is acceptable by caller precondition.
+ // * `print_act` will be called on a `seq_file` with private data set to the third argument,
+ // so we meet its safety requirements.
+ // * The `data` pointer passed in the third argument is a valid `T` pointer that outlives
+ // this call by caller preconditions.
+ unsafe { bindings::single_open(file, Some(display_act::<T>), (*inode).i_private) }
+ }
+
+ /// Prints private data stashed in a seq_file to that seq file.
+ ///
+ /// # Safety
+ ///
+ /// `seq` must point to a live `seq_file` whose private data is a live pointer to a `T` which is
+ /// not being mutated.
+ pub(crate) unsafe extern "C" fn display_act<T: Display>(
+ seq: *mut bindings::seq_file,
+ _: *mut core::ffi::c_void,
+ ) -> i32 {
+ // SAFETY: By caller precondition, this pointer is live, points to a value of type `T`, and
+ // is not being mutated.
+ let data = unsafe { &*((*seq).private as *mut T) };
+ // SAFETY: By caller precondition, `seq_file` points to a live `seq_file`, so we can lift
+ // it.
+ let seq_file = unsafe { SeqFile::from_raw(seq) };
+ seq_print!(seq_file, "{}", data);
+ 0
+ }
+
+ // Work around lack of generic const items.
+ pub(crate) trait DisplayFile: Display + Sized {
+ const VTABLE: bindings::file_operations = bindings::file_operations {
+ read: Some(bindings::seq_read),
+ llseek: Some(bindings::seq_lseek),
+ release: Some(bindings::single_release),
+ open: Some(display_open::<Self> as _),
+ // SAFETY: `file_operations` supports zeroes in all fields.
+ ..unsafe { core::mem::zeroed() }
+ };
+ }
+
+ impl<T: Display + Sized> DisplayFile for T {}
+}
+
+#[cfg(CONFIG_DEBUG_FS)]
+use helpers::*;
--
2.49.0.906.g1f30a19c02-googOn Thu, May 01, 2025 at 10:47:42PM +0000, Matthew Maurer wrote:
> +/// Handle to a DebugFS file.
> +#[repr(transparent)]
> +pub struct File(ManuallyDrop<Dir>);
Same as with SubDir, please keep your original approach with keep().
Exposing this as a separate type is much better, but I still find it a bit weird
that it uses Dir internally, which still provides methods that are not
applicable.
I think it would be good to have the following types instead:
// Generic wrapper around the dentry pointer.
struct Entry;
// Based on Entry; provides Dir specific methods.
struct Dir;
// Based on Dir; implements Keep.
struct SubDir;
// Based on Entry; implements Keep.
struct File;
// Common trait that implements keep().
trait Keep;
> +impl File {
> + /// Remove the file from DebugFS.
> + ///
> + /// # Examples
> + /// ```
> + /// # use kernel::c_str;
> + /// # use kernel::debugfs::Dir;
> + /// let dir = Dir::new(c_str!("foo"));
> + /// {
> + /// let file = dir.display_file(c_str!("bar"), &0);
> + /// // "foo/bar" is created.
> + /// }
> + /// // "foo/bar" still exists.
> + /// {
> + /// let file = dir.display_file(c_str!("baz"), &0);
> + /// // "foo/baz" is created.
> + /// file.remove();
> + /// // "foo/baz" is gone.
> + /// }
> + pub fn remove(self) {
> + drop(ManuallyDrop::into_inner(self.0))
> + }
> +}
Same as with my comment on Dir::subdir(), it really gets confusing if we invert
the normal drop() logic. Removing the file when it is dropped and keeping it
when calling keep() is much more intuitive..
> +
> +#[cfg(CONFIG_DEBUG_FS)]
> +mod helpers {
> + use crate::seq_file::SeqFile;
> + use crate::seq_print;
> + use core::fmt::Display;
> +
> + /// Implements `open` for `file_operations` via `single_open` to fill out a `seq_file`.
> + ///
> + /// # Safety
> + ///
> + /// * `inode`'s private pointer must point to a value of type `T` which will outlive the `inode`
> + /// and will not be mutated during this call.
> + /// * `file` must point to a live, not-yet-initialized file object.
> + pub(crate) unsafe extern "C" fn display_open<T: Display>(
> + inode: *mut bindings::inode,
> + file: *mut bindings::file,
> + ) -> i32 {
> + // SAFETY:
> + // * `file` is acceptable by caller precondition.
> + // * `print_act` will be called on a `seq_file` with private data set to the third argument,
> + // so we meet its safety requirements.
> + // * The `data` pointer passed in the third argument is a valid `T` pointer that outlives
> + // this call by caller preconditions.
> + unsafe { bindings::single_open(file, Some(display_act::<T>), (*inode).i_private) }
Please split up unsafe operations.
> + }
> +
> + /// Prints private data stashed in a seq_file to that seq file.
> + ///
> + /// # Safety
> + ///
> + /// `seq` must point to a live `seq_file` whose private data is a live pointer to a `T` which is
> + /// not being mutated.
> + pub(crate) unsafe extern "C" fn display_act<T: Display>(
> + seq: *mut bindings::seq_file,
> + _: *mut core::ffi::c_void,
> + ) -> i32 {
> + // SAFETY: By caller precondition, this pointer is live, points to a value of type `T`, and
> + // is not being mutated.
> + let data = unsafe { &*((*seq).private as *mut T) };
This creates an intermediate reference to private, which is UB. Please use
addr_of! instead.
On Thu, May 1, 2025 at 11:52 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Thu, May 01, 2025 at 10:47:42PM +0000, Matthew Maurer wrote:
> > +/// Handle to a DebugFS file.
> > +#[repr(transparent)]
> > +pub struct File(ManuallyDrop<Dir>);
>
> Same as with SubDir, please keep your original approach with keep().
>
> Exposing this as a separate type is much better, but I still find it a bit weird
> that it uses Dir internally, which still provides methods that are not
> applicable.
>
> I think it would be good to have the following types instead:
>
> // Generic wrapper around the dentry pointer.
> struct Entry;
>
> // Based on Entry; provides Dir specific methods.
> struct Dir;
>
> // Based on Dir; implements Keep.
> struct SubDir;
>
> // Based on Entry; implements Keep.
> struct File;
>
> // Common trait that implements keep().
> trait Keep;
>
> > +impl File {
> > + /// Remove the file from DebugFS.
> > + ///
> > + /// # Examples
> > + /// ```
> > + /// # use kernel::c_str;
> > + /// # use kernel::debugfs::Dir;
> > + /// let dir = Dir::new(c_str!("foo"));
> > + /// {
> > + /// let file = dir.display_file(c_str!("bar"), &0);
> > + /// // "foo/bar" is created.
> > + /// }
> > + /// // "foo/bar" still exists.
> > + /// {
> > + /// let file = dir.display_file(c_str!("baz"), &0);
> > + /// // "foo/baz" is created.
> > + /// file.remove();
> > + /// // "foo/baz" is gone.
> > + /// }
> > + pub fn remove(self) {
> > + drop(ManuallyDrop::into_inner(self.0))
> > + }
> > +}
>
> Same as with my comment on Dir::subdir(), it really gets confusing if we invert
> the normal drop() logic. Removing the file when it is dropped and keeping it
> when calling keep() is much more intuitive..
>
> > +
> > +#[cfg(CONFIG_DEBUG_FS)]
> > +mod helpers {
> > + use crate::seq_file::SeqFile;
> > + use crate::seq_print;
> > + use core::fmt::Display;
> > +
> > + /// Implements `open` for `file_operations` via `single_open` to fill out a `seq_file`.
> > + ///
> > + /// # Safety
> > + ///
> > + /// * `inode`'s private pointer must point to a value of type `T` which will outlive the `inode`
> > + /// and will not be mutated during this call.
> > + /// * `file` must point to a live, not-yet-initialized file object.
> > + pub(crate) unsafe extern "C" fn display_open<T: Display>(
> > + inode: *mut bindings::inode,
> > + file: *mut bindings::file,
> > + ) -> i32 {
> > + // SAFETY:
> > + // * `file` is acceptable by caller precondition.
> > + // * `print_act` will be called on a `seq_file` with private data set to the third argument,
> > + // so we meet its safety requirements.
> > + // * The `data` pointer passed in the third argument is a valid `T` pointer that outlives
> > + // this call by caller preconditions.
> > + unsafe { bindings::single_open(file, Some(display_act::<T>), (*inode).i_private) }
>
> Please split up unsafe operations.
>
> > + }
> > +
> > + /// Prints private data stashed in a seq_file to that seq file.
> > + ///
> > + /// # Safety
> > + ///
> > + /// `seq` must point to a live `seq_file` whose private data is a live pointer to a `T` which is
> > + /// not being mutated.
> > + pub(crate) unsafe extern "C" fn display_act<T: Display>(
> > + seq: *mut bindings::seq_file,
> > + _: *mut core::ffi::c_void,
> > + ) -> i32 {
> > + // SAFETY: By caller precondition, this pointer is live, points to a value of type `T`, and
> > + // is not being mutated.
> > + let data = unsafe { &*((*seq).private as *mut T) };
>
> This creates an intermediate reference to private, which is UB. Please use
> addr_of! instead.
I'm making this change, but so that I can be correct in the future,
can you explain why taking an intermediate reference to private is UB?
My understanding is that my provided vtable are supposed to be the
only methods looking at this field, and they don't mutate it.
Additionally, the `private_data` field on file is accessed this way in
`miscdevice` at the moment - what makes it safe there, and UB here?
On Fri, May 02, 2025 at 11:07:47AM -0700, Matthew Maurer wrote: > I'm making this change, but so that I can be correct in the future, > can you explain why taking an intermediate reference to private is UB? > My understanding is that my provided vtable are supposed to be the > only methods looking at this field, and they don't mutate it. You're right, I confused it with something else.
© 2016 - 2026 Red Hat, Inc.