linux-kernel - Re: [PATCH v2 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] [day] [month] [year] [list]

Message-Id: <DFRSH2462TT2.RZBLN08KS0IW@kernel.org>
Date: Sun, 18 Jan 2026 15:36:13 +0100
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Lyude Paul" <lyude@...hat.com>
Cc: <dri-devel@...ts.freedesktop.org>, <nouveau@...ts.freedesktop.org>,
 <linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.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>, "Atharv Dubey"
 <atharvd440@...il.com>, "Daniel Almeida" <daniel.almeida@...labora.com>
Subject: Re: [PATCH v2 2/3] rust/drm: Don't setup private driver data until
 registration

On Fri Jan 16, 2026 at 9:41 PM CET, Lyude Paul wrote:
> @@ -118,7 +120,7 @@ pub trait DeviceContext: Sealed + Send + Sync {}
>  ///
>  /// The device in `self.0` is guaranteed to be a newly created [`Device`] that has not yet been
>  /// registered with userspace until this type is dropped.
> -pub struct UnregisteredDevice<T: drm::Driver>(ARef<Device<T, Uninit>>, NotThreadSafe);
> +pub struct UnregisteredDevice<T: drm::Driver>(pub(crate) ARef<Device<T, Uninit>>, NotThreadSafe);
>  
>  impl<T: drm::Driver> Deref for UnregisteredDevice<T> {
>      type Target = Device<T, Uninit>;
> @@ -165,7 +167,7 @@ impl<T: drm::Driver> UnregisteredDevice<T> {
>      /// Create a new `UnregisteredDevice` for a `drm::Driver`.
>      ///
>      /// This can be used to create a [`Registration`](kernel::drm::Registration).
> -    pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<Self> {
> +    pub fn new(dev: &device::Device) -> Result<Self> {
>          // `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()`
>          // compatible `Layout`.
>          let layout = Kmalloc::aligned_layout(Layout::new::<Self>());
> @@ -184,22 +186,6 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<S
>          .cast();
>          let raw_drm = NonNull::new(from_err_ptr(raw_drm)?).ok_or(ENOMEM)?;
>  
> -        // SAFETY: `raw_drm` is a valid pointer to `Self`.
> -        let raw_data = unsafe { ptr::addr_of_mut!((*raw_drm.as_ptr()).data) };
> -
> -        // SAFETY:
> -        // - `raw_data` is a valid pointer to uninitialized memory.
> -        // - `raw_data` will not move until it is dropped.
> -        unsafe { data.__pinned_init(raw_data) }.inspect_err(|_| {
> -            // SAFETY: `raw_drm` is a valid pointer to `Self`, given that `__drm_dev_alloc` was
> -            // successful.
> -            let drm_dev = unsafe { Device::into_drm_device(raw_drm) };
> -
> -            // SAFETY: `__drm_dev_alloc()` was successful, hence `drm_dev` must be valid and the
> -            // refcount must be non-zero.
> -            unsafe { bindings::drm_dev_put(drm_dev) };
> -        })?;
> -
>          // SAFETY: The reference count is one, and now we take ownership of that reference as a
>          // `drm::Device`.
>          // INVARIANT: We just created the device above, but have yet to call `drm_dev_register`.
> @@ -231,7 +217,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,

Please use a kernel atomic, i.e. kernel::sync::atomic::Atomic<bool>. See also [1].

[1] https://lore.kernel.org/all/20251230093718.1852322-1-fujita.tomonori@gmail.com/

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

Why not just Opaque<T::Data>?

> +
>      _ctx: PhantomData<C>,
>  }