| 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
| ||
|
Message-Id: <95623012-BAA8-4AA5-9D34-42FAE44B0FFD@collabora.com>
Date: Wed, 4 Feb 2026 15:28:20 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Lyude Paul <lyude@...hat.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 v5 2/4] rust/drm: Don't setup private driver data until
registration
Hi Lyude,
> On 30 Jan 2026, at 21:13, Lyude Paul <lyude@...hat.com> wrote:
>
> Now that we have a DeviceContext that we can use to represent whether a
> Device is known to have been registered, we can make it so that drivers can
> create their Devices but wait until the registration phase to assign their
> private data to the Device. This is desirable as some drivers need to make
> use of the DRM device early on before finalizing their private driver data.
>
> As such, this change makes it so that the driver's private data can
> currently only be accessed through Device<T, Registered> types and not
> Device<T, Uninit>.
>
> Signed-off-by: Lyude Paul <lyude@...hat.com>
> ---
> V4:
> * Remove accidental double-aliasing &mut in Device::release()
> ---
> drivers/gpu/drm/nova/driver.rs | 4 +--
> drivers/gpu/drm/tyr/driver.rs | 4 +--
> rust/kernel/drm/device.rs | 64 +++++++++++++++++++++-------------
> rust/kernel/drm/driver.rs | 19 ++++++++--
> 4 files changed, 59 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
> index 99d6841b69cbc..8cea5f68c3b04 100644
> --- a/drivers/gpu/drm/nova/driver.rs
> +++ b/drivers/gpu/drm/nova/driver.rs
> @@ -56,8 +56,8 @@ impl auxiliary::Driver for NovaDriver {
> fn probe(adev: &auxiliary::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
> let data = try_pin_init!(NovaData { adev: adev.into() });
>
> - let drm = drm::UnregisteredDevice::<Self>::new(adev.as_ref(), data)?;
> - let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), 0)?;
> + let drm = drm::UnregisteredDevice::<Self>::new(adev.as_ref())?;
> + let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), data, 0)?;
>
> Ok(Self { drm: drm.into() })
> }
> diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> index 0d479b5d7bd62..b574ee1c24587 100644
> --- a/drivers/gpu/drm/tyr/driver.rs
> +++ b/drivers/gpu/drm/tyr/driver.rs
> @@ -133,8 +133,8 @@ fn probe(
> gpu_info,
> });
>
> - let tdev = drm::UnregisteredDevice::<Self>::new(pdev.as_ref(), data)?;
> - let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), 0)?;
> + let tdev = drm::UnregisteredDevice::<Self>::new(pdev.as_ref())?;
> + let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), data, 0)?;
>
> let driver = TyrDriver {
> _device: tdev.into(),
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index 8f1780b8f0196..a1d9b0e92f3fd 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
> @@ -26,13 +26,15 @@
> };
> use core::{
> alloc::Layout,
> + cell::UnsafeCell,
> marker::PhantomData,
> - mem,
> - ops::Deref,
> - ptr::{
> + mem::{
> self,
> - NonNull, //
> + MaybeUninit, //
> },
> + ops::Deref,
> + ptr::NonNull,
> + sync::atomic::*,
> };
>
> #[cfg(CONFIG_DRM_LEGACY)]
> @@ -141,7 +143,7 @@ impl DeviceContext for Uninit {}
> ///
> /// 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>;
> @@ -188,7 +190,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>());
> @@ -207,22 +209,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`.
> @@ -253,7 +239,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,
What about this?
/**
* @registered:
*
* Internally used by drm_dev_register() and drm_connector_register().
*/
bool registered;
Can’t we use this in lieu of this flag you’ve added?
> +
> + /// 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>,
> }
>
— Daniel
Powered by blists - more mailing lists