Re: [PATCH] rust: pci: Reject IRQ vector indices that do not fit in u32

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing bounds check in `IrqVectorRegistration::index()` allows out-of-bounds indices to trigger a C-side kernel warning and potential kernel panic.
- [High] `Device::alloc_irq_vectors` fails to enforce `min_vecs >= 1`, leading to a resource leak and broken device state when `0` is passed.
--

commit 4de7f8d3881b1c14eada957c4affb1c9815e3096
Author: Sophon Z <aiqubits@hotmail.com>

rust: pci: Reject IRQ vector indices that do not fit in u32

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.

> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index 6741046ec1c09..1bf91973cb1f4 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).map_err(|_| EINVAL)?;

[Severity: High]
This is a pre-existing issue, but does this function also need to verify that
the index is within self.len()?

If IrqVectorRegistration::index() receives an out-of-bounds index that fits
in u32, the bounds check is missed, and it is passed directly to the C PCI
core:

rust/kernel/pci/irq.rs:IrqVectorRegistration::index() {
    ...
    let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), index) };
    ...
}

This can allow large indices to reach msi_domain_get_virq() and trigger a
WARN_ON_ONCE in the C PCI core, which on systems with panic_on_warn=1 will
result in a kernel panic and a complete denial of service.

> +
>          // 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));
>          }

While reviewing this code, I also noticed another pre-existing issue in the
same file:

rust/kernel/pci/irq.rs:Device::alloc_irq_vectors() {
    ...
    let ret = unsafe {
        bindings::pci_alloc_irq_vectors(self.as_raw(), min_vecs, max_vecs, irq_types.as_raw())
    };
    to_result(ret)?;

    let len = NonZero::new(ret as usize).ok_or(EINVAL)?;
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but should alloc_irq_vectors()
enforce min_vecs >= 1 before calling the C API?

If alloc_irq_vectors() is called with min_vecs = 0, the C PCI core
initializes MSI/MSI-X capabilities for 0 vectors and returns 0. The Rust
wrapper treats this 0 return value as an error (NonZero::new fails) and bails
out without invoking pci_free_irq_vectors().

Because no IrqVectorRegistration is constructed, its Drop handler is
bypassed, permanently leaking the mapped MSI-X table and leaving the PCI
device in a broken state where subsequent allocation attempts will fail.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-fix-pci-irq-vector-index-truncation-v1-1-d63217d99b67@hotmail.com?part=1