Re: [PATCH 3/5] gpu: nova-core: vbios: use checked accesses in `setup_falcon_data`

Joel Fernandes
2026-04-10T14:53:24Z

Hi Eliot,

On 4/10/2026 4:38 AM, Eliot Courtney wrote:
> Use checked arithmetic and accesses where the values are firmware
> derived to prevent potential overflow.
>
> Fixes: dc70c6ae2441 ("gpu: nova-core: vbios: Add support to look up PMU table in FWSEC")
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
> drivers/gpu/nova-core/vbios.rs | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index de856000de23..2b0dc1a9125d 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -936,17 +936,12 @@ fn setup_falcon_data(
>
> self.falcon_data_offset = Some(offset);
>
> - if pmu_in_first_fwsec {
> - self.pmu_lookup_table = Some(PmuLookupTable::new(
> - &self.base.dev,
> - &first_fwsec.base.data[offset..],
> - )?);
> + let pmu_lookup_data = if pmu_in_first_fwsec {
> + &first_fwsec.base.data[offset..]

I suggest use get() here as well for consistency with your use of get()
further below.
first_fwsec.base.data.get(offset..).ok_or(EINVAL)?

> } else {
> - self.pmu_lookup_table = Some(PmuLookupTable::new(
> - &self.base.dev,
> - &self.base.data[offset..],
> - )?);
> - }
> + self.base.data.get(offset..).ok_or(EINVAL)?
> + };
> + self.pmu_lookup_table = Some(PmuLookupTable::new(&self.base.dev, pmu_lookup_data)?);
>
> match self
> .pmu_lookup_table
> @@ -955,8 +950,9 @@ fn setup_falcon_data(
> .find_entry_by_type(FALCON_UCODE_ENTRY_APPID_FWSEC_PROD)
> {
> Ok(entry) => {
> - let mut ucode_offset = usize::from_safe_cast(entry.data);
> - ucode_offset -= pci_at_image.base.data.len();
> + let mut ucode_offset = usize::from_safe_cast(entry.data)
> + .checked_sub(pci_at_image.base.data.len())
> + .ok_or(EINVAL)?;
> if ucode_offset < first_fwsec.base.data.len() {
> dev_err!(self.base.dev, "Falcon Ucode offset not in second Fwsec.\n");
> return Err(EINVAL);

How about replace this whole block with:

self.falcon_ucode_offset = Some(
usize::from_safe_cast(entry.data)
.checked_sub(pci_at_image.base.data.len())
.and_then(|o| o.checked_sub(first_fwsec.base.data.len()))
.ok_or(EINVAL)
.inspect_err(|_| {
dev_err!(self.base.dev,
"Falcon Ucode offset not in second Fwsec.\n");
})?,
);

That way, the error message also shows up when
checked_sub(pci_at_image.base.data.len()) fails and it is a bit cleaner.

If you agree with both the above suggestions:

Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>

thanks,

--
Joel Fernandes