Re: [PATCH v4 14/20] rust: io: add I/O backend for system memory with volatile access

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] SysMem allows unaligned volatile reads and writes because it fails to enforce base pointer alignment for the types accessed via the Io trait.
--

commit ab556784fb75f6ee7bad0986c5ef780c29189dc2
Author: Laura Nao <laura.nao@collabora.com>

Add SysMem, an Io trait implementation for kernel virtual address ranges.

This commit introduces SysMem to provide safe access to shared memory
that may be concurrently accessed by external hardware. It uses volatile
accessors and implements IoCapable for standard primitive types. This
provides an alternative to Coherent when other layers handle mapping
the system memory to the device.

> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index c7a9952af995..a418c8cbb031 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs

[ ... ]

> +/// Implements [`IoCapable`] on `SysMemBackend` for `$ty` using `read_volatile` and
> +/// `write_volatile`.
> +macro_rules! impl_sysmem_io_capable {
> +    ($ty:ty) => {
> +        impl IoCapable<$ty> for SysMemBackend {
> +            #[inline]
> +            fn io_read(view: SysMem<'_, $ty>) -> $ty {
> +                // SAFETY:
> +                // - Per type invariant, `ptr` is valid and aligned.
> +                // - Using read_volatile() here so that race with hardware is well-defined.
> +                // - Using read_volatile() here is not sound if it races with other CPU per Rust
> +                //   rules, but this is allowed per LKMM.
> +                // - The macro is only used on primitives so all bit patterns are valid.
> +                unsafe { view.ptr.read_volatile() }

[Severity: High]
If the original type T is something with a 1-byte alignment like [u8], does
SysMem::new() allow the base pointer to also be 1-byte aligned?

The offset checks in the Io framework only guarantee that the offset is
aligned for the target type, not the absolute resulting pointer. When
SysMemBackend::project_view() creates a view for a larger type like u32, the
resulting pointer could be unaligned.

Could this lead to undefined behavior and kernel panics on architectures like
ARM or RISC-V when read_volatile() is executed here on an unaligned pointer?

> +            }
> +
> +            #[inline]
> +            fn io_write(view: SysMem<'_, $ty>, value: $ty) {
> +                // SAFETY:
> +                // - Per type invariant, `ptr` is valid and aligned.
> +                // - Using write_volatile() here so that race with hardware is well-defined.
> +                // - Using write_volatile() here is not sound if it races with other CPU per Rust
> +                //   rules, but this is allowed per LKMM.
> +                unsafe { view.ptr.write_volatile(value) }
> +            }
> +        }
> +    };
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611-io_projection-v4-0-1f7224b02dcb@garyguo.net?part=14