rust: DebugFS Bindings

Extends the debugfs API to support creating read-only files. This is done via the Dir::read_only_file method, which takes a data object that implements the Writer trait.

The file's content is generated by the Writer implementation, and the file is automatically removed when the returned File handle is dropped.

Signed-off-by Matthew Maurer <[email protected]>

---
 rust/kernel/debugfs.rs          | 148 +++++++++++++++++++++++++++++++++++++++-
 rust/kernel/debugfs/entry.rs    |  42 ++++++++++++
 rust/kernel/debugfs/file_ops.rs | 128 ++++++++++++++++++++++++++++++++++
 rust/kernel/debugfs/traits.rs   |  33 +++++++++
 4 files changed, 350 insertions(+), 1 deletion(-)

rust/kernel/debugfs.rs +147 -1

index 65be71600b8eda83c0d313f3d205d0713e8e9510..b28665f58cd6a17e188aef5e8c539f6c7433a3b0 100644
@@ -8,12 +8,18 @@
 // When DebugFS is disabled, many parameters are dead. Linting for this isn't helpful.
 #![cfg_attr(not(CONFIG_DEBUG_FS), allow(unused_variables))]
 
-#[cfg(CONFIG_DEBUG_FS)]
 use crate::prelude::*;
 use crate::str::CStr;
 #[cfg(CONFIG_DEBUG_FS)]
 use crate::sync::Arc;
+use core::marker::PhantomPinned;
+use core::ops::Deref;
+
+mod traits;
+pub use traits::Writer;
 
+mod file_ops;
+use file_ops::{FileOps, ReadFile};
 #[cfg(CONFIG_DEBUG_FS)]
 mod entry;
 #[cfg(CONFIG_DEBUG_FS)]

@@ -53,6 +59,34 @@ fn create(name: &CStr, parent: Option<&Dir>) -> Self {
         Self()
     }
 
+    /// Creates a DebugFS file which will own the data produced by the initializer provided in
+    /// `data`.
+    fn create_file<'a, T, E: 'a>(
+        &'a self,
+        name: &'a CStr,
+        data: impl PinInit<T, E> + 'a,
+        file_ops: &'static FileOps<T>,
+    ) -> impl PinInit<File<T>, E> + 'a
+    where
+        T: Sync + 'static,
+    {
+        let scope = Scope::<T>::new(data, move |data| {
+            #[cfg(CONFIG_DEBUG_FS)]
+            if let Some(parent) = &self.0 {
+                // SAFETY: Because data derives from a scope, and our entry will be dropped before
+                // the data is dropped, it is guaranteed to outlive the entry we return.
+                unsafe { Entry::dynamic_file(name, parent.clone(), data, file_ops) }
+            } else {
+                Entry::empty()
+            }
+        });
+        try_pin_init! {
+            File {
+                scope <- scope
+            } ? E
+        }
+    }
+
     /// Create a new directory in DebugFS at the root.
     ///
     /// # Examples

@@ -79,4 +113,116 @@ pub fn new(name: &CStr) -> Self {
     pub fn subdir(&self, name: &CStr) -> Self {
         Dir::create(name, Some(self))
     }
+
+    /// Creates a read-only file in this directory.
+    ///
+    /// The file's contents are produced by invoking [`Writer::write`] on the value initialized by
+    /// `data`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// # use kernel::prelude::*;
+    /// # let dir = Dir::new(c_str!("my_debugfs_dir"));
+    /// let file = KBox::pin_init(dir.read_only_file(c_str!("foo"), 200), GFP_KERNEL)?;
+    /// // "my_debugfs_dir/foo" now contains the number 200.
+    /// // The file is removed when `file` is dropped.
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn read_only_file<'a, T, E: 'a>(
+        &'a self,
+        name: &'a CStr,
+        data: impl PinInit<T, E> + 'a,
+    ) -> impl PinInit<File<T>, E> + 'a
+    where
+        T: Writer + Send + Sync + 'static,
+    {
+        let file_ops = &<T as ReadFile<_>>::FILE_OPS;
+        self.create_file(name, data, file_ops)
+    }
+}
+
+#[pin_data]
+/// Handle to a DebugFS scope, which ensures that attached `data` will outlive the provided
+/// [`Entry`] without moving.
+/// Currently, this is used to back [`File`] so that its `read` and/or `write` implementations
+/// can assume that their backing data is still alive.
+struct Scope<T> {
+    // This order is load-bearing for drops - `_entry` must be dropped before `data`.
+    #[cfg(CONFIG_DEBUG_FS)]
+    _entry: Entry,
+    #[pin]
+    data: T,
+    // Even if `T` is `Unpin`, we still can't allow it to be moved.
+    #[pin]
+    _pin: PhantomPinned,
+}
+
+#[pin_data]
+/// Handle to a DebugFS file, owning its backing data.
+///
+/// When dropped, the DebugFS file will be removed and the attached data will be dropped.
+pub struct File<T> {
+    #[pin]
+    scope: Scope<T>,
+}
+
+#[cfg(not(CONFIG_DEBUG_FS))]
+impl<'b, T: 'b> Scope<T> {
+    fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
+    where
+        F: for<'a> FnOnce(&'a T) -> Entry + 'b,
+    {
+        try_pin_init! {
+            Self {
+                data <- data,
+                _pin: PhantomPinned
+            } ? E
+        }
+        .pin_chain(|scope| {
+            init(&scope.data);
+            Ok(())
+        })
+    }
+}
+
+#[cfg(CONFIG_DEBUG_FS)]
+impl<'b, T: 'b> Scope<T> {
+    fn entry_mut(self: Pin<&mut Self>) -> &mut Entry {
+        // SAFETY: _entry is not structurally pinned.
+        unsafe { &mut Pin::into_inner_unchecked(self)._entry }
+    }
+
+    fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
+    where
+        F: for<'a> FnOnce(&'a T) -> Entry + 'b,
+    {
+        try_pin_init! {
+            Self {
+                _entry: Entry::empty(),
+                data <- data,
+                _pin: PhantomPinned
+            } ? E
+        }
+        .pin_chain(|scope| {
+            *scope.entry_mut() = init(&scope.data);
+            Ok(())
+        })
+    }
+}
+
+impl<T> Deref for Scope<T> {
+    type Target = T;
+    fn deref(&self) -> &T {
+        &self.data
+    }
+}
+
+impl<T> Deref for File<T> {
+    type Target = T;
+    fn deref(&self) -> &T {
+        &self.scope
+    }
 }

rust/kernel/debugfs/entry.rs +42 -0

index d2fba0e65e20e954e2a33e776b872bac4adb12e8..227fa50b7a79aeab49779e54b8c4241f455777c3 100644
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (C) 2025 Google LLC.
 
+use crate::debugfs::file_ops::FileOps;
+use crate::ffi::c_void;
 use crate::str::CStr;
 use crate::sync::Arc;
 

@@ -40,6 +42,46 @@ pub(crate) fn dynamic_dir(name: &CStr, parent: Option<Arc<Self>>) -> Self {
         }
     }
 
+    /// # Safety
+    ///
+    /// * `data` must outlive the returned `Entry`.
+    pub(crate) unsafe fn dynamic_file<T>(
+        name: &CStr,
+        parent: Arc<Self>,
+        data: &T,
+        file_ops: &'static FileOps<T>,
+    ) -> Self {
+        // SAFETY: The invariants of this function's arguments ensure the safety of this call.
+        // * `name` is a valid C string by the invariants of `&CStr`.
+        // * `parent.as_ptr()` is a pointer to a valid `dentry` by invariant.
+        // * The caller guarantees that `data` will outlive the returned `Entry`.
+        // * The guarantees on `FileOps` assert the vtable will be compatible with the data we have
+        //   provided.
+        let entry = unsafe {
+            bindings::debugfs_create_file_full(
+                name.as_char_ptr(),
+                file_ops.mode(),
+                parent.as_ptr(),
+                core::ptr::from_ref(data) as *mut c_void,
+                core::ptr::null(),
+                &**file_ops,
+            )
+        };
+
+        Entry {
+            entry,
+            _parent: Some(parent),
+        }
+    }
+
+    /// Constructs a placeholder DebugFS [`Entry`].
+    pub(crate) fn empty() -> Self {
+        Self {
+            entry: core::ptr::null_mut(),
+            _parent: None,
+        }
+    }
+
     /// Returns the pointer representation of the DebugFS directory.
     ///
     /// # Guarantees

rust/kernel/debugfs/file_ops.rs new file +128 -0

new file mode 100644
index 0000000000000000000000000000000000000000..c2fbef96580eaa2fab7cc8c1ba559c3284d12e1b
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025 Google LLC.
+
+use super::Writer;
+use crate::prelude::*;
+use crate::seq_file::SeqFile;
+use crate::seq_print;
+use core::fmt::{Display, Formatter, Result};
+use core::marker::PhantomData;
+
+#[cfg(CONFIG_DEBUG_FS)]
+use core::ops::Deref;
+
+/// # Invariant
+///
+/// `FileOps<T>` will always contain an `operations` which is safe to use for a file backed
+/// off an inode which has a pointer to a `T` in its private data that is safe to convert
+/// into a reference.
+pub(super) struct FileOps<T> {
+    #[cfg(CONFIG_DEBUG_FS)]
+    operations: bindings::file_operations,
+    #[cfg(CONFIG_DEBUG_FS)]
+    mode: u16,
+    _phantom: PhantomData<T>,
+}
+
+impl<T> FileOps<T> {
+    /// # Safety
+    ///
+    /// The caller asserts that the provided `operations` is safe to use for a file whose
+    /// inode has a pointer to `T` in its private data that is safe to convert into a reference.
+    const unsafe fn new(operations: bindings::file_operations, mode: u16) -> Self {
+        Self {
+            #[cfg(CONFIG_DEBUG_FS)]
+            operations,
+            #[cfg(CONFIG_DEBUG_FS)]
+            mode,
+            _phantom: PhantomData,
+        }
+    }
+
+    #[cfg(CONFIG_DEBUG_FS)]
+    pub(crate) const fn mode(&self) -> u16 {
+        self.mode
+    }
+}
+
+#[cfg(CONFIG_DEBUG_FS)]
+impl<T> Deref for FileOps<T> {
+    type Target = bindings::file_operations;
+
+    fn deref(&self) -> &Self::Target {
+        &self.operations
+    }
+}
+
+struct WriterAdapter<T>(T);
+
+impl<'a, T: Writer> Display for WriterAdapter<&'a T> {
+    fn fmt(&self, f: &mut Formatter<'_>) -> Result {
+        self.0.write(f)
+    }
+}
+
+/// 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 have any unique references alias it during the call.
+/// * `file` must point to a live, not-yet-initialized file object.
+unsafe extern "C" fn writer_open<T: Writer + Sync>(
+    inode: *mut bindings::inode,
+    file: *mut bindings::file,
+) -> c_int {
+    // SAFETY: The caller ensures that `inode` is a valid pointer.
+    let data = unsafe { (*inode).i_private };
+    // 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(writer_act::<T>), data) }
+}
+
+/// 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 valid pointer to a `T` which may
+/// not have any unique references alias it during the call.
+unsafe extern "C" fn writer_act<T: Writer + Sync>(
+    seq: *mut bindings::seq_file,
+    _: *mut c_void,
+) -> c_int {
+    // SAFETY: By caller precondition, this pointer is valid pointer to a `T`, and
+    // there are not and will not be any unique references until we are done.
+    let data = unsafe { &*((*seq).private.cast::<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, "{}", WriterAdapter(data));
+    0
+}
+
+// Work around lack of generic const items.
+pub(crate) trait ReadFile<T> {
+    const FILE_OPS: FileOps<T>;
+}
+
+impl<T: Writer + Sync> ReadFile<T> for T {
+    const FILE_OPS: FileOps<T> = {
+        let operations = bindings::file_operations {
+            read: Some(bindings::seq_read),
+            llseek: Some(bindings::seq_lseek),
+            release: Some(bindings::single_release),
+            open: Some(writer_open::<Self>),
+            // SAFETY: `file_operations` supports zeroes in all fields.
+            ..unsafe { core::mem::zeroed() }
+        };
+        // SAFETY: `operations` is all stock `seq_file` implementations except for `writer_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 matches the
+        // `FileOps` requirements.
+        unsafe { FileOps::new(operations, 0o400) }
+    };
+}

rust/kernel/debugfs/traits.rs new file +23 -0

new file mode 100644
index 0000000000000000000000000000000000000000..0e6e461324de42a3d80b692264d50e78a48f561d
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025 Google LLC.
+
+//! Traits for rendering or updating values exported to DebugFS.
+
+use crate::sync::Mutex;
+use core::fmt::{self, Debug, Formatter};
+
+/// A trait for types that can be written into a string.
+///
+/// This works very similarly to `Debug`, and is automatically implemented if `Debug` is
+/// implemented for a type. It is also implemented for any writable type inside a `Mutex`.
+///
+/// The derived implementation of `Debug` [may
+/// change](https://doc.rust-lang.org/std/fmt/trait.Debug.html#stability)
+/// between Rust versions, so if stability is key for your use case, please implement `Writer`
+/// explicitly instead.
+pub trait Writer {
+    /// Formats the value using the given formatter.
+    fn write(&self, f: &mut Formatter<'_>) -> fmt::Result;
+}
+
+impl<T: Writer> Writer for Mutex<T> {

I tried using this in a "tiny" test module I had written, and I get the following build error:

   --> samples/rust/rust_debugfs2.rs:64:53
    |
64  |         _file = root.read_only_file(c_str!("name"), &hw_soc_info.name);
    |                      --------------                 ^^^^^^^^^^^^^^^^^ expected `&u32`, found `&&CStr`
    |                      |
    |                      arguments to this method are incorrect
    |

= note: expected reference &u32
found reference &&'static kernel::prelude::CStr

I'm trying to "just" print a CStr, which is defined as:

struct HwSocInfo {
    id: u32,
    ver: u32,
    raw_id: u32,
    foundry: u32,
    name: &'static CStr,
}

Is this just a "user is holding it wrong" error on my side, or can this api not handle CStr values?

Quoted 23 lines · Greg Kroah-Hartman

I tried using this in a "tiny" test module I had written, and I get the following build error:

   --> samples/rust/rust_debugfs2.rs:64:53
    |
64  |         _file = root.read_only_file(c_str!("name"), &hw_soc_info.name);
    |                      --------------                 ^^^^^^^^^^^^^^^^^ expected `&u32`, found `&&CStr`
    |                      |
    |                      arguments to this method are incorrect
    |

= note: expected reference &u32
found reference &&'static kernel::prelude::CStr

I'm trying to "just" print a CStr, which is defined as:

struct HwSocInfo {
    id: u32,
    ver: u32,
    raw_id: u32,
    foundry: u32,
    name: &'static CStr,
}

Quoted 2 lines

Is this just a "user is holding it wrong" error on my side, or can this api not handle CStr values?

What you're doing should fundamentally work.

The above error suggests that your declaration of _file is File<&u32> rather than File<&'static CStr>.

Also note the double reference you create with &hw_soc_info.name, this should just be hw_soc_info.name.

You can also test this case by applying the following diff the the sample in v5:

samples/rust/rust_debugfs.rs +3 -0

index b26eea3ee723..475502f30b1a 100644
@@ -59,6 +59,8 @@ struct RustDebugFs {
     #[pin]
     _compatible: File<CString>,
     #[pin]
+    _test: File<&'static CStr>,
+    #[pin]
     counter: File<AtomicUsize>,
     #[pin]
     inner: File<Mutex<Inner>>,

@@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
                         .property_read::<CString>(c_str!("compatible"))
                         .required_by(dev)?,
                 ),
+                _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),
                 counter <- Self::build_counter(&debugfs),
                 inner <- Self::build_inner(&debugfs),
                 _debugfs: debugfs,

Quoted 23 lines · Greg Kroah-Hartman

I tried using this in a "tiny" test module I had written, and I get the following build error:

   --> samples/rust/rust_debugfs2.rs:64:53
    |
64  |         _file = root.read_only_file(c_str!("name"), &hw_soc_info.name);
    |                      --------------                 ^^^^^^^^^^^^^^^^^ expected `&u32`, found `&&CStr`
    |                      |
    |                      arguments to this method are incorrect
    |

= note: expected reference &u32
found reference &&'static kernel::prelude::CStr

I'm trying to "just" print a CStr, which is defined as:

struct HwSocInfo {
    id: u32,
    ver: u32,
    raw_id: u32,
    foundry: u32,
    name: &'static CStr,
}

Quoted 2 lines

Is this just a "user is holding it wrong" error on my side, or can this api not handle CStr values?

What you're doing should fundamentally work.

The above error suggests that your declaration of _file is File<&u32> rather than File<&'static CStr>.

Also note the double reference you create with &hw_soc_info.name, this should just be hw_soc_info.name.

You can also test this case by applying the following diff the the sample in v5:

samples/rust/rust_debugfs.rs +2 -0

index b26eea3ee723..475502f30b1a 100644
@@ -59,6 +59,8 @@ struct RustDebugFs {
     #[pin]
     _compatible: File<CString>,
     #[pin]
+    _test: File<&'static CStr>,
+    #[pin]
     counter: File<AtomicUsize>,
     #[pin]
     inner: File<Mutex<Inner>>,

Quoted 15 lines · Danilo Krummrich

samples/rust/rust_debugfs.rs +2 -0

index b26eea3ee723..475502f30b1a 100644
@@ -59,6 +59,8 @@ struct RustDebugFs {
     #[pin]
     _compatible: File<CString>,
     #[pin]
+    _test: File<&'static CStr>,
+    #[pin]
     counter: File<AtomicUsize>,
     #[pin]
     inner: File<Mutex<Inner>>,

@@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
                         .property_read::<CString>(c_str!("compatible"))

samples/rust/rust_debugfs.rs fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ { · line 145

Quoted 3 lines

samples/rust/rust_debugfs.rs +1 -0

                         .required_by(dev)?,
                 ),
+                _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),

Cool, but again, we do not want to ever be storing individual debugfs files. Well, we can, but for 90% of the cases, we do not, we only want to remove the whole directory when that goes out of scope, which will clean up the files then.

Quoted 16 lines · Greg Kroah-Hartman

Quoted 15 lines · Danilo Krummrich

samples/rust/rust_debugfs.rs +2 -0

index b26eea3ee723..475502f30b1a 100644
@@ -59,6 +59,8 @@ struct RustDebugFs {
     #[pin]
     _compatible: File<CString>,
     #[pin]
+    _test: File<&'static CStr>,
+    #[pin]
     counter: File<AtomicUsize>,
     #[pin]
     inner: File<Mutex<Inner>>,

@@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
                         .property_read::<CString>(c_str!("compatible"))

Quoted 8 lines

samples/rust/rust_debugfs.rs fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ { · line 145

Quoted 3 lines

samples/rust/rust_debugfs.rs +1 -0

                         .required_by(dev)?,
                 ),
+                _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),

Cool, but again, we do not want to ever be storing individual debugfs files. Well, we can, but for 90% of the cases, we do not, we only want to remove the whole directory when that goes out of scope, which will clean up the files then.

This API does not work in the way that you have a struct storing the data you want to expose *and* another one for the files with the data attached.

The File type contains the actual data. For instance, if you have a struct Foo, where you want to expose the members through debugfs you would *not* do:

	struct Foo {
	   a: u32,
	   b: u32,
	}

	struct FooFiles {
	   a: File<&u32>,
	   b: File<&u32>
	}

and then create an instance of Foo *and* another instance of FooFiles to export them via debugfs.

Instead you would change your struct Foo to just be:

	struct Foo {
	   a: File<u32>,
	   b: File<u32>,
	}

If you now create an instance of Foo (let's call it foo), then foo.a or foo.b dereferences to the inner type, i.e. the u32. Or in other words foo still behaves as if a and b would be u32 values. For instance:

   if foo.a == 42 {
      pr_info!("Foo::b = {}\n", foo.b);
   }

The fact that the backing files of a and b are removed from debugfs when Foo is dropped is necessary since otherwise we create a UAF.

Think of File<T> as a containers like you think of KBox<T>.

KBox<T> behaves exactly like T, but silently manages the backing kmalloc() allocation that T lives in.

With File<T> it's exactly the same, it behaves exactly like the T that lives within File<T>, but silently manages the debugfs file the T is exposed by.

Quoted 42 lines · Danilo Krummrich

Quoted 16 lines · Greg Kroah-Hartman

Quoted 15 lines · Danilo Krummrich

samples/rust/rust_debugfs.rs +2 -0

index b26eea3ee723..475502f30b1a 100644
@@ -59,6 +59,8 @@ struct RustDebugFs {
     #[pin]
     _compatible: File<CString>,
     #[pin]
+    _test: File<&'static CStr>,
+    #[pin]
     counter: File<AtomicUsize>,
     #[pin]
     inner: File<Mutex<Inner>>,

@@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
                         .property_read::<CString>(c_str!("compatible"))

Quoted 8 lines

samples/rust/rust_debugfs.rs fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ { · line 145

Quoted 3 lines

samples/rust/rust_debugfs.rs +1 -0

                         .required_by(dev)?,
                 ),
+                _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),

Cool, but again, we do not want to ever be storing individual debugfs files. Well, we can, but for 90% of the cases, we do not, we only want to remove the whole directory when that goes out of scope, which will clean up the files then.

This API does not work in the way that you have a struct storing the data you want to expose *and* another one for the files with the data attached.

The File type contains the actual data. For instance, if you have a struct Foo, where you want to expose the members through debugfs you would *not* do:

      struct Foo {
         a: u32,
         b: u32,
      }

      struct FooFiles {
         a: File<&u32>,
         b: File<&u32>
      }

Quoted 2 lines

and then create an instance of Foo *and* another instance of FooFiles to export them via debugfs.

Ah, that's exactly what I was trying to do.

Quoted 7 lines

Instead you would change your struct Foo to just be:

      struct Foo {
         a: File<u32>,
         b: File<u32>,
      }

Quoted 7 lines

If you now create an instance of Foo (let's call it foo), then foo.a or foo.b dereferences to the inner type, i.e. the u32. Or in other words foo still behaves as if a and b would be u32 values. For instance:

   if foo.a == 42 {
      pr_info!("Foo::b = {}\n", foo.b);
   }

Oh that's not going to work well at all :(

Think about something "simple" like a pci config descriptor. You have a structure, with fields, already sitting there. You want to expose those fields in debugfs. So you want to only create debugfs files in one location in a driver, you don't want ALL users of those fields to have to go through a File<T> api, right? That would be crazy, all drivers would end up always having File<T> everywhere.

Quoted 2 lines

The fact that the backing files of a and b are removed from debugfs when Foo is dropped is necessary since otherwise we create a UAF.

That's fine, but:

Quoted 1 line

Think of File<T> as a containers like you think of KBox<T>.

Ok, but again, you are now forcing all users to think of debugfs as the main "interface" to those variables, which is not true (nor should it be.)

All of these things is why I recommended to Matthew that he should add back the scoped API, since it doesn't have all these drawbacks.

Quoted 42 lines · Danilo Krummrich

Quoted 16 lines · Greg Kroah-Hartman

Quoted 15 lines · Danilo Krummrich

samples/rust/rust_debugfs.rs +2 -0

index b26eea3ee723..475502f30b1a 100644
@@ -59,6 +59,8 @@ struct RustDebugFs {
     #[pin]
     _compatible: File<CString>,
     #[pin]
+    _test: File<&'static CStr>,
+    #[pin]
     counter: File<AtomicUsize>,
     #[pin]
     inner: File<Mutex<Inner>>,

@@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
                         .property_read::<CString>(c_str!("compatible"))

Quoted 8 lines

samples/rust/rust_debugfs.rs fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ { · line 145

Quoted 3 lines

samples/rust/rust_debugfs.rs +1 -0

                         .required_by(dev)?,
                 ),
+                _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),

Cool, but again, we do not want to ever be storing individual debugfs files. Well, we can, but for 90% of the cases, we do not, we only want to remove the whole directory when that goes out of scope, which will clean up the files then.

This API does not work in the way that you have a struct storing the data you want to expose *and* another one for the files with the data attached.

The File type contains the actual data. For instance, if you have a struct Foo, where you want to expose the members through debugfs you would *not* do:

	struct Foo {
	   a: u32,
	   b: u32,
	}

	struct FooFiles {
	   a: File<&u32>,
	   b: File<&u32>
	}

Quoted 2 lines

and then create an instance of Foo *and* another instance of FooFiles to export them via debugfs.

Ah, that's exactly what I was trying to do.

Quoted 7 lines

Instead you would change your struct Foo to just be:

	struct Foo {
	   a: File<u32>,
	   b: File<u32>,
	}

Quoted 7 lines

If you now create an instance of Foo (let's call it foo), then foo.a or foo.b dereferences to the inner type, i.e. the u32. Or in other words foo still behaves as if a and b would be u32 values. For instance:

   if foo.a == 42 {
      pr_info!("Foo::b = {}\n", foo.b);
   }

Oh that's not going to work well at all :(

Think about something "simple" like a pci config descriptor. You have a structure, with fields, already sitting there. You want to expose those fields in debugfs.

Quoted 2 lines

The fact that the backing files of a and b are removed from debugfs when Foo is dropped is necessary since otherwise we create a UAF.

That's fine, but:

Quoted 1 line

Think of File<T> as a containers like you think of KBox<T>.

Ok, but again, you are now forcing all users to think of debugfs as the main "interface" to those variables, which is not true (nor should it be.)

Quoted 3 lines

KBox<T> behaves exactly like T, but silently manages the backing kmalloc() allocation that T lives in.

Quoted 2 lines

With File<T> it's exactly the same, it behaves exactly like the T that lives within File<T>, but silently manages the debugfs file the T is exposed by.

And what happens if debugfs is not enabled? What about if creating the file fails? The variable still needs to be present and active and working.

Quoted 46 lines · Greg Kroah-Hartman

Quoted 44 lines · Danilo Krummrich

Quoted 16 lines · Greg Kroah-Hartman

Quoted 15 lines · Danilo Krummrich

samples/rust/rust_debugfs.rs +2 -0

index b26eea3ee723..475502f30b1a 100644
@@ -59,6 +59,8 @@ struct RustDebugFs {
     #[pin]
     _compatible: File<CString>,
     #[pin]
+    _test: File<&'static CStr>,
+    #[pin]
     counter: File<AtomicUsize>,
     #[pin]
     inner: File<Mutex<Inner>>,

@@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
                         .property_read::<CString>(c_str!("compatible"))

Quoted 8 lines

samples/rust/rust_debugfs.rs fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ { · line 145

Quoted 3 lines

samples/rust/rust_debugfs.rs +1 -0

                         .required_by(dev)?,
                 ),
+                _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),

Cool, but again, we do not want to ever be storing individual debugfs files. Well, we can, but for 90% of the cases, we do not, we only want to remove the whole directory when that goes out of scope, which will clean up the files then.

This API does not work in the way that you have a struct storing the data you want to expose *and* another one for the files with the data attached.

The File type contains the actual data. For instance, if you have a struct Foo, where you want to expose the members through debugfs you would *not* do:

	struct Foo {
	   a: u32,
	   b: u32,
	}

	struct FooFiles {
	   a: File<&u32>,
	   b: File<&u32>
	}

and then create an instance of Foo *and* another instance of FooFiles to export them via debugfs.

Quoted 1 line

Ah, that's exactly what I was trying to do.

But that's bad, then we're back at the lifetime problem from the beginning, because the File<&Foo> then somehow needs to ensure that the instance Foo remains alive as long as File<&Foo> or the backing directory exists.

So, you eventually end of with Foo needing to be reference counted with its own memory allocation, which horribly messes with your lifetimes in the driver.

You don't want a a field to be reference counted just because it's exposed via debugfs.

Quoted 15 lines

Quoted 14 lines

Instead you would change your struct Foo to just be:

	struct Foo {
	   a: File<u32>,
	   b: File<u32>,
	}

If you now create an instance of Foo (let's call it foo), then foo.a or foo.b dereferences to the inner type, i.e. the u32. Or in other words foo still behaves as if a and b would be u32 values. For instance:

   if foo.a == 42 {
      pr_info!("Foo::b = {}\n", foo.b);
   }

Quoted 5 lines

Oh that's not going to work well at all :(

Think about something "simple" like a pci config descriptor. You have a structure, with fields, already sitting there. You want to expose those fields in debugfs.

This is more of a special case that is addressed by the Scope API in patch 6 and patch 7, so we should be good.

Quoted 3 lines

And what happens if debugfs is not enabled? What about if creating the file fails? The variable still needs to be present and active and working.

This is the case, the variable will still be present and active in any case.

Quoted 49 lines · Danilo Krummrich

Quoted 47 lines · Greg Kroah-Hartman

Quoted 42 lines · Danilo Krummrich

Quoted 16 lines · Greg Kroah-Hartman

Quoted 15 lines · Danilo Krummrich

samples/rust/rust_debugfs.rs +2 -0

index b26eea3ee723..475502f30b1a 100644
@@ -59,6 +59,8 @@ struct RustDebugFs {
     #[pin]
     _compatible: File<CString>,
     #[pin]
+    _test: File<&'static CStr>,
+    #[pin]
     counter: File<AtomicUsize>,
     #[pin]
     inner: File<Mutex<Inner>>,

@@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
                         .property_read::<CString>(c_str!("compatible"))

Quoted 8 lines

samples/rust/rust_debugfs.rs fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ { · line 145

Quoted 3 lines

samples/rust/rust_debugfs.rs +1 -0

                         .required_by(dev)?,
                 ),
+                _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),

Cool, but again, we do not want to ever be storing individual debugfs files. Well, we can, but for 90% of the cases, we do not, we only want to remove the whole directory when that goes out of scope, which will clean up the files then.

This API does not work in the way that you have a struct storing the data you want to expose *and* another one for the files with the data attached.

The File type contains the actual data. For instance, if you have a struct Foo, where you want to expose the members through debugfs you would *not* do:

	struct Foo {
	   a: u32,
	   b: u32,
	}

	struct FooFiles {
	   a: File<&u32>,
	   b: File<&u32>
	}

Quoted 2 lines

and then create an instance of Foo *and* another instance of FooFiles to export them via debugfs.

Ah, that's exactly what I was trying to do.

Quoted 6 lines

But that's bad, then we're back at the lifetime problem from the beginning, because the File<&Foo> then somehow needs to ensure that the instance Foo remains alive as long as File<&Foo> or the backing directory exists.

So, you eventually end of with Foo needing to be reference counted with its own memory allocation, which horribly messes with your lifetimes in the driver.

Once I want to drop Foo, FooFiles should "go out of scope" and be gone.

If a backing file descriptor is still held open, it will then become "stale" and not work. Much like the revokable stuff works.

Note, none of this is in the C code today, and debugfs is bound to root permissions, so it's not really an issue, but I can understand the goal of correctness...

Anyway, I looked at the scoped example here, and I don't see how that works any differently. How can I use it to have a single Dir "handle" that when goes out of scope, can drop the files attached to it that were created to reference Foo.a and Foo.b in your example above?

Quoted 50 lines · Greg Kroah-Hartman

Quoted 49 lines · Danilo Krummrich

Quoted 47 lines · Greg Kroah-Hartman

Quoted 42 lines · Danilo Krummrich

Quoted 16 lines · Greg Kroah-Hartman

Quoted 15 lines · Danilo Krummrich

samples/rust/rust_debugfs.rs +2 -0

index b26eea3ee723..475502f30b1a 100644
@@ -59,6 +59,8 @@ struct RustDebugFs {
     #[pin]
     _compatible: File<CString>,
     #[pin]
+    _test: File<&'static CStr>,
+    #[pin]
     counter: File<AtomicUsize>,
     #[pin]
     inner: File<Mutex<Inner>>,

@@ -140,6 +142,7 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
                         .property_read::<CString>(c_str!("compatible"))

Quoted 8 lines

samples/rust/rust_debugfs.rs fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ { · line 145

Quoted 3 lines

samples/rust/rust_debugfs.rs +1 -0

                         .required_by(dev)?,
                 ),
+                _test <- debugfs.read_only_file(c_str!("test"), c_str!("some_value")),

Cool, but again, we do not want to ever be storing individual debugfs files. Well, we can, but for 90% of the cases, we do not, we only want to remove the whole directory when that goes out of scope, which will clean up the files then.

This API does not work in the way that you have a struct storing the data you want to expose *and* another one for the files with the data attached.

The File type contains the actual data. For instance, if you have a struct Foo, where you want to expose the members through debugfs you would *not* do:

	struct Foo {
	   a: u32,
	   b: u32,
	}

	struct FooFiles {
	   a: File<&u32>,
	   b: File<&u32>
	}

Quoted 2 lines

and then create an instance of Foo *and* another instance of FooFiles to export them via debugfs.

Ah, that's exactly what I was trying to do.

Quoted 8 lines

Quoted 6 lines

But that's bad, then we're back at the lifetime problem from the beginning, because the File<&Foo> then somehow needs to ensure that the instance Foo remains alive as long as File<&Foo> or the backing directory exists.

So, you eventually end of with Foo needing to be reference counted with its own memory allocation, which horribly messes with your lifetimes in the driver.

Once I want to drop Foo, FooFiles should "go out of scope" and be gone.

We agree on the goal here, but unfortunately it's not really possible. There are two options that were already exercised:

(1) Force that FooFiles (or FooDir) is bound to the lifetime of a reference of Foo with FooDir<&'a Foo>.

This isn't workable because we then can't store both of them into the same parent structure.

(2) Reference count Foo (Arc<Foo>) and make FooDir own a referenc count of Foo.

But this is bad for the mentioned reasons. :(

(3) The File<T> API we have now, which gives you the behavior you ask for with Scope<T>.

Where Scope<T> creates a directory and owns the data you pass to it, e.g. a pci config descriptor.

The user can create an arbitrary number of files exporting any of the fields in date that live in the scope and don't need to be tracked separately, i.e. don't create separate object instances.

The directory (and hence all the files) is removed once the Scope<T> is dropped, including the data it owns.

Quoted 6 lines

If a backing file descriptor is still held open, it will then become "stale" and not work. Much like the revokable stuff works.

Note, none of this is in the C code today, and debugfs is bound to root permissions, so it's not really an issue, but I can understand the goal of correctness...

The lifetime guarantee we talk about is about the debugfs file still having a pointer to data that has already been dropped / freed.

In C you have to remove the debugfs file or directly (and hence the file) before the data exposed through it is freed. In C this is on the driver to take care of.

(If in C a driver has multiple structures exported in the same debugfs directory it has to manually take care of keeping all structures alive as long as the directory (and hence all files) exist.)

In Rust we need the abstraction to guarantee this.

Quoted 4 lines

Anyway, I looked at the scoped example here, and I don't see how that works any differently. How can I use it to have a single Dir "handle" that when goes out of scope, can drop the files attached to it that were created to reference Foo.a and Foo.b in your example above?

In the example above you would move Foo into the Scope<Foo>. For instance:

	let dir = root_dir.scope(foo, cstr!("subdir"), |foo, dir| {
		dir.read_only_file(c_str!("a"), foo.a);
		dir.read_only_file(c_str!("b"), foo.b);
	});

Note that those methods don't return anything, they're automatically bound to the Scope in lifetime.

So, Foo could be your pci config descriptor.

If dir is dropped, everything dies, the Scope, the "subdir" directory, all the files and also Foo.

I can provide some working code later on (currently in a meeting). :)

On Mon, Sep 08, 2025 at 04:59:16PM +0200, Danilo Krummrich wrote:

Quoted 26 lines

We agree on the goal here, but unfortunately it's not really possible. There are two options that were already exercised:

(1) Force that FooFiles (or FooDir) is bound to the lifetime of a reference of Foo with FooDir<&'a Foo>.

This isn't workable because we then can't store both of them into the same parent structure.

(2) Reference count Foo (Arc<Foo>) and make FooDir own a referenc count of Foo.

But this is bad for the mentioned reasons. :(

(3) The File<T> API we have now, which gives you the behavior you ask for with Scope<T>.

Where Scope<T> creates a directory and owns the data you pass to it, e.g. a pci config descriptor.

The user can create an arbitrary number of files exporting any of the fields in date that live in the scope and don't need to be tracked separately, i.e. don't create separate object instances.

The directory (and hence all the files) is removed once the Scope<T> is dropped, including the data it owns.

Quoted 1 line

I can provide some working code later on (currently in a meeting). :)

Working code for the simple "foo" example will be good. Here's my horrible (and will not build) example I was trying to get to work.

Quoted 1 line · Greg Kroah-Hartman

On Mon, Sep 08, 2025 at 04:59:16PM +0200, Danilo Krummrich wrote:

<snip>

Quoted 26 lines

Quoted 26 lines

We agree on the goal here, but unfortunately it's not really possible. There are two options that were already exercised:

(1) Force that FooFiles (or FooDir) is bound to the lifetime of a reference of Foo with FooDir<&'a Foo>.

This isn't workable because we then can't store both of them into the same parent structure.

(2) Reference count Foo (Arc<Foo>) and make FooDir own a referenc count of Foo.

But this is bad for the mentioned reasons. :(

(3) The File<T> API we have now, which gives you the behavior you ask for with Scope<T>.

Where Scope<T> creates a directory and owns the data you pass to it, e.g. a pci config descriptor.

The user can create an arbitrary number of files exporting any of the fields in date that live in the scope and don't need to be tracked separately, i.e. don't create separate object instances.

The directory (and hence all the files) is removed once the Scope<T> is dropped, including the data it owns.

<snip>

Quoted 4 lines

Quoted 1 line

I can provide some working code later on (currently in a meeting). :)

Working code for the simple "foo" example will be good. Here's my horrible (and will not build) example I was trying to get to work.

Here it comes [1]. :)

[1] rust_debugfs_soc_info.rs

// SPDX-License-Identifier: GPL-2.0

//! Simple debugfs::Scope example.

use kernel::c_str;
use kernel::debugfs::{Dir, Scope};
use kernel::prelude::*;

module! {
type: MyModule,
name: "MyModule",
description: "Just a simple test module.",
license: "GPL",
}

#[derive(Debug)]

struct HwSocInfo {
    name: &'static CStr,
    ver: u32,
    id: u32,
}

impl HwSocInfo {
    fn new(name: &'static CStr, ver: u32, id: u32) -> Self {
        Self { name, ver, id }
    }
}

struct MyModule {
    // Dropped when MyModule is released (e.g. through `rmmod`).
    //
    // This will drop the inner `HwSocInfo`, the "foo" directory, and all files created within this
    // directory.
    _scope: Pin<KBox<Scope<HwSocInfo>>>,

}

impl kernel::Module for MyModule {
    fn init(_module: &'static kernel::ThisModule) -> Result<Self, Error> {
        let root_dir = Dir::new(c_str!("my_module"));

        // Obtain some `HwSocInfo`, could from anywhere.
        let soc_info = HwSocInfo::new(c_str!("foo"), 24, 42);

        let scope = KBox::pin_init(
            // Create directory scope, that contains some data and a bunch of files exporting this
            // data.
            root_dir.scope(soc_info, c_str!("hw_soc_info"), |soc_info, dir| {
                dir.read_only_file(c_str!("name"), &soc_info.name);
                dir.read_only_file(c_str!("ver"), &soc_info.ver);
                dir.read_only_file(c_str!("id"), &soc_info.id);
            }),
            GFP_KERNEL,
        )?;

        // Print the contents of `soc_info` that were moved into `scope`.
        pr_info!("HwSocInfo: {:?}\n", &**scope);

        Ok(Self { _scope: scope })
    }
}

Quoted 71 lines · Danilo Krummrich

Quoted 1 line · Greg Kroah-Hartman

On Mon, Sep 08, 2025 at 04:59:16PM +0200, Danilo Krummrich wrote:

<snip>

Quoted 26 lines

Quoted 26 lines

We agree on the goal here, but unfortunately it's not really possible. There are two options that were already exercised:

(1) Force that FooFiles (or FooDir) is bound to the lifetime of a reference of Foo with FooDir<&'a Foo>.

This isn't workable because we then can't store both of them into the same parent structure.

(2) Reference count Foo (Arc<Foo>) and make FooDir own a referenc count of Foo.

But this is bad for the mentioned reasons. :(

(3) The File<T> API we have now, which gives you the behavior you ask for with Scope<T>.

Where Scope<T> creates a directory and owns the data you pass to it, e.g. a pci config descriptor.

The user can create an arbitrary number of files exporting any of the fields in date that live in the scope and don't need to be tracked separately, i.e. don't create separate object instances.

The directory (and hence all the files) is removed once the Scope<T> is dropped, including the data it owns.

<snip>

Quoted 4 lines

Quoted 1 line

I can provide some working code later on (currently in a meeting). :)

Working code for the simple "foo" example will be good. Here's my horrible (and will not build) example I was trying to get to work.

Here it comes [1]. :)

[1] rust_debugfs_soc_info.rs

// SPDX-License-Identifier: GPL-2.0

//! Simple debugfs::Scope example.

use kernel::c_str;
use kernel::debugfs::{Dir, Scope};
use kernel::prelude::*;

module! {
type: MyModule,
name: "MyModule",
description: "Just a simple test module.",
license: "GPL",
}

#[derive(Debug)]

struct HwSocInfo {
    name: &'static CStr,
    ver: u32,
    id: u32,
}

impl HwSocInfo {
    fn new(name: &'static CStr, ver: u32, id: u32) -> Self {
        Self { name, ver, id }
    }
}

Quoted 6 lines

struct MyModule {
    // Dropped when MyModule is released (e.g. through `rmmod`).
    //
    // This will drop the inner `HwSocInfo`, the "foo" directory, and all files created within this
    // directory.
    _scope: Pin<KBox<Scope<HwSocInfo>>>,

And yes, I get that HwSocInfo now lives within a debugfs structure, just like with

	struct Data {
	   version: File<u32>,
	}

but those become transparent wrappers if debugfs is disabled, i.e. zero overhead. They also won't make the driver fail if anything with debugfs goes south if enabled.

And I also understand your point that now they're part of a "real" data structure. But in the end, debugfs *is* part of the driver. And while we should ensure that it doesn't impact drivers as much as possible (which we do), I don't think that we necessarily have to hide the fact entirely.

Having that said, I also don't really see an alternative. If we really want debugfs structures to be entirely separate we would have to either

(1) reference count fields exposed through debugfs, or

(2) make the interface unsafe, use raw pointers and assert that a debugfs file never out-lives the data it exposes, just like in C.

As I mentioned previously, while File<T> is visible in driver structures only,
(1) even enforces drivers to break their lifetime patterns, which is much worse and not acceptable I think.

I would even say that (2) is better than (1), because it becomes safe when debugfs is disabled. Yet I think both (1) and (2) are much worse that what we have right now.

And I think the Scope API helps a lot in keeping things reasonable for cases where a lot of fields of a single structure should be exposed separately, such as the one you sketched up.

Working code for the simple "foo" example will be good. Here's my horrible (and will not build) example I was trying to get to work.

Extends the debugfs API to support creating read-only files. This is done via the Dir::read_only_file method, which takes a data object that implements the Writer trait.

The file's content is generated by the Writer implementation, and the file is automatically removed when the returned File handle is dropped.

Signed-off-by Matthew Maurer <[email protected]>

---
 rust/kernel/debugfs.rs          | 148 +++++++++++++++++++++++++++++++++++++++-
 rust/kernel/debugfs/entry.rs    |  42 ++++++++++++
 rust/kernel/debugfs/file_ops.rs | 128 ++++++++++++++++++++++++++++++++++
 rust/kernel/debugfs/traits.rs   |  33 +++++++++
 4 files changed, 350 insertions(+), 1 deletion(-)

rust/kernel/debugfs.rs +86 -1

index 65be71600b8eda83c0d313f3d205d0713e8e9510..b28665f58cd6a17e188aef5e8c539f6c7433a3b0 100644
@@ -8,12 +8,18 @@
 // When DebugFS is disabled, many parameters are dead. Linting for this isn't helpful.
 #![cfg_attr(not(CONFIG_DEBUG_FS), allow(unused_variables))]
 
-#[cfg(CONFIG_DEBUG_FS)]
 use crate::prelude::*;
 use crate::str::CStr;
 #[cfg(CONFIG_DEBUG_FS)]
 use crate::sync::Arc;
+use core::marker::PhantomPinned;
+use core::ops::Deref;
+
+mod traits;
+pub use traits::Writer;
 
+mod file_ops;
+use file_ops::{FileOps, ReadFile};
 #[cfg(CONFIG_DEBUG_FS)]
 mod entry;
 #[cfg(CONFIG_DEBUG_FS)]

@@ -53,6 +59,34 @@ fn create(name: &CStr, parent: Option<&Dir>) -> Self {
         Self()
     }
 
+    /// Creates a DebugFS file which will own the data produced by the initializer provided in
+    /// `data`.
+    fn create_file<'a, T, E: 'a>(
+        &'a self,
+        name: &'a CStr,
+        data: impl PinInit<T, E> + 'a,
+        file_ops: &'static FileOps<T>,
+    ) -> impl PinInit<File<T>, E> + 'a
+    where
+        T: Sync + 'static,
+    {
+        let scope = Scope::<T>::new(data, move |data| {
+            #[cfg(CONFIG_DEBUG_FS)]
+            if let Some(parent) = &self.0 {
+                // SAFETY: Because data derives from a scope, and our entry will be dropped before
+                // the data is dropped, it is guaranteed to outlive the entry we return.
+                unsafe { Entry::dynamic_file(name, parent.clone(), data, file_ops) }
+            } else {
+                Entry::empty()
+            }
+        });
+        try_pin_init! {
+            File {
+                scope <- scope
+            } ? E
+        }
+    }
+
     /// Create a new directory in DebugFS at the root.
     ///
     /// # Examples

@@ -79,4 +113,116 @@ pub fn new(name: &CStr) -> Self {
     pub fn subdir(&self, name: &CStr) -> Self {
         Dir::create(name, Some(self))
     }
+
+    /// Creates a read-only file in this directory.
+    ///
+    /// The file's contents are produced by invoking [`Writer::write`] on the value initialized by
+    /// `data`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// # use kernel::prelude::*;
+    /// # let dir = Dir::new(c_str!("my_debugfs_dir"));
+    /// let file = KBox::pin_init(dir.read_only_file(c_str!("foo"), 200), GFP_KERNEL)?;
+    /// // "my_debugfs_dir/foo" now contains the number 200.
+    /// // The file is removed when `file` is dropped.
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn read_only_file<'a, T, E: 'a>(
+        &'a self,
+        name: &'a CStr,
+        data: impl PinInit<T, E> + 'a,
+    ) -> impl PinInit<File<T>, E> + 'a
+    where
+        T: Writer + Send + Sync + 'static,
+    {
+        let file_ops = &<T as ReadFile<_>>::FILE_OPS;
+        self.create_file(name, data, file_ops)
+    }
+}
+
+#[pin_data]
+/// Handle to a DebugFS scope, which ensures that attached `data` will outlive the provided
+/// [`Entry`] without moving.
+/// Currently, this is used to back [`File`] so that its `read` and/or `write` implementations
+/// can assume that their backing data is still alive.
+struct Scope<T> {
+    // This order is load-bearing for drops - `_entry` must be dropped before `data`.
+    #[cfg(CONFIG_DEBUG_FS)]
+    _entry: Entry,
+    #[pin]
+    data: T,
+    // Even if `T` is `Unpin`, we still can't allow it to be moved.
+    #[pin]
+    _pin: PhantomPinned,
+}
+
+#[pin_data]
+/// Handle to a DebugFS file, owning its backing data.
+///
+/// When dropped, the DebugFS file will be removed and the attached data will be dropped.

Review

Quoted 5 lines · Matthew Maurer

(quoted diff) +5 -0

+#[cfg(not(CONFIG_DEBUG_FS))]
+impl<'b, T: 'b> Scope<T> {
+    fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
+    where
+        F: for<'a> FnOnce(&'a T) -> Entry + 'b,

Inspired by Greg's & Danilo's discussion I tried building with CONFIG_DEBUG_FS disabled. And get

error[E0412]: cannot find type Entry in this scope
--> rust/kernel/debugfs.rs:351:37

    |
351 |         F: for<'a> FnOnce(&'a T) -> Entry + 'b,
    |                                     ^^^^^ not found in this scope

And giving it some Entry (for my 1.81.0)

error: hidden lifetime parameters in types are deprecated
--> rust/kernel/debugfs.rs:352:37

    |
352 |         F: for<'a> FnOnce(&'a T) -> Entry + 'b,
    |                                     ^^^^^ expected lifetime parameter

Quoted 16 lines · Dirk Behme

Review

Quoted 5 lines · Matthew Maurer

(quoted diff) +5 -0

+#[cfg(not(CONFIG_DEBUG_FS))]
+impl<'b, T: 'b> Scope<T> {
+    fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
+    where
+        F: for<'a> FnOnce(&'a T) -> Entry + 'b,

Inspired by Greg's & Danilo's discussion I tried building with CONFIG_DEBUG_FS disabled. And get

error[E0412]: cannot find type Entry in this scope
--> rust/kernel/debugfs.rs:351:37

    |
351 |         F: for<'a> FnOnce(&'a T) -> Entry + 'b,
    |                                     ^^^^^ not found in this scope

Quoted 7 lines

And giving it some Entry (for my 1.81.0)

error: hidden lifetime parameters in types are deprecated
--> rust/kernel/debugfs.rs:352:37

    |
352 |         F: for<'a> FnOnce(&'a T) -> Entry + 'b,
    |                                     ^^^^^ expected lifetime parameter

Yeah, I caught this as well and fixed it up on my end with the following diff:

rust/kernel/debugfs.rs +0 -0

index ecfcce845d3f..1f25777743db 100644

Quoted 27 lines · Danilo Krummrich

Quoted 16 lines · Dirk Behme

Review

Quoted 5 lines · Matthew Maurer

(quoted diff) +5 -0

+#[cfg(not(CONFIG_DEBUG_FS))]
+impl<'b, T: 'b> Scope<T> {
+    fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
+    where
+        F: for<'a> FnOnce(&'a T) -> Entry + 'b,

Inspired by Greg's & Danilo's discussion I tried building with CONFIG_DEBUG_FS disabled. And get

error[E0412]: cannot find type Entry in this scope
--> rust/kernel/debugfs.rs:351:37

    |
351 |         F: for<'a> FnOnce(&'a T) -> Entry + 'b,
    |                                     ^^^^^ not found in this scope

Quoted 7 lines

And giving it some Entry (for my 1.81.0)

error: hidden lifetime parameters in types are deprecated
--> rust/kernel/debugfs.rs:352:37

    |
352 |         F: for<'a> FnOnce(&'a T) -> Entry + 'b,
    |                                     ^^^^^ expected lifetime parameter

Yeah, I caught this as well and fixed it up on my end with the following diff:

Quoted 6 lines

rust/kernel/debugfs.rs +0 -0

index ecfcce845d3f..1f25777743db 100644

rust/kernel/debugfs.rs pub struct File<T> { · line 354

Quoted 8 lines

rust/kernel/debugfs.rs +1 -1

@@ -348,7 +348,7 @@ pub struct File<T> {
 impl<'b, T: 'b> Scope<T> {
     fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
     where
-        F: for<'a> FnOnce(&'a T) -> Entry + 'b,
+        F: for<'a> FnOnce(&'a T) + 'b,
     {
         try_pin_init! {
             Self {

Can you send this as a fix-up patch?

Quoted 28 lines · Greg Kroah-Hartman

Quoted 27 lines · Danilo Krummrich

Quoted 16 lines · Dirk Behme

Review

Quoted 5 lines · Matthew Maurer

(quoted diff) +5 -0

+#[cfg(not(CONFIG_DEBUG_FS))]
+impl<'b, T: 'b> Scope<T> {
+    fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
+    where
+        F: for<'a> FnOnce(&'a T) -> Entry + 'b,

Inspired by Greg's & Danilo's discussion I tried building with CONFIG_DEBUG_FS disabled. And get

error[E0412]: cannot find type Entry in this scope
--> rust/kernel/debugfs.rs:351:37

    |
351 |         F: for<'a> FnOnce(&'a T) -> Entry + 'b,
    |                                     ^^^^^ not found in this scope

Quoted 7 lines

And giving it some Entry (for my 1.81.0)

error: hidden lifetime parameters in types are deprecated
--> rust/kernel/debugfs.rs:352:37

    |
352 |         F: for<'a> FnOnce(&'a T) -> Entry + 'b,
    |                                     ^^^^^ expected lifetime parameter

Yeah, I caught this as well and fixed it up on my end with the following diff:

Quoted 16 lines

Quoted 6 lines

rust/kernel/debugfs.rs +0 -0

index ecfcce845d3f..1f25777743db 100644

rust/kernel/debugfs.rs pub struct File<T> { · line 354

Quoted 8 lines

rust/kernel/debugfs.rs +1 -1

@@ -348,7 +348,7 @@ pub struct File<T> {
 impl<'b, T: 'b> Scope<T> {
     fn new<E: 'b, F>(data: impl PinInit<T, E> + 'b, init: F) -> impl PinInit<Self, E> + 'b
     where
-        F: for<'a> FnOnce(&'a T) -> Entry + 'b,
+        F: for<'a> FnOnce(&'a T) + 'b,
     {
         try_pin_init! {
             Self {

Can you send this as a fix-up patch?

If you don't mind I would fix this (and one other nit) up on apply. :)

I've pushed these to driver-core-testing if you could fix it up as I don't know what the other change was?

Quoted 2 lines · Greg Kroah-Hartman

I've pushed these to driver-core-testing if you could fix it up as I don't know what the other change was?

Sure, will do.

+ const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);

samples/rust/rust_debugfs.rs line 98

Quoted 5 lines · Matthew Maurer

samples/rust/rust_debugfs.rs new file +5 -0

+kernel::acpi_device_table!(
+    ACPI_TABLE,
+    MODULE_ACPI_TABLE,
+    <RustDebugFs as platform::Driver>::IdInfo,
+    [(acpi::DeviceId::new(c_str!("LNUXDEBF")), ())]

This should use "LNUXBEEF", as we explicitly reserved it for sample and test code.

I think we could reserve more if we see a benefit, but so far it's only used by the platform driver sample, so we should be good.

Either way, no need to resend for this only, it can be fixed up on apply. :)

samples/rust/rust_debugfs.rs line 103

Quoted 6 lines

samples/rust/rust_debugfs.rs new file +5 -0

+);
+
+impl platform::Driver for RustDebugFs {
+    type IdInfo = ();
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;

NIT: This defaults to None, so it can be omitted.

Quoted 1 line

+ const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);

Quoted 11 lines · Danilo Krummrich

samples/rust/rust_debugfs.rs line 98

Quoted 5 lines · Matthew Maurer

samples/rust/rust_debugfs.rs new file +5 -0

+kernel::acpi_device_table!(
+    ACPI_TABLE,
+    MODULE_ACPI_TABLE,
+    <RustDebugFs as platform::Driver>::IdInfo,
+    [(acpi::DeviceId::new(c_str!("LNUXDEBF")), ())]

This should use "LNUXBEEF", as we explicitly reserved it for sample and test code.

Quoted 4 lines

I think we could reserve more if we see a benefit, but so far it's only used by the platform driver sample, so we should be good.

Either way, no need to resend for this only, it can be fixed up on apply. :)

Ah, thanks, I was unaware that it was specially reserved and so tried to pick something distinct. You fixing up this and the OF_ID_TABLE on apply SGTM.

Adds a new sample driver that demonstrates the debugfs APIs.

The driver creates a directory in debugfs and populates it with a few files:
- A read-only file that displays a fwnode property.
- A read-write file that exposes an atomic counter.
- A read-write file that exposes a custom struct.

This sample serves as a basic example of how to use the debugfs::Dir and debugfs::File APIs to create and manage debugfs entries.

Signed-off-by Matthew Maurer <[email protected]>

---
 MAINTAINERS                  |   1 +
 samples/rust/Kconfig         |  11 ++++
 samples/rust/Makefile        |   1 +
 samples/rust/rust_debugfs.rs | 151 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 164 insertions(+)

MAINTAINERS +1 -0

index 8f2dbf71ca3f8f97e4d7619375279ed11d1261b2..5b6db584a33dd7ee39de3fdd0085d2bd7b7bef0e 100644
@@ -7481,6 +7481,7 @@ F:	rust/kernel/devres.rs
 F:	rust/kernel/driver.rs
 F:	rust/kernel/faux.rs
 F:	rust/kernel/platform.rs
+F:	samples/rust/rust_debugfs.rs
 F:	samples/rust/rust_driver_platform.rs
 F:	samples/rust/rust_driver_faux.rs
 

samples/rust/Kconfig +11 -0

index 7f7371a004ee0a8f67dca99c836596709a70c4fa..01101db41ae31b08a86d048cdd27da8ef9bb23a2 100644
@@ -62,6 +62,17 @@ config SAMPLE_RUST_DMA
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_DEBUGFS
+	tristate "DebugFS Test Module"
+	depends on DEBUG_FS
+	help
+	  This option builds the Rust DebugFS Test module sample.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_debugfs.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_DRIVER_PCI
 	tristate "PCI Driver"
 	depends on PCI

samples/rust/Makefile +1 -0

index bd2faad63b4f3befe7d1ed5139fe25c7a8b6d7f6..61276222a99f8cc6d7f84c26d0533b30815ebadd 100644
@@ -4,6 +4,7 @@ ccflags-y += -I$(src)				# needed for trace events
 obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
 obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE)		+= rust_misc_device.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
+obj-$(CONFIG_SAMPLE_RUST_DEBUGFS)		+= rust_debugfs.o
 obj-$(CONFIG_SAMPLE_RUST_DMA)			+= rust_dma.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI)		+= rust_driver_pci.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM)	+= rust_driver_platform.o

samples/rust/rust_debugfs.rs new file +54 -0

new file mode 100644
index 0000000000000000000000000000000000000000..8d394f06b37e69ea1c30a3b0d8444c80562cc5ab
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Google LLC.
+
+//! Sample DebugFS exporting platform driver
+//!
+//! To successfully probe this driver with ACPI, use an ssdt that looks like
+//!
+//! ```dsl
+//! DefinitionBlock ("", "SSDT", 2, "TEST", "VIRTACPI", 0x00000001)
+//! {
+//!    Scope (\_SB)
+//!    {
+//!        Device (T432)
+//!        {
+//!            Name (_HID, "LNUXDEBF")  // ACPI hardware ID to match
+//!            Name (_UID, 1)
+//!            Name (_STA, 0x0F)        // Device present, enabled
+//!            Name (_DSD, Package () { // Sample attribute
+//!                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+//!                Package() {
+//!                    Package(2) {"compatible", "sample-debugfs"}
+//!                }
+//!            })
+//!            Name (_CRS, ResourceTemplate ()
+//!            {
+//!                Memory32Fixed (ReadWrite, 0xFED00000, 0x1000)
+//!            })
+//!        }
+//!    }
+//! }
+//! ```
+
+use core::str::FromStr;
+use core::sync::atomic::AtomicUsize;
+use core::sync::atomic::Ordering;
+use kernel::c_str;
+use kernel::debugfs::{Dir, File};
+use kernel::new_mutex;
+use kernel::prelude::*;
+use kernel::sync::Mutex;
+
+use kernel::{acpi, device::Core, of, platform, str::CString, types::ARef};
+
+kernel::module_platform_driver! {
+    type: RustDebugFs,
+    name: "rust_debugfs",
+    authors: ["Matthew Maurer"],
+    description: "Rust DebugFS usage sample",
+    license: "GPL",
+}
+
+#[pin_data]
+struct RustDebugFs {

Quoted 131 lines · Matthew Maurer

Adds a new sample driver that demonstrates the debugfs APIs.

The driver creates a directory in debugfs and populates it with a few files:
- A read-only file that displays a fwnode property.
- A read-write file that exposes an atomic counter.
- A read-write file that exposes a custom struct.

This sample serves as a basic example of how to use the debugfs::Dir and debugfs::File APIs to create and manage debugfs entries.

Signed-off-by Matthew Maurer <[email protected]>

---
 MAINTAINERS                  |   1 +
 samples/rust/Kconfig         |  11 ++++
 samples/rust/Makefile        |   1 +
 samples/rust/rust_debugfs.rs | 151 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 164 insertions(+)

MAINTAINERS +1 -0

index 8f2dbf71ca3f8f97e4d7619375279ed11d1261b2..5b6db584a33dd7ee39de3fdd0085d2bd7b7bef0e 100644
@@ -7481,6 +7481,7 @@ F:	rust/kernel/devres.rs
 F:	rust/kernel/driver.rs
 F:	rust/kernel/faux.rs
 F:	rust/kernel/platform.rs
+F:	samples/rust/rust_debugfs.rs
 F:	samples/rust/rust_driver_platform.rs
 F:	samples/rust/rust_driver_faux.rs
 

samples/rust/Kconfig +11 -0

index 7f7371a004ee0a8f67dca99c836596709a70c4fa..01101db41ae31b08a86d048cdd27da8ef9bb23a2 100644
@@ -62,6 +62,17 @@ config SAMPLE_RUST_DMA
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_DEBUGFS
+	tristate "DebugFS Test Module"
+	depends on DEBUG_FS
+	help
+	  This option builds the Rust DebugFS Test module sample.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_debugfs.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_DRIVER_PCI
 	tristate "PCI Driver"
 	depends on PCI

samples/rust/Makefile +1 -0

index bd2faad63b4f3befe7d1ed5139fe25c7a8b6d7f6..61276222a99f8cc6d7f84c26d0533b30815ebadd 100644
@@ -4,6 +4,7 @@ ccflags-y += -I$(src)				# needed for trace events
 obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
 obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE)		+= rust_misc_device.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
+obj-$(CONFIG_SAMPLE_RUST_DEBUGFS)		+= rust_debugfs.o
 obj-$(CONFIG_SAMPLE_RUST_DMA)			+= rust_dma.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI)		+= rust_driver_pci.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM)	+= rust_driver_platform.o

samples/rust/rust_debugfs.rs new file +60 -0

new file mode 100644
index 0000000000000000000000000000000000000000..8d394f06b37e69ea1c30a3b0d8444c80562cc5ab
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Google LLC.
+
+//! Sample DebugFS exporting platform driver
+//!
+//! To successfully probe this driver with ACPI, use an ssdt that looks like
+//!
+//! ```dsl
+//! DefinitionBlock ("", "SSDT", 2, "TEST", "VIRTACPI", 0x00000001)
+//! {
+//!    Scope (\_SB)
+//!    {
+//!        Device (T432)
+//!        {
+//!            Name (_HID, "LNUXDEBF")  // ACPI hardware ID to match
+//!            Name (_UID, 1)
+//!            Name (_STA, 0x0F)        // Device present, enabled
+//!            Name (_DSD, Package () { // Sample attribute
+//!                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+//!                Package() {
+//!                    Package(2) {"compatible", "sample-debugfs"}
+//!                }
+//!            })
+//!            Name (_CRS, ResourceTemplate ()
+//!            {
+//!                Memory32Fixed (ReadWrite, 0xFED00000, 0x1000)
+//!            })
+//!        }
+//!    }
+//! }
+//! ```
+
+use core::str::FromStr;
+use core::sync::atomic::AtomicUsize;
+use core::sync::atomic::Ordering;
+use kernel::c_str;
+use kernel::debugfs::{Dir, File};
+use kernel::new_mutex;
+use kernel::prelude::*;
+use kernel::sync::Mutex;
+
+use kernel::{acpi, device::Core, of, platform, str::CString, types::ARef};
+
+kernel::module_platform_driver! {
+    type: RustDebugFs,
+    name: "rust_debugfs",
+    authors: ["Matthew Maurer"],
+    description: "Rust DebugFS usage sample",
+    license: "GPL",
+}
+
+#[pin_data]
+struct RustDebugFs {
+    pdev: ARef<platform::Device>,
+    // As we only hold these for drop effect (to remove the directory/files) we have a leading
+    // underscore to indicate to the compiler that we don't expect to use this field directly.
+    _debugfs: Dir,
+    #[pin]
+    _compatible: File<CString>,

samples/rust/rust_debugfs.rs line 64

Quoted 4 lines

samples/rust/rust_debugfs.rs new file +4 -0

+    #[pin]
+    counter: File<AtomicUsize>,
+    #[pin]
+    inner: File<Mutex<Inner>>,

Why are you saving the file pointers here at all? They shouldn't be needed to keep around, all that should be needed is the Dir, right? If not, this needs to be revisited...

Quoted 10 lines

samples/rust/rust_debugfs.rs new file +10 -0

+impl platform::Driver for RustDebugFs {
+    type IdInfo = ();
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
+    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
+
+    fn probe(
+        pdev: &platform::Device<Core>,
+        _info: Option<&Self::IdInfo>,
+    ) -> Result<Pin<KBox<Self>>> {
+        let result = KBox::try_pin_init(RustDebugFs::new(pdev), GFP_KERNEL)?;

samples/rust/rust_debugfs.rs line 112

Quoted 2 lines

samples/rust/rust_debugfs.rs new file +2 -0

+        // We can still mutate fields through the files which are atomic or mutexed:
+        result.counter.store(91, Ordering::Relaxed);

Here? This feels odd, the file is the wrapper for the variable you want the file to show? That feels backwards. Usually the variable is always present, and then you can create a debugfs file to read/modify it. But if debugfs is NOT enabled, you can still use the variable, it's just the file logic that is gone, right?

So what would this file look like without debugfs being enabled? Would the atomic variable still be here? I think it wouldn't, which is not ok...

Adds a new sample driver rust_scoped_debugfs that demonstrates the use of the scoped debugfs APIs.

The driver creates a control directory with two write-only files, create and remove. Writing a name and a series of numbers to create will create a new subdirectory under a dynamic directory. This new subdirectory will contain files that expose the numbers as atomic values.

Writing a name to remove will remove the corresponding subdirectory from the dynamic directory.

This sample serves as an example of how to use the debugfs::Scope and debugfs::ScopedDir APIs to create and manage debugfs entries that are tied to the lifetime of a data structure.

Signed-off-by Matthew Maurer <[email protected]>

---
 MAINTAINERS                         |   1 +
 samples/rust/Kconfig                |  11 +++
 samples/rust/Makefile               |   1 +
 samples/rust/rust_scoped_debugfs.rs | 134 ++++++++++++++++++++++++++++++++++++

This series provides safe DebugFS bindings for Rust, with sample modules using them.

Signed-off-by Matthew Maurer <[email protected]>