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 <[email protected]> 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/[email protected]?part=14
