Re: [PATCH v5 18/19] rust: drm: Pass registration data to ioctl handlers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Unsound lifetime shortening of the associated type `RegistrationData<'a>` allows safe code to cause a use-after-free.
--

commit b0c6a9cfef59ba2cf3ed097deb2601fbd756cc0d
Author: Danilo Krummrich <dakr@kernel.org>

rust: drm: Pass registration data to ioctl handlers
    
This commit updates the DRM ioctl macro to pass registration data to the ioctl
handlers using a closure-based HRTB pattern.

> diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
> index f24cf7b146a8e..9b45b741db1b9 100644
> --- a/rust/kernel/drm/ioctl.rs
> +++ b/rust/kernel/drm/ioctl.rs

[ ... ]

> @@ -193,7 +195,9 @@ macro_rules! declare_drm_ioctls {
>                              // SAFETY: This is just the DRM file structure
>                              let file = unsafe { $crate::drm::File::from_raw(raw_file) };
>  
> -                            match $func(&*guard, data, file) {
> +                            match guard.registration_data_with(|reg_data| {
> +                                $func(&*guard, reg_data, data, file)
> +                            }) {
 
[Severity: Critical]
Does this HRTB closure pattern safely handle invariant types for
RegistrationData<'a>?

The commit message states that the HRTB ties the lifetime to the closure scope
and the internal pointer cast shortens it from 'static. However, if a driver
uses an invariant type for RegistrationData<'a> (for example, something like
Mutex<Option<&'a mut SomeStruct>>), couldn't this allow safe code to store the
short-lived `data` or `file` reference into the underlying 'static
registration data?

While the HRTB prevents references from returning out of the closure, it
might not prevent storing external, longer-lived references into an invariant
type provided by the closure. Once the ioctl returns and the local variable is
dropped, the 'static registration data could still hold a dangling pointer to
it, potentially leading to a use-after-free.

>                                  Err(e) => e.to_errno(),
>                                  Ok(i) => i.try_into()
>                                              .unwrap_or($crate::error::code::ERANGE.to_errno()),

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628145406.2107056-1-dakr@kernel.org?part=18