| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2025090838-twelve-snap-683a@gregkh>
Date: Mon, 8 Sep 2025 15:08:40 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Matthew Maurer <mmaurer@...gle.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Sami Tolvanen <samitolvanen@...gle.com>,
Timur Tabi <ttabi@...dia.com>, Benno Lossin <lossin@...nel.org>,
Dirk Beheme <dirk.behme@...bosch.com>, linux-kernel@...r.kernel.org,
rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v11 5/7] samples: rust: Add debugfs sample driver
On Thu, Sep 04, 2025 at 09:13:56PM +0000, Matthew Maurer wrote:
> 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 <mmaurer@...gle.com>
> ---
> MAINTAINERS | 1 +
> samples/rust/Kconfig | 11 ++++
> samples/rust/Makefile | 1 +
> samples/rust/rust_debugfs.rs | 151 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 164 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8f2dbf71ca3f8f97e4d7619375279ed11d1261b2..5b6db584a33dd7ee39de3fdd0085d2bd7b7bef0e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -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
>
> diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> index 7f7371a004ee0a8f67dca99c836596709a70c4fa..01101db41ae31b08a86d048cdd27da8ef9bb23a2 100644
> --- a/samples/rust/Kconfig
> +++ b/samples/rust/Kconfig
> @@ -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
> diff --git a/samples/rust/Makefile b/samples/rust/Makefile
> index bd2faad63b4f3befe7d1ed5139fe25c7a8b6d7f6..61276222a99f8cc6d7f84c26d0533b30815ebadd 100644
> --- a/samples/rust/Makefile
> +++ b/samples/rust/Makefile
> @@ -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
> diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..8d394f06b37e69ea1c30a3b0d8444c80562cc5ab
> --- /dev/null
> +++ b/samples/rust/rust_debugfs.rs
> @@ -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>,
> + #[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...
And why do you need to keep "counter"? Shouldn't that be passed back
somehow in the callback automatically?
You use it:
> +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)?;
> + // 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...
Or am I confused again?
thanks,
greg k-h
Powered by blists - more mailing lists