> On 6 Jul 2026, at 09:44, Gary Guo <[email protected]> wrote:
>
> The current safety comment on `io_read`/`io_write` does not cover the topic
> about alignment. Add it so it can be relied on by implementor of
> `IoCapable`.
>
> Expand the check performed by `Io` by taking `self.addr()` into
> consideration when checking if `offset` is aligned. For the compile-time
> `io_addr_assert` check, check using the known minimum alignment of
> `Io::Target` and the accessed type.
>
> While at it, fix the alignment check to use `align_of` instead of
> `size_of`. The values match for all primitives (including u64, given that
> we do not provide u64 accessor on 32-bit platforms), but are not
> necessarily true for custom types.
>
> Reviewed-by: Alexandre Courbot <[email protected]>
> Signed-off-by: Gary Guo <[email protected]>
> ---
> rust/kernel/io.rs | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index d1c5f0121994..d821ee48ed31 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -195,13 +195,14 @@ pub fn maxsize(&self) -> usize {
> #[repr(transparent)]
> pub struct Mmio<const SIZE: usize = 0>(MmioRaw<SIZE>);
>
> -/// Checks whether an access of type `U` at the given `offset`
> +/// Checks whether an access of type `U` at the given `base` and the given
> `offset`
> /// is valid within this region.
> +///
> +/// The `base` is used for alignment checking only. This can be set to 0 to
> skip the check.
> #[inline]
> -const fn offset_valid<U>(offset: usize, size: usize) -> bool {
> - let type_size = core::mem::size_of::<U>();
> - if let Some(end) = offset.checked_add(type_size) {
> - end <= size && offset % type_size == 0
> +const fn offset_valid<U>(base: usize, offset: usize, size: usize) -> bool {
> + if let Some(end) = offset.checked_add(size_of::<U>()) {
> + end <= size && (base.wrapping_add(offset) % align_of::<U>() == 0)
> } else {
> false
> }
> @@ -220,14 +221,16 @@ pub trait IoCapable<T> {
> ///
> /// # Safety
> ///
> - /// The range `[address..address + size_of::<T>()]` must be within the
> bounds of `Self`.
> + /// - The range `[address..address + size_of::<T>()]` must be within the
> bounds of `Self`.
> + /// - `address` must be aligned.
> unsafe fn io_read(&self, address: usize) -> T;
>
> /// Performs an I/O write of `value` at `address`.
> ///
> /// # Safety
> ///
> - /// The range `[address..address + size_of::<T>()]` must be within the
> bounds of `Self`.
> + /// - The range `[address..address + size_of::<T>()]` must be within the
> bounds of `Self`.
> + /// - `address` must be aligned.
> unsafe fn io_write(&self, value: T, address: usize);
> }
>
> @@ -309,7 +312,11 @@ pub trait Io {
> // Always inline to optimize out error path of `build_assert`.
> #[inline(always)]
> fn io_addr_assert<U>(&self, offset: usize) -> usize {
> - build_assert!(offset_valid::<U>(offset, Self::Target::MIN_SIZE));
> + // We cannot check alignment with `offset_valid` using
> `self.addr()`. So set 0 for it and
> + // ensure alignment by checking that the alignment of `U` is smaller
> or equal to the
> + // alignment of `Self::Target`.
> + const_assert!(Alignment::of::<U>().as_usize() <=
> Self::Target::MIN_ALIGN.as_usize());
> + build_assert!(offset_valid::<U>(0, offset, Self::Target::MIN_SIZE));
>
> self.addr() + offset
> }
> @@ -318,7 +325,7 @@ fn io_addr_assert<U>(&self, offset: usize) -> usize {
> /// performing runtime bound checks.
> #[inline]
> fn io_addr<U>(&self, offset: usize) -> Result<usize> {
> - if !offset_valid::<U>(offset, self.maxsize()) {
> + if !offset_valid::<U>(self.addr(), offset, self.maxsize()) {
> return Err(EINVAL);
> }
>
>
> --
> 2.54.0
>
Reviewed-by: Daniel Almeida <[email protected]>
