linux-kernel - Re: [PATCH v3 2/3] rust/drm: Don't setup private driver data until registration

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: <47cbb2bb4462331e5ffa56da488d8ffab9a5fcd7.camel@redhat.com>
Date: Fri, 23 Jan 2026 12:00:28 -0500
From: lyude@...hat.com
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	rust-for-linux@...r.kernel.org, Danilo Krummrich <dakr@...nel.org>, 
	nouveau@...ts.freedesktop.org, Miguel Ojeda <ojeda@...nel.org>, Simona
 Vetter	 <simona@...ll.ch>, Alice Ryhl <aliceryhl@...gle.com>, Shankari
 Anand	 <shankari.ak0208@...il.com>, David Airlie <airlied@...il.com>, Benno
 Lossin	 <lossin@...nel.org>, Asahi Lina <lina+kernel@...hilina.net>
Subject: Re: [PATCH v3 2/3] rust/drm: Don't setup private driver data until
 registration

On Thu, 2026-01-22 at 22:52 -0300, Daniel Almeida wrote:
> > @@ -254,7 +240,15 @@ pub fn new(dev: &device::Device, data: impl
> > PinInit<T::Data, Error>) -> Result<S
> > #[repr(C)]
> > pub struct Device<T: drm::Driver, C: DeviceContext = Registered> {
> >      dev: Opaque<bindings::drm_device>,
> > -    data: T::Data,
> > +
> > +    /// Keeps track of whether we've initialized the device data
> > yet.
> > +    pub(crate) data_is_init: AtomicBool,
> 
> Why don’t we make the data a member of the Registered context?

A couple of different reasons. For one: not having it embedded as part
of Device would complicate trying to go from the device's private data
using container_of! to the actual Device struct, which isn't great for
workqueues.

The other much more important reason is that Registered isn't going to
be the only typestate that has access to the driver's private data in
the future. With KMS, nearly all of the modesetting callbacks for a
driver can be invoked before userspace registration. E.g. consider a
modesetting driver that needs to perform a modeset pre-registration so
that the hardware is in a known good state before being exposed to
userspace.

To clarify what this looks like: you'll recall that I made a diagram
showing a high-level overview of the DRM initialization process for the
documentation for DeviceContext. The second stage in that diagram,
which I'm currently calling Init, is the context that we're going to
need to eventually add a typestate for.

FWIW, this is more or less what the flow will look like with this new
context. Indenting indicates calling down to a function from within the
function above

 * UnregisteredDevice::new(d: device::Device) -> UnregisteredDevice<T>
    - // Creates Crtcs, Connectors, etc.
      KmsDriver::probe(d: &UninitializedKmsDevice)
 * // The driver sets stuff up
 * Registration::new_foreign_owned(
     dev: UnregisteredDevice,
     data: impl PinInit<T::Data>
   ):
    - // Initialize `data`, so driver data is ready at this point
    - KmsDriver::pre_registration_init(d: &Device<T, Init>)
    - // Perform actual userspace registration

If it wasn't clear too: this means tyr won't really need to do anything
when we add the new DeviceContext typestate :)

> 
> > +
> > +    /// The Driver's private data.
> > +    ///
> > +    /// This must only be written to from
> > [`drm::Registration::new`].
> > +    pub(crate) data: UnsafeCell<MaybeUninit<T::Data>>,
> > +
> >      _ctx: PhantomData<C>,