Re: [PATCH] rust: drm: fix unsound initialization in drm::Device::new
From: Eliot Courtney
Date: Fri May 01 2026 - 06:50:55 EST
On Wed Apr 29, 2026 at 8:34 PM JST, Gary Guo wrote:
> On Wed Apr 29, 2026 at 9:03 AM BST, Alice Ryhl wrote:
>>
>>> @@ -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 = &Self::VTABLE };
>>
>> It would be bad if this ended up being a reference to a local variable.
>> Please use `&const { Self::VTABLE }` so that it doesn't compile if this
>> occurs.
>
> Self::VTABLE and `const {}` are both just constants and there's no difference
> here.
>
> If you want to guaranteed static promotion it should be
>
> const { &Self::VTABLE }
>
> Best,
> Gary
Thanks all, I have done both of these things (const{&} + stack alloc).
The `drm_driver` struct is 200 bytes, for reference (w.r.t. stack
alloc).