回复: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32

[ai qubits]

Thanks for the question, Alexandre, and for the clarification, Danilo.

I agree that usize is the more appropriate type for an index-oriented
Rust API. Callers are likely to obtain indices from ranges, iterators, or
other Rust indexing operations, which naturally use usize.

The unsigned int expected by pci_irq_vector() is an implementation detail
of the C interface, so the necessary validation and conversion should
remain inside the Rust abstraction rather than being exposed to callers.

I will therefore keep IrqVectorRegistration::index() accepting usize and
handle the C API boundary internally.

Best regards,
Sophon

________________________________________
发件人: Alexandre Courbot <acourbot@xxxxxxxxxx>
发送时间: 2026年9月1日 18:58
收件人: Sophon Zhang via B4 Relay
抄送: aiqubits@xxxxxxxxxxx; Danilo Krummrich; Bjorn Helgaas; Krzysztof Wilczyński; Miguel Ojeda; Boqun Feng; Gary Guo; Björn Roy Baron; Benno Lossin; Andreas Hindborg; Alice Ryhl; Trevor Gross; Daniel Almeida; Tamir Duberstein; Onur Özkan; linux-pci@xxxxxxxxxxxxxxx; rust-for-linux@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
主题: Re: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32

On Tue Sep 1, 2026 at 2:09 AM JST, Sophon Zhang via B4 Relay wrote:
> From: Sophon Zhang <aiqubits@xxxxxxxxxxx>
>
> IrqVectorRegistration::index() accepts a usize, but pci_irq_vector()
> takes an unsigned int. On 64-bit architectures, casting an index larger
> than u32::MAX wraps it before the PCI core can validate it. In
> particular, u32::MAX + 1 becomes zero and can resolve to the first
> allocated vector.
>
> Use a checked conversion and return EINVAL when the index cannot be
> represented by the C API.
>
> Fixes: 2fb7755b0a7e ("rust: pci: resolve IRQ in index() and embed IrqRequest in IrqVector")
> Signed-off-by: Sophon Zhang <aiqubits@xxxxxxxxxxx>
> ---
> Prevent 64-bit Rust IRQ vector indices from wrapping when they cross the
> PCI C API boundary.
> ---
> Changes in v4:
> - Drop the explicit length check in favor of PCI core range validation.
> - Use the existing TryFromIntError-to-Error conversion directly.
> - Keep commit trailers adjacent and narrow the description to truncation.
> - Link to v3: https://patch.msgid.link/20260831-fix-pci-irq-vector-index-truncation-v3-1-a2103084d20e@xxxxxxxxxxx
>
> Changes in v3:
> - Check the index against the allocated vector count before entering the C API.
> - Keep the checked usize-to-u32 conversion and document the C-side warning.
> - Link to v2: https://patch.msgid.link/20260831-fix-pci-irq-vector-index-truncation-v2-1-4030ea7746a9@xxxxxxxxxxx
>
> Changes in v2:
> - No code changes.
> - Link to v1: https://patch.msgid.link/20260831-fix-pci-irq-vector-index-truncation-v1-1-d63217d99b67@xxxxxxxxxxx
>
> Testing:
> - make rustfmtcheck
> - Not build- or hardware-tested; bindgen is unavailable in the test environment.
> ---
>  rust/kernel/pci/irq.rs | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index 6741046ec1c0..22e2cdf82a21 100644
> --- a/rust/kernel/pci/irq.rs
> +++ b/rust/kernel/pci/irq.rs
> @@ -151,8 +151,10 @@ pub fn irq_type(&self) -> IrqType {
>      /// [`Self::len()`].
>      #[inline]
>      pub fn index(&self, index: usize) -> Result<IrqVector<'_>> {
> +        let index = u32::try_from(index)?;
> +
>          // SAFETY: `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`.
> -        let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), index as u32) };
> +        let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), index) };
>          if irq < 0 {
>              return Err(Error::from_errno(irq));
>          }

That makes me wonder, shouldn't we make `index` take a `u32` directly?
If that's what the C API expects, it does make sense to align to it
instead of forcing users to make a potential unneeded conversion if they
already have a u32.