linux-kernel - Re: [PATCH v3 1/2] rust: irq: add support for request

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: <A7E3A124-AF77-4A4A-B4E2-AE7DDB1CE007@collabora.com>
Date: Thu, 15 May 2025 08:54:35 -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 1/2] rust: irq: add support for request_irq()

Hi Danilo,

> On 14 May 2025, at 18:53, Danilo Krummrich <dakr@...nel.org> wrote:
> 
> On Wed, May 14, 2025 at 04:20:51PM -0300, Daniel Almeida wrote:
>> +/// // This is running in process context.
>> +/// fn register_irq(irq: u32, handler: Handler) -> Result<Arc<Registration<Handler>>> {
>> +///     let registration = Registration::register(irq, flags::SHARED, c_str!("my-device"), handler);
>> +///
>> +///     // You can have as many references to the registration as you want, so
>> +///     // multiple parts of the driver can access it.
>> +///     let registration = Arc::pin_init(registration, GFP_KERNEL)?;
> 
> This makes it possible to arbitrarily extend the lifetime of an IRQ
> registration. However, we must guarantee that the IRQ is unregistered when the
> corresponding device is unbound. We can't allow drivers to hold on to device
> resources after the corresponding device has been unbound.
> 
> Why does the data need to be part of the IRQ registration itself? Why can't we
> pass in an Arc<T> instance already when we register the IRQ?
> 
> This way we'd never have a reason to ever access the Registration instance
> itself ever again and we can easily wrap it as Devres<irq::Registration> -
> analogously to devm_request_irq() on the C side - without any penalties.
> 
>> +///     // The handler may be called immediately after the function above
>> +///     // returns, possibly in a different CPU.
>> +///
>> +///     {
>> +///         // The data can be accessed from the process context too.
>> +///         let mut data = registration.handler().0.lock();
>> +///         *data = 42;
>> +///     }
>> +///
>> +///     Ok(registration)
>> +/// }
> 

Up until this point, there was no need for the data to not be inline with the
registration. This new design would force an Arc, which, apart from the
heap-allocation, is restrictive for users.

Can’t we use Devres with the current implementation?

IIUC from a very cursory glance, all that would mean is that you'd have to call
try_access() on your handler, which should be fine?

— Daniel