linux-kernel - Re: [PATCH v7 07/16] rust: add `io::{Io, IoRaw}` base types

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <1fef097c-d2d8-4d98-ab83-09ad5ae0b2e1@kernel.org>
Date: Mon, 20 Jan 2025 13:54:37 +0100
From: Danilo Krummrich <dakr@...nel.org>
To: Fiona Behrens <me@...enk.dev>
Cc: gregkh@...uxfoundation.org, rafael@...nel.org, bhelgaas@...gle.com,
 ojeda@...nel.org, alex.gaynor@...il.com, boqun.feng@...il.com,
 gary@...yguo.net, bjorn3_gh@...tonmail.com, benno.lossin@...ton.me,
 tmgross@...ch.edu, a.hindborg@...sung.com, aliceryhl@...gle.com,
 airlied@...il.com, fujita.tomonori@...il.com, lina@...hilina.net,
 pstanner@...hat.com, ajanulgu@...hat.com, lyude@...hat.com, robh@...nel.org,
 daniel.almeida@...labora.com, saravanak@...gle.com, dirk.behme@...bosch.com,
 j@...nau.net, fabien.parent@...aro.org, chrisi.schrefl@...il.com,
 paulmck@...nel.org, rust-for-linux@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
 devicetree@...r.kernel.org, rcu@...r.kernel.org
Subject: Re: [PATCH v7 07/16] rust: add `io::{Io, IoRaw}` base types

On 1/16/25 11:31 AM, Fiona Behrens wrote:
> On 19 Dec 2024, at 18:04, Danilo Krummrich wrote:

>> +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() }
>> +    }
>> +
>> +    /// Returns the base address of this mapping.
>> +    #[inline]
>> +    pub fn addr(&self) -> usize {
>> +        self.0.addr()
>> +    }
>> +
>> +    /// 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
>> +        }
>> +    }
>> +
>> +    #[inline]
>> +    fn io_addr<U>(&self, offset: usize) -> Result<usize> {
>> +        if !Self::offset_valid::<U>(offset, self.maxsize()) {
>> +            return Err(EINVAL);
>> +        }
>> +
>> +        // Probably no need to check, since the safety requirements of `Self::new` guarantee that
>> +        // this can't overflow.
>> +        self.addr().checked_add(offset).ok_or(EINVAL)
>> +    }
>> +
>> +    #[inline]
>> +    fn io_addr_assert<U>(&self, offset: usize) -> usize {
>> +        build_assert!(Self::offset_valid::<U>(offset, SIZE));
>> +
>> +        self.addr() + offset
>> +    }
> 
> Currently reworking the portmem abstractions I wrote for the LED/SE10 driver.
> Right now I’m wondering if it would make sense to move the 3 functions above (`offset_valid`, `io_addr` and `io_addr_assert` into IoRaw),
> as I’m considering reusing the IoRaw in the portmem and then just offer the outb/outw/outl functions on a wraping type around IoRaw.
> For this I would also use the same functions to check bounds.

Sure, feel free to move them. I think I should have moved those functions to
IoRaw from the get-go.

- Danilo