From: Eliot Courtney <ecourtney@nvidia.com>
To: "David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>,
"Danilo Krummrich" <dakr@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <lossin@kernel.org>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Trevor Gross" <tmgross@umich.edu>
Cc: Alexandre Courbot <acourbot@nvidia.com>,
dri-devel@lists.freedesktop.org, rust-for-linux@vger.kernel.org,
linux-kernel@vger.kernel.org,
Eliot Courtney <ecourtney@nvidia.com>
Subject: [PATCH v2] rust: drm: fix unsound initialization in drm::Device::new
Date: Fri, 01 May 2026 19:49:37 +0900 [thread overview]
Message-ID: <20260501-fix-drm-1-v2-1-5c4f681837bc@nvidia.com> (raw)
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>
next reply other threads:[~2026-05-01 10:49 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-01 10:49 Eliot Courtney [this message]
2026-05-03 11:49 ` [PATCH v2] rust: drm: fix unsound initialization in drm::Device::new Gary Guo
2026-05-03 12:21 ` Danilo Krummrich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260501-fix-drm-1-v2-1-5c4f681837bc@nvidia.com \
--to=ecourtney@nvidia.com \
--cc=a.hindborg@kernel.org \
--cc=acourbot@nvidia.com \
--cc=airlied@gmail.com \
--cc=aliceryhl@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun@kernel.org \
--cc=dakr@kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=gary@garyguo.net \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=simona@ffwll.ch \
--cc=tmgross@umich.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox