Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The patch routes the ioctl data buffer to `__call_ioctl` as a generic void pointer, inferring the type of `Data` purely from the user-provided handler closure rather than enforcing it matches the UAPI struct. This removes compile-time type safety and allows an incorrect handler signature to cause a silent heap out-of-bounds access. --
commit cb52c14a04f657a72b41bd28dae4125aa6d86907 Author: Danilo Krummrich <[email protected]> rust: drm: ioctl: fix unbounded lifetimes in ioctl handler arguments This patch fixes unbounded lifetimes in declare_drm_ioctls! by routing references through a helper function __call_ioctl tied to a local anchor variable. Closes: https://lore.kernel.org/all/[email protected]/ > diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs > index cf328101dde4e..023e6da5c1e4b 100644 > --- a/rust/kernel/drm/ioctl.rs > +++ b/rust/kernel/drm/ioctl.rs > @@ -70,6 +70,39 @@ pub mod internal { [ ... ] > + pub unsafe fn __call_ioctl< > + 'a, > + Dev: 'a, > + Data: 'a, > + F: super::super::file::DriverFile + 'a, > + Ret, > + >( > + _anchor: &'a (), > + dev: &'a Dev, > + raw_data: *mut ::core::ffi::c_void, > + raw_file: *mut drm_file, > + f: impl FnOnce(&'a Dev, &'a mut Data, &'a super::super::File<F>) -> > Ret, > + ) -> Ret { > + // SAFETY: Caller guarantees raw_data points to a valid instance of > Data with the correct > + // size and alignment, exclusively owned for the duration of the > ioctl call. > + let data = unsafe { &mut *(raw_data.cast::<Data>()) }; [Severity: High] Does this type inference break the macro's memory safety guarantees? Because Data is a generic type parameter and isn't explicitly specified, the Rust compiler will infer it from the signature of the handler function. If a driver author mistakenly writes an ioctl handler that takes a differently sized or larger UAPI struct, Data will be inferred as that larger type. Since raw_data points to a kernel buffer of size _IOC_SIZE(cmd) matching the original macro struct, dereferencing it as a larger type would create a reference that extends past the end of the allocated memory, leading to a silent heap out-of-bounds access. > + // SAFETY: Caller guarantees raw_file is a valid pointer to a > `struct drm_file`. > + let file = unsafe { super::super::File::<F>::from_raw(raw_file) }; > + f(dev, data, file) > + } > } > > /// Declare the DRM ioctls for a driver. [ ... ] > @@ -135,19 +168,19 @@ macro_rules! declare_drm_ioctls { > - let data = unsafe { &mut > *(raw_data.cast::<$crate::uapi::$struct>()) }; > - // SAFETY: This is just the DRM file structure > - let file = unsafe { > $crate::drm::File::from_raw(raw_file) }; > - > - match $func(dev, data, file) { > + let __anchor = (); > + > + // 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. The argument is > exclusively owned by this > + // handler, guaranteed by `drm_ioctl()` to > remain valid for the > + // duration of the call. > + // - `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, > + ) } { [Severity: High] Is there a way to enforce the UAPI $struct type here? Prior to this patch, declare_drm_ioctls! explicitly cast raw_data to the declared UAPI struct, which forced a compile-time error if the user-provided handler expected a different type. By passing the handler function directly to __call_ioctl without constraining the Data type to $crate::uapi::$struct, the compile-time type safety checking appears to be bypassed, allowing the inference issue described above. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
