linux-kernel - Re: [PATCH v3 1/2] rust: miscdevice: add additional data to MiscDeviceRegistration

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] [day] [month] [year] [list]

Message-ID: <aC5bOLsdoAET_IPR@google.com>
Date: Wed, 21 May 2025 23:01:12 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: Christian Schrefl <chrisi.schrefl@...il.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.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>, Trevor Gross <tmgross@...ch.edu>, Arnd Bergmann <arnd@...db.de>, 
	Lee Jones <lee@...nel.org>, Daniel Almeida <daniel.almeida@...labora.com>, 
	"Gerald Wisböck" <gerald.wisboeck@...ther.ink>, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] rust: miscdevice: add additional data to MiscDeviceRegistration

On Wed, May 21, 2025 at 02:16:20PM +0200, Christian Schrefl wrote:
> Hi Greg,
> 
> On 21.05.25 2:04 PM, Greg Kroah-Hartman wrote:
> > On Wed, May 21, 2025 at 01:55:36PM +0200, Greg Kroah-Hartman wrote:
> >> On Sat, May 17, 2025 at 03:42:29PM +0200, Danilo Krummrich wrote:
> >>> On Sat, May 17, 2025 at 01:33:49PM +0200, Christian Schrefl wrote:
> >>>> +pub struct MiscDeviceRegistration<T: MiscDevice> {
> >>>>      #[pin]
> >>>>      inner: Opaque<bindings::miscdevice>,
> >>>> +    #[pin]
> >>>> +    data: UnsafePinned<T::RegistrationData>,
> >>>>      _t: PhantomData<T>,
> >>>>  }
> >>>
> >>> I recommend not to store data within a Registration type itself.
> >>>
> >>> I know that this is designed with the focus on using misc device directly from
> >>> the module scope; and in this context it works great.
> >>>
> >>> However, it becomes quite suboptimal when used from a driver scope. For
> >>> instance, if the misc device is registered within a platform driver's probe()
> >>> function.
> >>>
> >>> I know this probably isn't supported yet. At least, I assume it isn't supported
> >>> "officially", given that the abstraction does not provide an option to set a
> >>> parent device. Yet I think we should consider it.
> >>
> >> It's going to be a requirement to properly set the parent device, and
> >> as you point out, this really should be in some sort of scope, not just
> >> a module.
> >>
> >> But, we have two types of users of a misc device, one like this is
> >> written, for a module-scope, and one for the "normal" device scope.  The
> >> device scope is going to be tricker as it can, and will, get
> >> disconnected from the device separately from the misc device lifespan,
> >> so when that logic is added, it's going to be tricky as you point out.
> >>
> >> So I'll take this now, but in the future this is going to have to be
> >> cleaned up and modified.
> > 
> > Nope, can't take it, it breaks the build from my tree:
> > 
> > error[E0432]: unresolved import `crate::types::UnsafePinned`
> >   --> rust/kernel/miscdevice.rs:20:37
> >    |
> > 20 |     types::{ForeignOwnable, Opaque, UnsafePinned},
> >    |                                     ^^^^^^^^^^^^ no `UnsafePinned` in `types`
> > 
> > error[E0432]: unresolved import `pin_init::Wrapper`
> >   --> rust/kernel/miscdevice.rs:23:5
> >    |
> > 23 | use pin_init::Wrapper;
> >    |     ^^^^^^^^^^^^^^^^^ no `Wrapper` in the root
> > 
> > error: aborting due to 2 previous errors
> > 
> > :(
> 
> This requires my `UnsafePinned` [0] patches (& with that pin-init-next) like I wrote
> in the cover letter.
> 
> [0]: https://lore.kernel.org/rust-for-linux/20250511-rust_unsafe_pinned-v4-0-a86c32e47e3d@gmail.com/

It looks like there's still a fair amount of discussion going on there.
Do you want to just use Opaque for now and then I'll fix it later once
the other series lands?

Alice