linux-kernel - Re: [PATCH v3 2/2] rust: platform: add irq accessors

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: <CA34AB78-D9DC-433A-B6DF-663849A07370@collabora.com>
Date: Mon, 2 Jun 2025 11:56:28 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Miguel Ojeda <ojeda@...nel.org>,
 Alex Gaynor <alex.gaynor@...il.com>,
 Boqun Feng <boqun.feng@...il.com>,
 Gary Guo <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>,
 Benno Lossin <benno.lossin@...ton.me>,
 Andreas Hindborg <a.hindborg@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>,
 Trevor Gross <tmgross@...ch.edu>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 "Rafael J. Wysocki" <rafael@...nel.org>,
 Thomas Gleixner <tglx@...utronix.de>,
 linux-kernel@...r.kernel.org,
 rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v3 2/2] rust: platform: add irq accessors

Hi Danilo,

[…]

>> +
>> +    /// Same as [`Self::irq_by_name`] but does not print an error message if an IRQ
>> +    /// cannot be obtained.
>> +    pub fn optional_irq_by_name(&self, name: &CStr) -> Result<u32> {
>> +        // SAFETY: `self.as_raw` returns a valid pointer to a `struct platform_device`.
>> +        let res = unsafe {
>> +            bindings::platform_get_irq_byname_optional(self.as_raw(), name.as_char_ptr())
>> +        };
>> +
>> +        if res < 0 {
>> +            return Err(Error::from_errno(res));
>> +        }
>> +
>> +        Ok(res as u32)
>> +    }
> 
> I don't like the indirection of claiming a u32 representing the IRQ number from
> a bus device and stuffing it into an irq::Registration.
> 
> It would be better we we'd make it impossible (or at least harder) for a driver
> to pass the wrong number to irq::Registration.
> 
> I see two options:
> 
>  1) Make the platform::Device accessors themselves return an
>     irq::Registration.
> 
>  2) Make the platform::Device accessors return some kind of transparent cookie,
>     that drivers can't create themselves that can be fed into the
>     irq::Registration.
> 
> My preference would be 1) if there's no major ergonomic issue with that.

Isn’t 1 way more cluttered?

That's because the accessors would have to forward all of the arguments (i.e.:
currently 4) to register().

Going with approach 2 lets us keep the two APIs distinct, we'd only have to
take in the cookie in place of the u32, of course.

— Daniel