From: Sophon Zhang <aiqubits@hotmail.com>

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@hotmail.com>
---
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@hotmail.com

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@hotmail.com

Changes in v2:
- No code changes.
- Link to v1: https://patch.msgid.link/20260831-fix-pci-irq-vector-index-truncation-v1-1-d63217d99b67@hotmail.com

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

---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260831-fix-pci-irq-vector-index-truncation-6752f3751a0d

Best regards,
--  
Sophon Zhang <aiqubits@hotmail.com>

On Tue, 01 Sep 2026 01:09:53 +0800, Sophon Zhang wrote:
> [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32

Applied, thanks!

  Branch: driver-core-linus
  Tree:   git://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git

[1/1] rust: pci: reject IRQ vector indices that do not fit in u32
      commit: 8d7b3e41ffec

The patch will appear in the next linux-next integration (typically within 24
hours on weekdays).

The patch is queued up for Linus's tree and should land in the next -rc release.

On Mon Aug 31, 2026 at 6:09 PM BST, Sophon Zhang via B4 Relay wrote:
> From: Sophon Zhang <aiqubits@hotmail.com>
> 
> 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@hotmail.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
> 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@hotmail.com
> 
> 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@hotmail.com
> 
> Changes in v2:
> - No code changes.
> - Link to v1: https://patch.msgid.link/20260831-fix-pci-irq-vector-index-truncation-v1-1-d63217d99b67@hotmail.com
> 
> 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(-)

On Tue Sep 1, 2026 at 2:09 AM JST, Sophon Zhang via B4 Relay wrote:
> From: Sophon Zhang <aiqubits@hotmail.com>
>
> 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@hotmail.com>
> ---
> 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@hotmail.com
>
> 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@hotmail.com
>
> Changes in v2:
> - No code changes.
> - Link to v1: https://patch.msgid.link/20260831-fix-pci-irq-vector-index-truncation-v1-1-d63217d99b67@hotmail.com
>
> 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.

On Tue, Sep 1, 2026 at 12:58 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> 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.

In general, Rust users shouldn't be dealing with C APIs, so if they
have a `u32` then it usually is because we exposed it from somewhere
else (e.g. we returned it to them) or because that is generally the
right underlying type, in which case it may make sense to align
everything.

But even in those cases, it may have made sense to define a Rust
newtype or similar instead.

So other than exceptional cases, the types that C APIs use shouldn't
drive the decisions on the Rust signatures, since they shouldn't be
seen by the Rust users to begin with.

That definitely introduces some friction on our side, but it does give
us a lot of freedom defining APIs the best way we can, which is a
major advantage, i.e. since we have to provide these abstractions, it
is a good time to be able to clean old decisions and improve on them
using whatever tools Rust give us.

Otherwise, one could also argue we should be passing the underlying
types in general, e.g. even pointers.

(Of course, you know this, I am just elaborating; and obviously there
may be cases it may make sense to just use the underlying type since
it is something that has never changed or is fixed due to some
"standard" etc.).

Cheers,
Miguel

On Tue Sep 1, 2026 at 12:58 PM CEST, Alexandre Courbot wrote:
> 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.

I intentionally did not do this, as the common type for an index is usize. Thus,
I do not expect anyone to already have a u32, but to already have a usize, e.g.
from some iterator.

The fact that the C API did pick unsigned int as index type is an implementation
detail the abstraction should bother with.

On Tue Sep 1, 2026 at 8:08 PM JST, Danilo Krummrich wrote:
> On Tue Sep 1, 2026 at 12:58 PM CEST, Alexandre Courbot wrote:
>> 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.
>
> I intentionally did not do this, as the common type for an index is usize. Thus,
> I do not expect anyone to already have a u32, but to already have a usize, e.g.
> from some iterator.
>
> The fact that the C API did pick unsigned int as index type is an implementation
> detail the abstraction should bother with.

Thanks for the clarification (and Miguel for elaborating - I wasn't
completely aware of it!). In that case this patch looks correct to me.

Note that there is another `as` right after, in the same method:

  if irq < 0 {
      return Err(Error::from_errno(irq));
  }

  // SAFETY: `irq` is a valid IRQ number for `self.dev`, resolved from this registration.
  Ok(unsafe { IrqVector::new(IrqRequest::new(self.dev.as_ref(), irq as u32), self) })


We could get rid of it by replacing the `if irq < 0` test with:

  let irq = u32::try_from(irq).map_err(|_| Error::from_errno(irq))?;

Sophon, if you feel like doing it I think this could improve the patch
further.

On Tue Sep 1, 2026 at 2:32 PM BST, Alexandre Courbot wrote:
> On Tue Sep 1, 2026 at 8:08 PM JST, Danilo Krummrich wrote:
>> On Tue Sep 1, 2026 at 12:58 PM CEST, Alexandre Courbot wrote:
>>> 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.
>>
>> I intentionally did not do this, as the common type for an index is usize. Thus,
>> I do not expect anyone to already have a u32, but to already have a usize, e.g.
>> from some iterator.
>>
>> The fact that the C API did pick unsigned int as index type is an implementation
>> detail the abstraction should bother with.
>
> Thanks for the clarification (and Miguel for elaborating - I wasn't
> completely aware of it!). In that case this patch looks correct to me.
>
> Note that there is another `as` right after, in the same method:
>
>   if irq < 0 {
>       return Err(Error::from_errno(irq));
>   }
>
>   // SAFETY: `irq` is a valid IRQ number for `self.dev`, resolved from this registration.
>   Ok(unsafe { IrqVector::new(IrqRequest::new(self.dev.as_ref(), irq as u32), self) })
>
>
> We could get rid of it by replacing the `if irq < 0` test with:
>
>   let irq = u32::try_from(irq).map_err(|_| Error::from_errno(irq))?;

I wonder if we can just change kernel::error::to_result to return `Result<u32>`
instead and have the cast there?

Best,
Gary

On Tue Sep 1, 2026 at 10:48 PM JST, Gary Guo wrote:
> On Tue Sep 1, 2026 at 2:32 PM BST, Alexandre Courbot wrote:
>> On Tue Sep 1, 2026 at 8:08 PM JST, Danilo Krummrich wrote:
>>> On Tue Sep 1, 2026 at 12:58 PM CEST, Alexandre Courbot wrote:
>>>> 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.
>>>
>>> I intentionally did not do this, as the common type for an index is usize. Thus,
>>> I do not expect anyone to already have a u32, but to already have a usize, e.g.
>>> from some iterator.
>>>
>>> The fact that the C API did pick unsigned int as index type is an implementation
>>> detail the abstraction should bother with.
>>
>> Thanks for the clarification (and Miguel for elaborating - I wasn't
>> completely aware of it!). In that case this patch looks correct to me.
>>
>> Note that there is another `as` right after, in the same method:
>>
>>   if irq < 0 {
>>       return Err(Error::from_errno(irq));
>>   }
>>
>>   // SAFETY: `irq` is a valid IRQ number for `self.dev`, resolved from this registration.
>>   Ok(unsafe { IrqVector::new(IrqRequest::new(self.dev.as_ref(), irq as u32), self) })
>>
>>
>> We could get rid of it by replacing the `if irq < 0` test with:
>>
>>   let irq = u32::try_from(irq).map_err(|_| Error::from_errno(irq))?;
>
> I wonder if we can just change kernel::error::to_result to return `Result<u32>`
> instead and have the cast there?

Looks like our messages crossed [1]. :)

Converting `to_result` would require quite a bit of work to update all
the callers, but maybe we can introduce a new variant indeed.

But this makes me think of another step we can take to harden
`to_result`: it should probably warn if the non-error value if not `0`,
as that would indicate the caller needs to consider it.

[1] https://lore.kernel.org/all/DL412XWBP4Y2.K1TH9NELBKPR@nvidia.com/

On 9/1/26 3:32 PM, Alexandre Courbot wrote:
> On Tue Sep 1, 2026 at 8:08 PM JST, Danilo Krummrich wrote:
>> On Tue Sep 1, 2026 at 12:58 PM CEST, Alexandre Courbot wrote:
>>> 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.
>>
>> I intentionally did not do this, as the common type for an index is usize. Thus,
>> I do not expect anyone to already have a u32, but to already have a usize, e.g.
>> from some iterator.
>>
>> The fact that the C API did pick unsigned int as index type is an implementation
>> detail the abstraction should bother with.
> 
> Thanks for the clarification (and Miguel for elaborating - I wasn't
> completely aware of it!). In that case this patch looks correct to me.
> 
> Note that there is another `as` right after, in the same method:
> 
>   if irq < 0 {
>       return Err(Error::from_errno(irq));
>   }
> 
>   // SAFETY: `irq` is a valid IRQ number for `self.dev`, resolved from this registration.
>   Ok(unsafe { IrqVector::new(IrqRequest::new(self.dev.as_ref(), irq as u32), self) })
> 
> 
> We could get rid of it by replacing the `if irq < 0` test with:
> 
>   let irq = u32::try_from(irq).map_err(|_| Error::from_errno(irq))?;

That's a good suggestion!

> Sophon, if you feel like doing it I think this could improve the patch
> further.

Let's please do that as a separate patch though.

Thanks,
Danilo

On Tue Sep 1, 2026 at 10:36 PM JST, Danilo Krummrich wrote:
> On 9/1/26 3:32 PM, Alexandre Courbot wrote:
>> On Tue Sep 1, 2026 at 8:08 PM JST, Danilo Krummrich wrote:
>>> On Tue Sep 1, 2026 at 12:58 PM CEST, Alexandre Courbot wrote:
>>>> 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.
>>>
>>> I intentionally did not do this, as the common type for an index is usize. Thus,
>>> I do not expect anyone to already have a u32, but to already have a usize, e.g.
>>> from some iterator.
>>>
>>> The fact that the C API did pick unsigned int as index type is an implementation
>>> detail the abstraction should bother with.
>> 
>> Thanks for the clarification (and Miguel for elaborating - I wasn't
>> completely aware of it!). In that case this patch looks correct to me.
>> 
>> Note that there is another `as` right after, in the same method:
>> 
>>   if irq < 0 {
>>       return Err(Error::from_errno(irq));
>>   }
>> 
>>   // SAFETY: `irq` is a valid IRQ number for `self.dev`, resolved from this registration.
>>   Ok(unsafe { IrqVector::new(IrqRequest::new(self.dev.as_ref(), irq as u32), self) })
>> 
>> 
>> We could get rid of it by replacing the `if irq < 0` test with:
>> 
>>   let irq = u32::try_from(irq).map_err(|_| Error::from_errno(irq))?;
>
> That's a good suggestion!

I expect this pattern to be common, so how about we add a variant of
`error::to_result` that returns the value as a `u32`? I.e.

  pub fn to_result_value(err: crate::ffi::c_int) -> Result<u32>

>
>> Sophon, if you feel like doing it I think this could improve the patch
>> further.
>
> Let's please do that as a separate patch though.

In this case, the current patch is

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>