linux-kernel - Re: [PATCH v7 3/6] rust: io: factor common I/O helpers into Io trait

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DEHU7A4DWOSX.PZ4CCKLAH9QV@nvidia.com>
Date: Tue, 25 Nov 2025 23:14:51 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Alexandre Courbot" <acourbot@...dia.com>, "Zhi Wang" <zhiw@...dia.com>,
 <rust-for-linux@...r.kernel.org>, <linux-pci@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>
Cc: <dakr@...nel.org>, <aliceryhl@...gle.com>, <bhelgaas@...gle.com>,
 <kwilczynski@...nel.org>, <ojeda@...nel.org>, <alex.gaynor@...il.com>,
 <boqun.feng@...il.com>, <gary@...yguo.net>, <bjorn3_gh@...tonmail.com>,
 <lossin@...nel.org>, <a.hindborg@...nel.org>, <tmgross@...ch.edu>,
 <markus.probst@...teo.de>, <helgaas@...nel.org>, <cjia@...dia.com>,
 <smitra@...dia.com>, <ankita@...dia.com>, <aniketa@...dia.com>,
 <kwankhede@...dia.com>, <targupta@...dia.com>, <joelagnelf@...dia.com>,
 <jhubbard@...dia.com>, <zhiwang@...nel.org>
Subject: Re: [PATCH v7 3/6] rust: io: factor common I/O helpers into Io
 trait

On Tue Nov 25, 2025 at 11:09 PM JST, Alexandre Courbot wrote:
> On Wed Nov 19, 2025 at 8:21 PM JST, Zhi Wang wrote:
> <snip>
>> -impl<const SIZE: usize> Io<SIZE> {
>> -    /// Converts an `IoRaw` into an `Io` instance, providing the accessors to the MMIO mapping.
>> -    ///
>> -    /// # Safety
>> -    ///
>> -    /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of size
>> -    /// `maxsize`.
>> -    pub unsafe fn from_raw(raw: &IoRaw<SIZE>) -> &Self {
>> -        // SAFETY: `Io` is a transparent wrapper around `IoRaw`.
>> -        unsafe { &*core::ptr::from_ref(raw).cast() }
>> +/// Checks whether an access of type `U` at the given `offset`
>> +/// is valid within this region.
>> +#[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
>> +    } else {
>> +        false
>>      }
>> +}
>> +
>> +/// Represents a region of I/O space of a fixed size.
>> +///
>> +/// Provides common helpers for offset validation and address
>> +/// calculation on top of a base address and maximum size.
>> +///
>> +pub trait Io {
>> +    /// Minimum usable size of this region.
>> +    const MIN_SIZE: usize;
>
> This associated constant should probably be part of `IoInfallible` -
> otherwise what value should it take if some type only implement
> `IoFallible`?
>
>>  
>>      /// Returns the base address of this mapping.
>> -    #[inline]
>> -    pub fn addr(&self) -> usize {
>> -        self.0.addr()
>> -    }
>> +    fn addr(&self) -> usize;
>>  
>>      /// Returns the maximum size of this mapping.
>> -    #[inline]
>> -    pub fn maxsize(&self) -> usize {
>> -        self.0.maxsize()
>> -    }
>> -
>> -    #[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
>> -        } else {
>> -            false
>> -        }
>> -    }
>> +    fn maxsize(&self) -> usize;
>>  
>> +    /// Returns the absolute I/O address for a given `offset`,
>> +    /// performing runtime bound checks.
>>      #[inline]
>>      fn io_addr<U>(&self, offset: usize) -> Result<usize> {
>> -        if !Self::offset_valid::<U>(offset, self.maxsize()) {
>> +        if !offset_valid::<U>(offset, self.maxsize()) {
>>              return Err(EINVAL);
>>          }
>
> Similarly I cannot find any context where `maxsize` and `io_addr` are
> used outside of `IoFallible`, hinting that these should be part of this
> trait.
>
>>  
>> @@ -239,50 +240,190 @@ fn io_addr<U>(&self, offset: usize) -> Result<usize> {
>>          self.addr().checked_add(offset).ok_or(EINVAL)
>>      }
>>  
>> +    /// Returns the absolute I/O address for a given `offset`,
>> +    /// performing compile-time bound checks.
>>      #[inline]
>>      fn io_addr_assert<U>(&self, offset: usize) -> usize {
>> -        build_assert!(Self::offset_valid::<U>(offset, SIZE));
>> +        build_assert!(offset_valid::<U>(offset, Self::MIN_SIZE));
>>  
>>          self.addr() + offset
>>      }
>
> ... and `io_addr_assert` is only used from `IoFallible`.
>
> So if my gut feeling is correct, we can disband `Io` entirely and only

... except that we can't due to `addr`, unless we find a better way to
provide this base. But even if we need to keep `Io`, the compile-time
and runtime members should be moved to their respective traits.