linux-kernel - Re: [PATCH 7/7] rust: sample: misc: implement device driver sample

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DA9RRZVPZSMW.1LGW9H4G0RLT5@kernel.org>
Date: Fri, 30 May 2025 22:15:37 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Danilo Krummrich" <dakr@...nel.org>, <gregkh@...uxfoundation.org>,
 <rafael@...nel.org>, <ojeda@...nel.org>, <alex.gaynor@...il.com>,
 <boqun.feng@...il.com>, <gary@...yguo.net>, <bjorn3_gh@...tonmail.com>,
 <benno.lossin@...ton.me>, <a.hindborg@...nel.org>, <aliceryhl@...gle.com>,
 <tmgross@...ch.edu>, <chrisi.schrefl@...il.com>
Cc: <rust-for-linux@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 7/7] rust: sample: misc: implement device driver sample

On Fri May 30, 2025 at 4:24 PM CEST, Danilo Krummrich wrote:
> In order to demonstrate and test a MiscDeviceRegistration with a parent
> device, introduce CONFIG_SAMPLE_RUST_MISC_DEVICE_WITH_PARENT.
>
> If CONFIG_SAMPLE_RUST_MISC_DEVICE_WITH_PARENT=y the misc device sample
> is initialized with a parent device (faux), otherwise it is initialized
> without a parent device, i.e. the exact same way as without this patch.
>
> Signed-off-by: Danilo Krummrich <dakr@...nel.org>
> ---
>  samples/rust/Kconfig             |  8 +++++
>  samples/rust/rust_misc_device.rs | 50 +++++++++++++++++++++++++++++---
>  2 files changed, 54 insertions(+), 4 deletions(-)
>
> diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> index b1006ab4bc3c..9948ec0939ef 100644
> --- a/samples/rust/Kconfig
> +++ b/samples/rust/Kconfig
> @@ -30,6 +30,14 @@ config SAMPLE_RUST_MISC_DEVICE
>  
>  	  If unsure, say N.
>  
> +config SAMPLE_RUST_MISC_DEVICE_WITH_PARENT
> +	bool "Create a misc device with a parent device"
> +	depends on SAMPLE_RUST_MISC_DEVICE
> +	default n
> +	help
> +	  Say Y here if you want the misc device sample to create a misc
> +	  device with a parent device.
> +

Why not create a separate file? The `cfg`s might confuse newcomers
looking at the sample.

>  config SAMPLE_RUST_PRINT
>  	tristate "Printing macros"
>  	help
> diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
> index 9bf1a0f64e6e..175638d6d341 100644
> --- a/samples/rust/rust_misc_device.rs
> +++ b/samples/rust/rust_misc_device.rs
> @@ -167,6 +167,9 @@
>      uaccess::{UserSlice, UserSliceReader, UserSliceWriter},
>  };
>  
> +#[cfg(CONFIG_SAMPLE_RUST_MISC_DEVICE_WITH_PARENT)]
> +use kernel::faux;
> +
>  const RUST_MISC_DEV_HELLO: u32 = _IO('|' as u32, 0x80);
>  const RUST_MISC_DEV_GET_VALUE: u32 = _IOR::<i32>('|' as u32, 0x81);
>  const RUST_MISC_DEV_SET_VALUE: u32 = _IOW::<i32>('|' as u32, 0x82);
> @@ -181,19 +184,33 @@
>      license: "GPL",
>  }
>  
> +#[cfg(not(CONFIG_SAMPLE_RUST_MISC_DEVICE_WITH_PARENT))]
>  #[pin_data]
>  struct RustMiscDeviceModule {
>      #[pin]
>      _miscdev: MiscDeviceRegistration<RustMiscDevice>,
>  }
>  
> -impl kernel::InPlaceModule for RustMiscDeviceModule {
> -    fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
> +#[cfg(CONFIG_SAMPLE_RUST_MISC_DEVICE_WITH_PARENT)]
> +struct RustMiscDeviceModule {
> +    _faux: faux::Registration,
> +    _miscdev: Pin<KBox<MiscDeviceRegistration<RustMiscDevice>>>,
> +}
> +
> +impl RustMiscDeviceModule {
> +    fn init() -> MiscDeviceOptions {
>          pr_info!("Initializing Rust Misc Device Sample\n");
>  
> -        let options = MiscDeviceOptions {
> +        MiscDeviceOptions {
>              name: c_str!("rust-misc-device"),
> -        };
> +        }
> +    }
> +}
> +
> +#[cfg(not(CONFIG_SAMPLE_RUST_MISC_DEVICE_WITH_PARENT))]
> +impl kernel::InPlaceModule for RustMiscDeviceModule {
> +    fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
> +        let options = Self::init();
>  
>          try_pin_init!(Self {
>              _miscdev <- MiscDeviceRegistration::register(
> @@ -205,6 +222,31 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
>      }
>  }
>  
> +#[cfg(CONFIG_SAMPLE_RUST_MISC_DEVICE_WITH_PARENT)]
> +impl kernel::Module for RustMiscDeviceModule {
> +    fn init(_module: &'static ThisModule) -> Result<Self> {
> +        let options = Self::init();
> +        let faux = faux::Registration::new(c_str!("rust-misc-device-sample"), None)?;
> +
> +        // For every other bus, this would be called from Driver::probe(), which would return a
> +        // `Result<Pin<KBox<T>>>`, but faux always binds to a "dummy" driver, hence probe() is

Not clear what `T` is supposed to be, do you mean `Self`?

> +        // not required.
> +        let misc = KBox::pin_init(
> +            MiscDeviceRegistration::register(
> +                options,
> +                Arc::pin_init(new_mutex!(Inner { value: 0_i32 }), GFP_KERNEL),
> +                Some(faux.as_ref()),
> +            ),
> +            GFP_KERNEL,
> +        )?;

You could also initialize this module variation in-place. (this would
also require the pin-init change to reference initialized fields)

---
Cheers,
Benno

> +
> +        Ok(Self {
> +            _faux: faux,
> +            _miscdev: misc,
> +        })
> +    }
> +}
> +
>  struct Inner {
>      value: i32,
>  }