Re: [PATCH v3 12/19] rust: io: add projection macro and methods

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unsound `AsBytes` implementation for `Region<SIZE>` leading to undefined behavior due to alignment padding.
- [Low] Incorrect use of `#[inline(always)]` on `ProjectHelper` methods, violating subsystem coding guidelines.
--

commit ab6f2e99706c5568aaec1a417178b7f6d1435b90
Author: Gary Guo <gary@garyguo.net>

rust: io: add projection macro and methods

Add a io_project!() macro allows projection from Io to a subview of it,
using the pointer projection mechanism to perform compile-time checks.
For cases where type-casting is required, the try_cast() function may be
used where the size and alignment checks are performed at runtime.

> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index 9c2ea17ca87b5..173f8c0ba2d6c 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
[ ... ]
> @@ -92,6 +96,11 @@ fn size(p: *const Self) -> usize {
>      }
>  }
>  
> +// SAFETY: I/O regions can compose of arbitrary bytes.
> +unsafe impl<const SIZE: usize> kernel::transmute::FromBytes for Region<SIZE> {}
> +// SAFETY: Values read from I/O are always treated as initialized.
> +unsafe impl<const SIZE: usize> kernel::transmute::AsBytes for Region<SIZE> {}

[Severity: High]
Does this implementation of AsBytes for Region<SIZE> introduce undefined
behavior due to alignment padding?

Region<SIZE> is defined with an alignment constraint and contains an
unsized [u8] field. Because core::mem::size_of_val for dynamically sized
types rounds the size up to a multiple of the alignment, a Region created
with a size that is not a multiple of 8 (on 64-bit systems) will include
trailing padding bytes.

Since AsBytes::as_bytes() relies on size_of_val(self) to slice the memory,
calling it on &Region<SIZE> could produce a &[u8] that extends past the
actual allocated length into out-of-bounds or uninitialized memory.

> +
>  /// Raw representation of an MMIO region.
>  ///
>  /// `MmioRaw<T>` is equivalent to `T __iomem *` in C.
[ ... ]
> @@ -983,3 +1039,78 @@ pub fn relaxed(self) -> RelaxedMmio<'a, T> {
>  // MMIO regions on 64-bit systems also support 64-bit accesses.
>  #[cfg(CONFIG_64BIT)]
>  impl_mmio_io_capable!(RelaxedMmioBackend, u64, readq_relaxed, writeq_relaxed);
> +
> +// This helper turns associated functions to methods so it can be invoked in macro.
> +// Used by `io_project!()` only.
> +#[doc(hidden)]
> +#[derive(Clone, Copy)]
> +pub struct ProjectHelper<T>(pub T);
> +
> +impl<'a, T> ProjectHelper<T>
> +where
> +    T: Io<'a, Backend: IoBackend<View<'a, T::Target> = T>>,
> +{
> +    // These helper methods must not have symbols present in binary to avoid confusion.
> +    #[inline(always)]
> +    pub fn as_ptr(self) -> *mut T::Target {

[Severity: Low]
This isn't a bug, but should this use #[inline] instead of #[inline(always)]?

According to the rust subsystem guidelines, functions that are small or
forwarding to a binding call in abstractions should be annotated with
#[inline].

The #[inline(always)] annotation should be reserved for functions using
build_assert!() that depend on function parameters.

> +        T::Backend::as_ptr(self.0)
> +    }
> +
> +    /// # Safety
> +    ///
> +    /// Same as `IoBackend::project_view`
> +    #[inline(always)]
> +    pub unsafe fn project_view<U: ?Sized + KnownSize>(

[Severity: Low]
This isn't a bug, but should this use #[inline] instead of #[inline(always)]?

According to the rust subsystem guidelines, functions that are small or
forwarding to a binding call in abstractions should be annotated with
#[inline].

The #[inline(always)] annotation should be reserved for functions using
build_assert!() that depend on function parameters.

> +        self,
> +        ptr: *mut U,
> +    ) -> <T::Backend as IoBackend>::View<'a, U> {
> +        // SAFETY: Per safety requirement.
> +        unsafe { T::Backend::project_view::<T::Target, _>(self.0, ptr) }
> +    }
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608-io_projection-v3-0-c5cde13a5ec4@garyguo.net?part=12