linux-kernel - Re: [PATCH v4 2/3] rust: io: mem: add a generic iomem abstraction

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: <EAAFB9D3-D83B-44B9-95CA-D09BE7C7F248@collabora.com>
Date: Thu, 9 Jan 2025 12:33:34 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: alex.gaynor@...il.com,
 boqun.feng@...il.com,
 gary@...yguo.net,
 bjorn3_gh@...tonmail.com,
 benno.lossin@...ton.me,
 a.hindborg@...nel.org,
 tmgross@...ch.edu,
 gregkh@...uxfoundation.org,
 rafael@...nel.org,
 dakr@...nel.org,
 boris.brezillon@...labora.com,
 rust-for-linux@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/3] rust: io: mem: add a generic iomem abstraction

Hi Alice,

> 
>> +pub struct IoMem<const SIZE: usize = 0> {
>> +    io: IoRaw<SIZE>,
>> +    res_start: u64,
>> +    exclusive: bool,
> 
> I don't think you need this after the `new` call, so you can remove
> the field from the struct.

Remove what?

Both res_start and exclusive are used in IoMem::drop().

> 
>> +    /// # Safety
>> +    ///
>> +    /// The caller must ensure that the underlying resource remains valid
>> +    /// throughout the `IoMem`'s lifetime. This is usually done by wrapping the
>> +    /// `IoMem` in a `Devres` instance, which will properly revoke the access
>> +    /// when the device is unbound from the matched driver.
> 
> I know that this is a requirement for IoRaw, but does the
> `request_mem_region` call not guarantee that the memory region stays
> valid until `drop` runs?

Hmm, I don’t think so. I’d expect the lifetime of the iomem to be tied to the lifetime
of the device, really. The mapping will probably remain, but other infrastructure will probably
be shutdown, like the clocks or regulators, so accessing the region will probably deadlock in such cases.

Other people feel free to correct me, of course ^
 
— Daniel