[PATCH v2] rust: drm: fix unsound initialization in drm::Device::new

[PATCH v2] rust: drm: fix unsound initialization in drm::Device::new

[Eliot Courtney]

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: 2e9fdbe5ec7a ("rust: drm: device: drop_in_place() the drm::Device in release()")
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
Changes in v2:
- stack allocate `alloc_vtable`
- use const { &vtable } trick to help prevent future issues
- Link to v1: https://patch.msgid.link/20260428-fix-drm-1-v1-1-755057178066@nvidia.com
---
 rust/kernel/drm/device.rs | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index adbafe8db54d..403fc35353c7 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -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) })

---
base-commit: d9a6809478f9815b6455a327aa001737ac7b2c09
change-id: 20260428-fix-drm-1-459b6357de23

Best regards,
--  
Eliot Courtney <ecourtney@nvidia.com>

Re: [PATCH v2] rust: drm: fix unsound initialization in drm::Device::new

[Gary Guo]

On Fri May 1, 2026 at 11:49 AM BST, Eliot Courtney wrote:
> 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: 2e9fdbe5ec7a ("rust: drm: device: drop_in_place() the drm::Device in release()")
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
> Changes in v2:
> - stack allocate `alloc_vtable`
> - use const { &vtable } trick to help prevent future issues
> - Link to v1: https://patch.msgid.link/20260428-fix-drm-1-v1-1-755057178066@nvidia.com
> ---
>  rust/kernel/drm/device.rs | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)

Re: [PATCH v2] rust: drm: fix unsound initialization in drm::Device::new

[Danilo Krummrich]

On Fri, 01 May 2026 19:49:37 +0900, Eliot Courtney wrote:
> [PATCH v2] rust: drm: fix unsound initialization in drm::Device::new

Applied, thanks!

  Branch: drm-rust-fixes
  Tree:   https://gitlab.freedesktop.org/drm/rust/kernel.git

[1/1] rust: drm: fix unsound initialization in drm::Device::new
      commit: 0a69ac25bd59

The patch will appear in the next linux-next integration (typically within 24
hours on weekdays).

The patch is queued up for Linus's tree and should land in the next -rc release.

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

[Claude Code Review Bot]

Patch Review

**Bug analysis — confirmed correct:**

The current code path on init failure is:
1. `data.__pinned_init(raw_data)` fails
2. `inspect_err` calls `drm_dev_put(drm_dev)`
3. Refcount → 0, triggers `drm_dev_release()` in C
4. `drm_dev_release()` checks `dev->driver->release` — it's `Some(Self::release)`
5. `Self::release()` calls `drop_in_place(this)`, dropping uninitialized `T::Data` — **UB**

**Fix analysis:**

The temporary stack-allocated vtable is correct:

```c
let alloc_vtable = bindings::drm_driver {
    release: None,
    ..Self::VTABLE
};
```

The lifetime is sound: `alloc_vtable` lives on the stack for the entire function body. It is only accessed through `drm_device->driver` in two places: (1) within `__drm_dev_alloc` → `drm_dev_init`, and (2) in the error path via `drm_dev_put` → `drm_dev_release`. In both cases `alloc_vtable` is still in scope. On error, `drm_dev_put` frees the device, so no dangling reference survives. On success, the driver pointer is replaced before the function returns.

The `const { &Self::VTABLE }` trick for installing the real vtable is correct and idiomatic:

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

The `const { ... }` block promotes the reference to `'static`, matching the original guarantee that the vtable pointer stored in the `drm_device` outlives the device. The SAFETY justification ("drm_dev is still private to this function") is correct — the device hasn't been returned or registered, so mutation is safe.

Moving `into_drm_device` before the init attempt is harmless (pure pointer arithmetic, no side effects) and avoids duplication between the error and success paths.

**Minor nit — SAFETY comment could be more precise:**

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

This is somewhat tautological. Consider something like:
```
// - `alloc_vtable` is stack-allocated and outlives all accesses: it is either
//   replaced by a static ref on success or the device is freed on failure,
```

Not a blocker — the code is correct regardless.

**Pre-existing typo (not introduced by this patch):** the SAFETY comment has `invarants` instead of `invariants`.

**Verdict: Looks good.** The fix is minimal, addresses a real soundness issue, and the lifetime reasoning is sound.

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

Overall Series Review

Subject: rust: drm: fix unsound initialization in drm::Device::new
Author: Eliot Courtney <ecourtney@nvidia.com>
Patches: 3
Reviewed: 2026-05-05T09:29:46.588748

---

This is a single-patch series fixing a real **soundness bug** in the Rust DRM device initialization path. The bug is: if `data.__pinned_init()` fails, the error path calls `drm_dev_put()`, which drops the refcount to zero, triggering `Self::release()`, which calls `drop_in_place()` on a `Device<T>` whose `data` field was never initialized — undefined behavior.

The fix is well-designed: use a temporary vtable with `release: None` during the initialization window, so that if init fails, `drm_dev_put()` skips the release callback and never drops uninitialized memory. After successful init, install the real vtable with the `const { &Self::VTABLE }` trick to guarantee a `'static` reference. The approach is sound and the implementation is correct.

**Recommendation: Accept.** One minor documentation nit below.

---

---
Generated by Claude Code Patch Reviewer