linux-kernel - Re: [PATCH v2] rust: pci: Allocate and manage PCI interrupt vectors

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: <58a26b94-bf06-413e-a61c-2e0d71de2ac7@nvidia.com>
Date: Wed, 8 Oct 2025 14:45:50 -0400
From: Joel Fernandes <joelagnelf@...dia.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 nouveau@...ts.freedesktop.org, rust-for-linux@...r.kernel.org,
 linux-pci@...r.kernel.org, acourbot@...dia.com,
 Alistair Popple <apopple@...dia.com>, 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 <lossin@...nel.org>,
 Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
 Trevor Gross <tmgross@...ch.edu>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>, John Hubbard <jhubbard@...dia.com>,
 Timur Tabi <ttabi@...dia.com>, joel@...lfernandes.org,
 Daniel Almeida <daniel.almeida@...labora.com>,
 Bjorn Helgaas <bhelgaas@...gle.com>,
 Krzysztof Wilczyński <kwilczynski@...nel.org>
Subject: Re: [PATCH v2] rust: pci: Allocate and manage PCI interrupt vectors

Hi Danilo,

On 10/5/2025 8:56 AM, Danilo Krummrich wrote:
> 
>> +    /// Allocate IRQ vectors for this PCI device with automatic cleanup.
>> +    ///
>> +    /// Allocates between `min_vecs` and `max_vecs` interrupt vectors for the device.
>> +    /// The allocation will use MSI-X, MSI, or legacy interrupts based on the `irq_types`
>> +    /// parameter and hardware capabilities. When multiple types are specified, the kernel
>> +    /// will try them in order of preference: MSI-X first, then MSI, then legacy interrupts.
>> +    ///
>> +    /// The allocated vectors are automatically freed when the device is unbound, using the
>> +    /// devres (device resource management) system.
>> +    ///
>> +    /// # Arguments
>> +    ///
>> +    /// * `min_vecs` - Minimum number of vectors required
>> +    /// * `max_vecs` - Maximum number of vectors to allocate
>> +    /// * `irq_types` - Types of interrupts that can be used
>> +    ///
>> +    /// # Returns
>> +    ///
>> +    /// Returns a range of IRQ vectors that were successfully allocated, or an error if the
>> +    /// allocation fails or cannot meet the minimum requirement.
>> +    ///
>> +    /// # Examples
>> +    ///
>> +    /// ```ignore
>> +    /// // Allocate using any available interrupt type in the order mentioned above.
>> +    /// let vectors = dev.alloc_irq_vectors(1, 32, IrqTypes::all())?;
>> +    ///
>> +    /// // Allocate MSI or MSI-X only (no legacy interrupts)
>> +    /// let msi_only = IrqTypes::default()
>> +    ///     .with(IrqType::Msi)
>> +    ///     .with(IrqType::MsiX);
>> +    /// let vectors = dev.alloc_irq_vectors(4, 16, msi_only)?;
>> +    /// ```
>> +    pub fn alloc_irq_vectors(
>> +        &self,
>> +        min_vecs: u32,
>> +        max_vecs: u32,
>> +        irq_types: IrqTypes,
>> +    ) -> Result<RangeInclusive<IrqVector<'_>>> {
>> +        let (irq_vecs, range) = IrqVectorRegistration::new(self, min_vecs, max_vecs, irq_types)?;
>> +
>> +        devres::register(self.as_ref(), irq_vecs, GFP_KERNEL)?;
>> If we move the call to devres::register() into IrqVectorRegistration::new()
> (which I'd call IrqVectorRegistration::register() then) we can enforce the
> guarantee that an IrqVectorRegistration must not out-live the device / driver
> binding internally.

Great idea, so paraphrasing for myself, your point is with the above code,
someone could theoretically do:

  1. Call new() directly on IrqVectorRegistration (bypassing alloc_irq_vectors()).
  2. Forget to call devres::register().
  3. Store the IrqVectorRegistration somewhere.
  4. Device gets unbound.
  5. Later when IrqVectorRegistration::drop() runs, it tries to free vectors on
a device that's gone.

Is that right?

So a better approach as you mentioned, is to do the devres registration during
the construction of the IrqVectorRegistration, so there's no way to do one
without the other. Did I get that right? Anyway great point and I have made this
change, thanks!

 - Joel