Re: [PATCH v3 11/13] rust: drm: Wrap ioctl dispatch in RegistrationGuard

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Type inference hole in declare_drm_ioctls! allows safe handlers to trigger UB by inferring a Registered context before registration is verified.
- [High] Unbounded lifetimes for `data` and `file` in declare_drm_ioctls! allow safe ioctl handlers to stash references, causing use-after-free.
--

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

rust: drm: Wrap ioctl dispatch in RegistrationGuard

This commit wraps every ioctl handler inside a drm_dev_enter/exit critical
section using RegistrationGuard. It returns ENODEV if the device is unplugged
and anchors the driver type with a never-called closure to assist compiler
type inference.

> diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
> index cf328101dde4e..c6a03be018e6d 100644
> --- a/rust/kernel/drm/ioctl.rs
> +++ b/rust/kernel/drm/ioctl.rs
[ ... ]
> @@ -135,6 +138,16 @@ macro_rules! declare_drm_ioctls {
>                              // dev/file match the current driver these ioctls are being declared
>                              // for, and it's not clear how to enforce this within the type system.
>                              let dev = $crate::drm::device::Device::from_raw(raw_dev);
> +
> +                            // Type-inference anchor: the closure is never called but ties `dev`'s
> +                            // type to `$func`'s first parameter, which the compiler cannot infer
> +                            // through method resolution and associated-type projections alone.
> +                            #[allow(unreachable_code)]
> +                            let _ = || $func(dev, unreachable!(), unreachable!());
> +
> +                            let Some(_guard) = dev.registration_guard() else {
> +                                return $crate::error::code::ENODEV.to_errno();
> +                            };

[Severity: High]
This is a pre-existing issue, but does this type inference anchor create a
scenario where safe driver code can trigger undefined behavior? 

If a driver author writes a safe ioctl handler that expects a registered
context, such as &Device<T, Registered>, the type-inference anchor will tie
the type of dev to that first parameter. This causes Device::from_raw() to be
evaluated with C = Registered.

Since Device::from_raw() is called before dev.registration_guard() can
verify the device is actually bound, doesn't this create a Registered
reference for an unplugged device? Would this violate the type invariant
and safety contract of from_raw() before the guard is even checked?

[Severity: High]
This is also a pre-existing issue, but are the lifetimes of the data and file
arguments passed to the user's $func properly constrained?

While the guard bounds the lifetime of the device access, the data and file
parameters instantiated from raw pointers later in this macro don't appear to
have explicit lifetime bounds. If $func demands a 'static lifetime for these
arguments, will the compiler incorrectly infer them as 'static?

If so, could a safe ioctl handler stash these references in a global variable
or async context, causing a use-after-free once the ioctl callback returns and
the C API invalidates the pointers?

>                              // SAFETY: The ioctl argument has size `_IOC_SIZE(cmd)`, which we
>                              // asserted above matches the size of this type, and all bit patterns of
>                              // UAPI structs must be valid.

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