Re: [PATCH v4 02/20] rust: io: add missing safety requirement in `IoCapable` methods

From: Alexandre Courbot

Date: Mon Jun 15 2026 - 10:10:24 EST

On Mon Jun 15, 2026 at 7:13 PM JST, Gary Guo wrote:
> On Mon Jun 15, 2026 at 5:28 AM BST, Alexandre Courbot wrote:
>> On Fri Jun 12, 2026 at 1:28 AM JST, Gary Guo 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 `Io` by taking `self.addr()` into consideration when
>>
>> "the check performed by `Io`" maybe?
>>
>>> checking if `offset` is aligned. For the compile-time `io_addr_assert`
>>> check, check using the known minimum alignment of `IO::Target` and the
>>
>> typo: s/IO/Io.
>>
>>> 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.
>>>
>>> Signed-off-by: Gary Guo <gary@xxxxxxxxxxx>
>>> ---
>>> 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 bef571dad6eb..fa9ae39ad9d2 100644
>>> --- a/rust/kernel/io.rs
>>> +++ b/rust/kernel/io.rs
>>> @@ -196,13 +196,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(offset: usize, size: usize) -> bool {
>>> - let type_size = core::mem::size_of::();
>>> - if let Some(end) = offset.checked_add(type_size) {
>>> - end <= size && offset % type_size == 0
>>> +const fn offset_valid(base: usize, offset: usize, size: usize) -> bool {
>>> + if let Some(end) = offset.checked_add(size_of::()) {
>>> + end <= size && (base.wrapping_add(offset) % align_of::() == 0)
>>> } else {
>>> false
>>> }
>>> @@ -221,14 +222,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);
>>> }
>>>
>>> @@ -310,7 +313,11 @@ pub trait Io {
>>> // Always inline to optimize out error path of `build_assert`.
>>> #[inline(always)]
>>> fn io_addr_assert(&self, offset: usize) -> usize {
>>> - build_assert!(offset_valid::(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::().as_usize() <= Self::Target::MIN_ALIGN.as_usize());
>>> + build_assert!(offset_valid::(0, offset, Self::Target::MIN_SIZE));
>>
>> IIUC this can allow unaligned accesses if `self.addr()` itself is not
>> properly aligned. Do we need a new `Io` invariant for that or is it
>> already enforced somewhere?
>
> Adding a trait invariant would require marking the trait as `unsafe`, which I
> don't want to do because the `addr()` method is removed later anyway.
>
> One argument is that it's `Io` implementation causing issue for its own if its
> `addr()` is not aligned. This is later redefined using projection and views,
> which further shifts responsiblity of upholding invariants to the `Io` type
> implementator itself.

If we end up removing `addr` anyway it is less critical to solve this.
Especially since this is not a new issue.