Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Critical] The `declare_drm_ioctls!` macro fails to constrain the ioctl 
handler's data argument type to the declared UAPI `$struct`, which allows 
handlers to silently accept larger types and causes out-of-bounds memory 
accesses.
--

commit aaaa8c4ed4a35894ea9d49cf012ca359ca7c8ab5
Author: Danilo Krummrich <[email protected]>

rust: drm: Wrap ioctl dispatch in RegistrationGuard

This patch ensures that ioctl handlers receive a reference to a registered
device, proving at the type level that the device has not been unplugged.
It achieves this by calling `registration_guard()` on the device and using a
dead-code closure as a type-inference anchor to resolve the driver type
parameter.

> diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
> index 70cf1aa4d788f..6cefd26b31f91 100644
> --- a/rust/kernel/drm/ioctl.rs
> +++ b/rust/kernel/drm/ioctl.rs

[ ... ]

> @@ -164,11 +176,38 @@ macro_rules! declare_drm_ioctls {

[ ... ]

> +                            // 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(
> +                                    // SAFETY: This closure is never 
> executed; the dereference
> +                                    // exists purely to unify the type 
> parameter with `$func`.
> +                                    // The pointer is valid regardless.
> +                                    unsafe { &*__ptr },
> +                                    unreachable!(),
> +                                    unreachable!(),

[Severity: Critical]
Does using unreachable!() here drop the type constraint on the UAPI struct?

Because unreachable!() coerces to any type, it appears this closure fails to
anchor the data argument to the expected UAPI struct.

This allows the compiler to infer the data type entirely from the user-provided
handler signature without enforcing that it matches the declared UAPI struct.

> +                                )
> +                            };
> +
> +                            let Some(guard) = dev.registration_guard() else {
> +                                return 
> $crate::error::code::ENODEV.to_errno();
> +                            };
>                              let __anchor = ();
>  
>                              // SAFETY:
> @@ -180,7 +219,7 @@ macro_rules! declare_drm_ioctls {
>                              // - `raw_file` is a valid `struct drm_file` 
> pointer provided by the
>                              //   DRM core.
>                              match unsafe { 
> $crate::drm::ioctl::internal::__call_ioctl(
> -                                &__anchor, dev, raw_data, raw_file, $func,
> +                                &__anchor, &*guard, raw_data, raw_file, 
> $func,

[Severity: Critical]
If the handler author accidentally specifies a larger type for the data
argument, could this lead to out-of-bounds memory access?

Since __call_ioctl() infers its generic data type directly from the $func
signature, it will cast the raw_data pointer to that inferred type. 

The DRM core allocates the raw_data buffer exactly based on the expected UAPI
struct size. If the handler silently accepts a larger struct type, casting and
dereferencing it in __call_ioctl() would cause out-of-bounds memory accesses.

>                              ) } {
>                                  Err(e) => e.to_errno(),
>                                  Ok(i) => i.try_into()

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=11

Reply via email to