linux-kernel - Re: [PATCH 2/2] rust/kernel: Add platform::ModuleDevice

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: <2025012329-candied-walk-f57d@gregkh>
Date: Thu, 23 Jan 2025 07:23:08 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Lyude Paul <lyude@...hat.com>
Cc: rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
	Maíra Canal <mairacanal@...eup.net>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Danilo Krummrich <dakr@...nel.org>, 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>,
	Benno Lossin <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>
Subject: Re: [PATCH 2/2] rust/kernel: Add platform::ModuleDevice

On Wed, Jan 22, 2025 at 06:49:22PM -0500, Lyude Paul wrote:
> A number of kernel modules work with virtual devices, where being virtual
> implies that there's no physical device to actually be plugged into the
> system. Because of that, such modules need to be able to manually
> instantiate a kernel device themselves - which can then be probed in the
> same manner as any other kernel device.
> 
> This adds support for such a usecase by introducing another platform device
> type, ModuleDevice. This type is interchangeable with normal platform
> devices, with the one exception being that it controls the lifetime of the
> registration of the device.

Sorry, but a "virtual" device is NOT a platform device at all.  Platform
devices are things that are not on a real bus and are described by
firmware somehow.

The kernel has "virtual" devices today just fine, look at
/sys/devices/virtual/ so why not just use that api instead of making up
something new?

And modules are code, not data.  Let's not start to even attempt to tie
lifetimes of device structures to module code, that way lies madness and
rolls back the work we did decades ago to split the two apart :)
> 
> Signed-off-by: Lyude Paul <lyude@...hat.com>
> Co-authored-by: Maíra Canal <mairacanal@...eup.net>
> ---
>  rust/kernel/platform.rs | 96 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 94 insertions(+), 2 deletions(-)
> 
> diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
> index 75dc7824eccf4..b5d38bb182e93 100644
> --- a/rust/kernel/platform.rs
> +++ b/rust/kernel/platform.rs
> @@ -13,8 +13,11 @@
>      types::{ARef, ForeignOwnable, Opaque},
>      ThisModule,
>  };
> -
> -use core::ptr::{NonNull, addr_of_mut};
> +use core::{
> +    mem::ManuallyDrop,
> +    ops::*,
> +    ptr::{addr_of_mut, NonNull},
> +};
>  
>  /// An adapter for the registration of platform drivers.
>  pub struct Adapter<T: Driver>(T);
> @@ -213,3 +216,92 @@ fn as_ref(&self) -> &device::Device {
>          &self.0
>      }
>  }
> +
> +/// A platform device ID specifier.
> +///
> +/// This type is used for selecting the kind of device ID to use when constructing a new
> +/// [`ModuleDevice`].
> +#[derive(Copy, Clone)]
> +pub enum ModuleDeviceId {
> +    /// Do not use a device ID with a device.
> +    None,
> +    /// Automatically allocate a device ID for a device.
> +    Auto,
> +    /// Explicitly specify a device ID for a device.
> +    Explicit(i32),
> +}
> +
> +impl ModuleDeviceId {
> +    fn as_raw(self) -> Result<i32> {
> +        match self {
> +            ModuleDeviceId::Explicit(id) => {
> +                if matches!(
> +                    id,
> +                    bindings::PLATFORM_DEVID_NONE | bindings::PLATFORM_DEVID_AUTO
> +                ) {
> +                    Err(EINVAL)
> +                } else {
> +                    Ok(id)
> +                }
> +            }
> +            ModuleDeviceId::None => Ok(bindings::PLATFORM_DEVID_NONE),
> +            ModuleDeviceId::Auto => Ok(bindings::PLATFORM_DEVID_AUTO),
> +        }
> +    }
> +}
> +
> +/// A platform device that was created by a module.

Again, no, sorry.

If you want a virtual device, make a virtual device.  Ideally using the
auxbus api, but if you REALLY want a "raw" struct device, wonderful,
create that and register it with the driver core which will throw it
under /sys/devices/virtual/ which is where it belongs.

But I don't think you want that, what you want is all the power of a
real bus, but none of the hassle, which is why people abuse the platform
device code so much.  And why I complain about it so much.

So, sorry, I am not going to allow the abuse of the platform device code
to carry over into rust drivers if I can possibly help it.  Make a real
bus.  Or a raw device.  Or use the busses you have today (again,
auxbus), but do NOT abuse platform devices for things they are not.

thanks,

greg k-h