Commit 0a69ac25 authored by Eliot Courtney's avatar Eliot Courtney Committed by Danilo Krummrich
Browse files

rust: drm: fix unsound initialization in drm::Device::new 



If pinned initialization of drm::Device::Data fails, it calls
drm::Device::release via drm_dev_put. This materializes a reference to
&drm::Device, but it's not fully constructed yet, because initializing
`data` failed. It should not be dropped either. Instead, if pinned
initialization fails, make sure drm::Device::release isn't called.

Fixes: 2e9fdbe5 ("rust: drm: device: drop_in_place() the drm::Device in release()")
Signed-off-by: default avatarEliot Courtney <ecourtney@nvidia.com>
Reviewed-by: default avatarGary Guo <gary@garyguo.net>
Link: https://patch.msgid.link/20260501-fix-drm-1-v2-1-5c4f681837bc@nvidia.com


Signed-off-by: default avatarDanilo Krummrich <dakr@kernel.org>
parent 2e42a17b

rust/kernel/drm/device.rs

+16 −6
Original line number Diff line number Diff line
@@ -119,13 +119,20 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<A 
        // compatible `Layout`.

        let layout = Kmalloc::aligned_layout(Layout::new::<Self>());



        // Use a temporary vtable without a `release` callback until `data` is initialized, so

        // init failure can release the DRM device without dropping uninitialized fields.

        let alloc_vtable = bindings::drm_driver {

            release: None,

            ..Self::VTABLE

        };



        // SAFETY:

        // - `VTABLE`, as a `const` is pinned to the read-only section of the compilation,

        // - `alloc_vtable` reference remains valid until no longer used,

        // - `dev` is valid by its type invarants,

        let raw_drm: *mut Self = unsafe {

            bindings::__drm_dev_alloc(

                dev.as_raw(),

                &Self::VTABLE,

                &alloc_vtable,

                layout.size(),

                mem::offset_of!(Self, dev),

            )
@@ -133,6 +140,10 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<A 
        .cast();

        let raw_drm = NonNull::new(from_err_ptr(raw_drm)?).ok_or(ENOMEM)?;



        // SAFETY: `raw_drm` is a valid pointer to `Self`, given that `__drm_dev_alloc` was

        // successful.

        let drm_dev = unsafe { Self::into_drm_device(raw_drm) };



        // SAFETY: `raw_drm` is a valid pointer to `Self`.

        let raw_data = unsafe { ptr::addr_of_mut!((*raw_drm.as_ptr()).data) };
@@ -140,15 +151,14 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<A 
        // - `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 { Self::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: `drm_dev` is still private to this function.

        unsafe { (*drm_dev).driver = const { &Self::VTABLE } };



        // SAFETY: The reference count is one, and now we take ownership of that reference as a

        // `drm::Device`.

        Ok(unsafe { ARef::from_raw(raw_drm) })