Re: [PATCH v3 3/5] rust: pci: remove request_irq() and request_threaded_irq() from Device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The new `IrqVectorRegistration` lifetime design creates a self-referential lifetime trap, making it practically impossible for drivers to safely store their IRQ handler registrations in their driver data structures.
--

--- Patch [3]: [PATCH v3 3/5] rust: pci: remove request_irq() and request_threaded_irq() from Device ---
commit 391f2070be9b225ebd0a122e28fa0091732c94e1
Author: Danilo Krummrich <dakr@kernel.org>

rust: pci: remove request_irq() and request_threaded_irq() from Device

Remove the thin wrappers on Device<Bound> that only forwarded to
irq::Registration::new() and irq::ThreadedRegistration::new(). With
IrqVector embedding a resolved IrqRequest, the conversion is infallible
and drivers call irq::Registration::new(vector.into(), ...) directly.

> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index 81b74c4c17d98..b6d1699ee3ebf 100644
> --- a/rust/kernel/pci/irq.rs
> +++ b/rust/kernel/pci/irq.rs
> @@ -70,9 +67,8 @@ const fn as_raw(self) -> u32 {
>  
>  /// A resolved IRQ vector from a PCI interrupt vector allocation.
>  ///
> -/// Created by [`IrqVectorRegistration::index`] and consumed by [`Device::request_irq`] or
> -/// [`Device::request_threaded_irq`]. Borrows the [`IrqVectorRegistration`] it was derived from,
> -/// so the allocation stays live until the handler is freed.
> +/// Created by [`IrqVectorRegistration::index`]. Convert to [`IrqRequest`] via [`From`] to register
> +/// a handler with [`irq::Registration::new`](crate::irq::Registration::new).
>  pub struct IrqVector<'a> {
>      request: IrqRequest<'a>,
>      reg: &'a IrqVectorRegistration<'a>,
>  }

[Severity: High]
Does this new IrqVectorRegistration lifetime design create a self-referential
lifetime trap that makes it impossible for drivers to safely store their IRQ
handler registrations?

Because IrqVector uses a single lifetime parameter 'a for both its embedded
IrqRequest<'a> and its borrow of the registration &'a IrqVectorRegistration<'a>,
the returned IrqRequest is forced to inherit the short lifetime of the method
call's borrow of the IrqVectorRegistration when converted via From:

rust/kernel/pci/irq.rs:
impl<'a> From<IrqVector<'a>> for IrqRequest<'a> {
    #[inline]
    fn from(vector: IrqVector<'a>) -> Self {
        vector.request
    }
}

This conversion propagates the strictly constrained borrow lifetime into the
IrqRequest, permanently tying any resulting irq::Registration to the local
scope of the IrqVectorRegistration. 

For a driver to return from its probe() function, it must store both the vector
registration and the IRQ handler registration in its main device struct.
However, this creates a self-referential struct (the IRQ registration borrows
the sibling vector registration field), which is forbidden in safe Rust.

There is no devres escape hatch because devres::register requires 'static, and
DevresLt cannot encapsulate a borrow of a local field.

This affects all PCI drivers attempting to use the new API, breaking the
primary use case of the subsystem.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813165234.620555-1-dakr@kernel.org?part=3