Re: [PATCH v3 03/11] gpu: nova-core: vbios: use checked ops and accesses in `FwSecBiosImage::ucode`

["Alexandre Courbot" <acourbot@nvidia.com>]

On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
> Use checked arithmetic and access for extracting the microcode since the
> offsets are firmware derived.
>
> Fixes: 47c4846e4319 ("gpu: nova-core: vbios: Add support for FWSEC ucode extraction")
> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/vbios.rs | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index de856000de23..632c8a90ea76 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -1029,16 +1029,21 @@ pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
>  
>      /// Get the ucode data as a byte slice
>      pub(crate) fn ucode(&self, desc: &FalconUCodeDesc) -> Result<&[u8]> {
> -        let falcon_ucode_offset = self.falcon_ucode_offset;
> -
>          // The ucode data follows the descriptor.
> -        let ucode_data_offset = falcon_ucode_offset + desc.size();
> -        let size = usize::from_safe_cast(desc.imem_load_size() + desc.dmem_load_size());
> +        let data = self
> +            .base
> +            .data
> +            .get(self.falcon_ucode_offset..)
> +            .ok_or(ERANGE)?;
> +        let size = usize::from_safe_cast(
> +            desc.imem_load_size()
> +                .checked_add(desc.dmem_load_size())
> +                .ok_or(ERANGE)?,
> +        );
>  
>          // Get the data slice, checking bounds in a single operation.
> -        self.base
> -            .data
> -            .get(ucode_data_offset..ucode_data_offset + size)
> +        data.get(desc.size()..)
> +            .and_then(|data| data.get(..size))
>              .ok_or(ERANGE)
>              .inspect_err(|_| {
>                  dev_err!(

This looks like we are doing part of the operation, stop to compute
size, and then resume the operation. As a result the `inspect_err` only
applies to the last `get` operation.

The following would flow better IMHO:

    let size = usize::from_safe_cast(
        desc.imem_load_size()
            .checked_add(desc.dmem_load_size())
            .ok_or(ERANGE)?
    );

    self.base
        .data
        .get(self.falcon_ucode_offset..)
        .and_then(|data| data.get(desc.size()..))
        .and_then(|data| data.get(..size))
        .ok_or(ERANGE)
        .inspect_err(|_| {
            dev_err!(
                self.base.dev,
                "fwsec ucode data not contained within BIOS bounds\n"
            )
        })