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 <mmaurer@google.com>
---
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(-)
diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index 65be71600b8eda83c0d313f3d205d0713e8e9510..b28665f58cd6a17e188aef5e8c539f6c7433a3b0 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -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
+ }
}
diff --git a/rust/kernel/debugfs/entry.rs b/rust/kernel/debugfs/entry.rs
index d2fba0e65e20e954e2a33e776b872bac4adb12e8..227fa50b7a79aeab49779e54b8c4241f455777c3 100644
--- a/rust/kernel/debugfs/entry.rs
+++ b/rust/kernel/debugfs/entry.rs
@@ -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
diff --git a/rust/kernel/debugfs/file_ops.rs b/rust/kernel/debugfs/file_ops.rs
new file mode 100644
index 0000000000000000000000000000000000000000..c2fbef96580eaa2fab7cc8c1ba559c3284d12e1b
--- /dev/null
+++ b/rust/kernel/debugfs/file_ops.rs
@@ -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) }
+ };
+}
diff --git a/rust/kernel/debugfs/traits.rs b/rust/kernel/debugfs/traits.rs
new file mode 100644
index 0000000000000000000000000000000000000000..0e6e461324de42a3d80b692264d50e78a48f561d
--- /dev/null
+++ b/rust/kernel/debugfs/traits.rs
@@ -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> {
+ fn write(&self, f: &mut Formatter<'_>) -> fmt::Result {
+ self.lock().write(f)
+ }
+}
+
+impl<T: Debug> Writer for T {
+ fn write(&self, f: &mut Formatter<'_>) -> fmt::Result {
+ writeln!(f, "{self:?}")
+ }
+}
--
2.51.0.355.g5224444f11-googOn 04/09/2025 23:13, Matthew Maurer wrote:
> 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 <mmaurer@google.com>
> ---
> 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(-)
>
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index 65be71600b8eda83c0d313f3d205d0713e8e9510..b28665f58cd6a17e188aef5e8c539f6c7433a3b0 100644
> --- a/rust/kernel/debugfs.rs
> +++ b/rust/kernel/debugfs.rs
> @@ -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,
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
Dirk
On Tue Sep 9, 2025 at 9:29 AM CEST, Dirk Behme wrote:
> On 04/09/2025 23:13, Matthew Maurer wrote:
>> +#[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
Yeah, I caught this as well and fixed it up on my end with the following diff:
diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index ecfcce845d3f..1f25777743db 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -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 {
On Tue, Sep 09, 2025 at 10:29:13AM +0200, Danilo Krummrich wrote:
> On Tue Sep 9, 2025 at 9:29 AM CEST, Dirk Behme wrote:
> > On 04/09/2025 23:13, Matthew Maurer wrote:
> >> +#[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
>
> Yeah, I caught this as well and fixed it up on my end with the following diff:
>
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index ecfcce845d3f..1f25777743db 100644
> --- a/rust/kernel/debugfs.rs
> +++ b/rust/kernel/debugfs.rs
> @@ -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?
thanks,
greg k-h
On 9/10/25 5:22 PM, Greg Kroah-Hartman wrote:
> On Tue, Sep 09, 2025 at 10:29:13AM +0200, Danilo Krummrich wrote:
>> On Tue Sep 9, 2025 at 9:29 AM CEST, Dirk Behme wrote:
>>> On 04/09/2025 23:13, Matthew Maurer wrote:
>>>> +#[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
>>
>> Yeah, I caught this as well and fixed it up on my end with the following diff:
>>
>> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
>> index ecfcce845d3f..1f25777743db 100644
>> --- a/rust/kernel/debugfs.rs
>> +++ b/rust/kernel/debugfs.rs
>> @@ -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. :)
On Wed, Sep 10, 2025 at 05:23:26PM +0200, Danilo Krummrich wrote:
> On 9/10/25 5:22 PM, Greg Kroah-Hartman wrote:
> > On Tue, Sep 09, 2025 at 10:29:13AM +0200, Danilo Krummrich wrote:
> >> On Tue Sep 9, 2025 at 9:29 AM CEST, Dirk Behme wrote:
> >>> On 04/09/2025 23:13, Matthew Maurer wrote:
> >>>> +#[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
> >>
> >> Yeah, I caught this as well and fixed it up on my end with the following diff:
> >>
> >> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> >> index ecfcce845d3f..1f25777743db 100644
> >> --- a/rust/kernel/debugfs.rs
> >> +++ b/rust/kernel/debugfs.rs
> >> @@ -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?
thanks,
greg k-h
On 9/10/25 5:36 PM, Greg Kroah-Hartman wrote: > 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.
On Wed Sep 10, 2025 at 5:43 PM CEST, Danilo Krummrich wrote:
> On 9/10/25 5:36 PM, Greg Kroah-Hartman wrote:
>> 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.
* rust: debugfs: Add initial support for directories
* rust: debugfs: Add support for read-only files
[ Fixup build failure when CONFIG_DEBUGFS=n. - Danilo ]
* rust: debugfs: Add support for writable files
[ Fix up Result<()> -> Result. - Danilo ]
* rust: debugfs: Add support for callback-based files
[ Fix up Result<(), Error> -> Result. - Danilo ]
* samples: rust: Add debugfs sample driver
[ Change ACPI ID "LNUXDEBF" to "LNUXBEEF". - Danilo ]
* rust: debugfs: Add support for scoped directories
[ Fix up Result<(), Error> -> Result; fix spurious backtick in
doc-comment. - Danilo ]
* samples: rust: Add scoped debugfs sample driver
[ Rename "scoped_debugfs" -> "debugfs_scoped", fix up
Result<(), Error> -> Result. - Danilo ]
On Thu, Sep 04, 2025 at 09:13:53PM +0000, Matthew Maurer wrote:
> 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 <mmaurer@google.com>
> ---
> 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(-)
>
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index 65be71600b8eda83c0d313f3d205d0713e8e9510..b28665f58cd6a17e188aef5e8c539f6c7433a3b0 100644
> --- a/rust/kernel/debugfs.rs
> +++ b/rust/kernel/debugfs.rs
> @@ -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
> + }
> }
> diff --git a/rust/kernel/debugfs/entry.rs b/rust/kernel/debugfs/entry.rs
> index d2fba0e65e20e954e2a33e776b872bac4adb12e8..227fa50b7a79aeab49779e54b8c4241f455777c3 100644
> --- a/rust/kernel/debugfs/entry.rs
> +++ b/rust/kernel/debugfs/entry.rs
> @@ -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
> diff --git a/rust/kernel/debugfs/file_ops.rs b/rust/kernel/debugfs/file_ops.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..c2fbef96580eaa2fab7cc8c1ba559c3284d12e1b
> --- /dev/null
> +++ b/rust/kernel/debugfs/file_ops.rs
> @@ -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) }
> + };
> +}
> diff --git a/rust/kernel/debugfs/traits.rs b/rust/kernel/debugfs/traits.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..0e6e461324de42a3d80b692264d50e78a48f561d
> --- /dev/null
> +++ b/rust/kernel/debugfs/traits.rs
> @@ -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> {
> + fn write(&self, f: &mut Formatter<'_>) -> fmt::Result {
> + self.lock().write(f)
> + }
> +}
> +
> +impl<T: Debug> Writer for T {
> + fn write(&self, f: &mut Formatter<'_>) -> fmt::Result {
> + writeln!(f, "{self:?}")
> + }
> +}
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?
thanks,
greg k-h
On Mon Sep 8, 2025 at 12:17 PM CEST, Greg Kroah-Hartman wrote:
> 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?
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:
diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
index b26eea3ee723..475502f30b1a 100644
--- a/samples/rust/rust_debugfs.rs
+++ b/samples/rust/rust_debugfs.rs
@@ -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,
On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
> On Mon Sep 8, 2025 at 12:17 PM CEST, Greg Kroah-Hartman wrote:
> > 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?
>
> What you're doing should fundamentally work.
>
> The above error suggests that your declaration of `_file` is File<&u32> rather
> than File<&'static CStr>.
Ah, ick, I missed that the return type would be different here. Yes, I
was doing a bunch of file creation calls:
let mut _file = root.read_only_file(c_str!("id"), &hw_soc_info.id);
_file = root.read_only_file(c_str!("ver"), &hw_soc_info.ver);
_file = root.read_only_file(c_str!("raw_id"), &hw_soc_info.raw_id);
_file = root.read_only_file(c_str!("name"), &hw_soc_info.name);
As I don't care about the return value here at all.
But really, I should just write this as:
root.read_only_file(c_str!("id"), &hw_soc_info.id);
root.read_only_file(c_str!("ver"), &hw_soc_info.ver);
root.read_only_file(c_str!("raw_id"), &hw_soc_info.raw_id);
root.read_only_file(c_str!("name"), hw_soc_info.name);
with, as you point out:
> Also note the double reference you create with `&hw_soc_info.name`, this should
> just be `hw_soc_info.name`.
Yes, sorry, my fault there.
> You can also test this case by applying the following diff the the sample in v5:
>
> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
> index b26eea3ee723..475502f30b1a 100644
> --- a/samples/rust/rust_debugfs.rs
> +++ b/samples/rust/rust_debugfs.rs
> @@ -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")),
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.
thanks,
greg k-h
On Mon Sep 8, 2025 at 2:48 PM CEST, Greg Kroah-Hartman wrote:
> On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
>> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
>> index b26eea3ee723..475502f30b1a 100644
>> --- a/samples/rust/rust_debugfs.rs
>> +++ b/samples/rust/rust_debugfs.rs
>> @@ -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")),
>
> 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.
On Mon, Sep 08, 2025 at 03:22:41PM +0200, Danilo Krummrich wrote:
> On Mon Sep 8, 2025 at 2:48 PM CEST, Greg Kroah-Hartman wrote:
> > On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
> >> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
> >> index b26eea3ee723..475502f30b1a 100644
> >> --- a/samples/rust/rust_debugfs.rs
> >> +++ b/samples/rust/rust_debugfs.rs
> >> @@ -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")),
> >
> > 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.
Ah, that's exactly what I was trying to do.
> 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);
> }
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.
> 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:
> 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.)
> 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.
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.
thanks,
greg k-h
On Mon Sep 8, 2025 at 3:30 PM CEST, Greg Kroah-Hartman wrote:
> On Mon, Sep 08, 2025 at 03:22:41PM +0200, Danilo Krummrich wrote:
>> On Mon Sep 8, 2025 at 2:48 PM CEST, Greg Kroah-Hartman wrote:
>> > On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
>> >> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
>> >> index b26eea3ee723..475502f30b1a 100644
>> >> --- a/samples/rust/rust_debugfs.rs
>> >> +++ b/samples/rust/rust_debugfs.rs
>> >> @@ -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")),
>> >
>> > 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.
>
> 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.
>> 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);
>> }
>
> 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.
>> 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:
>
>> 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.)
>
>> 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.
>
> 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.
On Mon, Sep 08, 2025 at 03:36:46PM +0200, Danilo Krummrich wrote:
> On Mon Sep 8, 2025 at 3:30 PM CEST, Greg Kroah-Hartman wrote:
> > On Mon, Sep 08, 2025 at 03:22:41PM +0200, Danilo Krummrich wrote:
> >> On Mon Sep 8, 2025 at 2:48 PM CEST, Greg Kroah-Hartman wrote:
> >> > On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
> >> >> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
> >> >> index b26eea3ee723..475502f30b1a 100644
> >> >> --- a/samples/rust/rust_debugfs.rs
> >> >> +++ b/samples/rust/rust_debugfs.rs
> >> >> @@ -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")),
> >> >
> >> > 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.
> >
> > 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.
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?
> You don't want a a field to be reference counted just because it's exposed via
> debugfs.
Exactly, the data is the thing driving this, not the debugfs file.
> >> 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);
> >> }
> >
> > 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.
See above for my lack of understanding of that :)
> > 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.
Ugh, but really, that's very unworkable overall. While I see the logic
here, it's making the debugfs interface be the "main" one, when really
that is just an afterthought and is NOT the thing to focus on at all.
Again, debugfs is just "on the side for debugging", let's not force it
to be the way that data is accessed within the kernel itself, like is
being done with the wrapping of File<T> here.
thanks,
greg k-h
On Mon Sep 8, 2025 at 4:16 PM CEST, Greg Kroah-Hartman wrote:
> On Mon, Sep 08, 2025 at 03:36:46PM +0200, Danilo Krummrich wrote:
>> On Mon Sep 8, 2025 at 3:30 PM CEST, Greg Kroah-Hartman wrote:
>> > On Mon, Sep 08, 2025 at 03:22:41PM +0200, Danilo Krummrich wrote:
>> >> On Mon Sep 8, 2025 at 2:48 PM CEST, Greg Kroah-Hartman wrote:
>> >> > On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
>> >> >> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
>> >> >> index b26eea3ee723..475502f30b1a 100644
>> >> >> --- a/samples/rust/rust_debugfs.rs
>> >> >> +++ b/samples/rust/rust_debugfs.rs
>> >> >> @@ -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")),
>> >> >
>> >> > 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.
>> >
>> > 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.
>
> 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.
> 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.
> 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:
> On Mon Sep 8, 2025 at 4:16 PM CEST, Greg Kroah-Hartman wrote:
> > On Mon, Sep 08, 2025 at 03:36:46PM +0200, Danilo Krummrich wrote:
> >> On Mon Sep 8, 2025 at 3:30 PM CEST, Greg Kroah-Hartman wrote:
> >> > On Mon, Sep 08, 2025 at 03:22:41PM +0200, Danilo Krummrich wrote:
> >> >> On Mon Sep 8, 2025 at 2:48 PM CEST, Greg Kroah-Hartman wrote:
> >> >> > On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
> >> >> >> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
> >> >> >> index b26eea3ee723..475502f30b1a 100644
> >> >> >> --- a/samples/rust/rust_debugfs.rs
> >> >> >> +++ b/samples/rust/rust_debugfs.rs
> >> >> >> @@ -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")),
> >> >> >
> >> >> > 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.
> >> >
> >> > 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.
> >
> > 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.
>
> > 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.
>
> > 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). :)
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.
thanks,
greg k-h
On 9/8/25 6:19 PM, Greg Kroah-Hartman wrote: > 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. I think our examples were pretty close already, but here's also your file. :)
On Mon Sep 8, 2025 at 6:19 PM CEST, Greg Kroah-Hartman wrote:
> On Mon, Sep 08, 2025 at 04:59:16PM +0200, Danilo Krummrich wrote:
<snip>
>> 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>
>> 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 })
}
}
On Mon Sep 8, 2025 at 6:30 PM CEST, Danilo Krummrich wrote:
> On Mon Sep 8, 2025 at 6:19 PM CEST, Greg Kroah-Hartman wrote:
>> On Mon, Sep 08, 2025 at 04:59:16PM +0200, Danilo Krummrich wrote:
>
> <snip>
>
>>> 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>
>
>>> 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>>>,
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.
> }
>
> 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 })
> }
> }
On Mon, Sep 08, 2025 at 06:55:48PM +0200, Danilo Krummrich wrote:
> On Mon Sep 8, 2025 at 6:30 PM CEST, Danilo Krummrich wrote:
> > On Mon Sep 8, 2025 at 6:19 PM CEST, Greg Kroah-Hartman wrote:
> >> On Mon, Sep 08, 2025 at 04:59:16PM +0200, Danilo Krummrich wrote:
> >
> > <snip>
> >
> >>> 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>
> >
> >>> 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>>>,
>
> 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.
Thanks for the example (and the rewrite of my example.)
This makes more sense, but I'm worried that this puts debugfs "front and
center" for data structures that normally had nothing to do with debugfs
at all.
So I took at look at the USB and PCI drivers to see what they do as
those are "real-world" users of debugfs, not just "soc info" stuff, and
it turns out that you are right. The use of debugfs is normally NOT
using just simple wrappers around variables like our examples are doing,
but are explicit "read/write" type things that do stuff to a structure,
AND those structures need to be properly referenced, so that things do
not go wrong when structures are removed.
So, after digging through this some more today, I think I'm ok with it.
Let's merge this now, and let's see how it gets used and if it's
unwieldy, hey, we can always change it!
thanks,
greg k-h
On Mon, Sep 8, 2025 at 3:30 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Sep 08, 2025 at 03:22:41PM +0200, Danilo Krummrich wrote:
> > On Mon Sep 8, 2025 at 2:48 PM CEST, Greg Kroah-Hartman wrote:
> > > On Mon, Sep 08, 2025 at 12:54:46PM +0200, Danilo Krummrich wrote:
> > >> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
> > >> index b26eea3ee723..475502f30b1a 100644
> > >> --- a/samples/rust/rust_debugfs.rs
> > >> +++ b/samples/rust/rust_debugfs.rs
> > >> @@ -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")),
> > >
> > > 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.
>
> Ah, that's exactly what I was trying to do.
>
> > 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);
> > }
>
> 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.
>
> > 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:
>
> > 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.
Alice
On Mon Sep 8, 2025 at 3:34 PM CEST, Alice Ryhl wrote: > 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. Yeah, I think it's the correct solution to make those cases nicer.
On Mon Sep 8, 2025 at 12:54 PM CEST, Danilo Krummrich wrote:
> On Mon Sep 8, 2025 at 12:17 PM CEST, Greg Kroah-Hartman wrote:
>> 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?
>
> 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:
*sample in patch 5
>
> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
> index b26eea3ee723..475502f30b1a 100644
> --- a/samples/rust/rust_debugfs.rs
> +++ b/samples/rust/rust_debugfs.rs
> @@ -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,
© 2016 - 2025 Red Hat, Inc.