[PATCH 0/5] gpu: nova-core: vbios: harden various array accesses

[PATCH 0/5] gpu: nova-core: vbios: harden various array accesses

[Eliot Courtney]

We have some code that accesses arrays based on values from firmware.
This patch series makes a bunch of those accesses more robust. This
series only touches accesses that are not guaranteed to be safe by local
invariants - some accesses are safe due to earlier checks and I haven't
modified those.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
Eliot Courtney (5):
      gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN`
      gpu: nova-core: vbios: limit `BitToken` entry reads
      gpu: nova-core: vbios: use checked accesses in `setup_falcon_data`
      gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header`
      gpu: nova-core: vbios: use checked ops and accesses in `FwSecBiosImage::ucode`

 drivers/gpu/nova-core/vbios.rs | 99 +++++++++++++++++++++---------------------
 1 file changed, 50 insertions(+), 49 deletions(-)
---
base-commit: a7a080bb4236ebe577b6776d940d1717912ff6dd
change-id: 20260409-fix-vbios-d668e9c21d23

Best regards,
--  
Eliot Courtney <ecourtney@nvidia.com>

[PATCH 1/5] gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN`

[Eliot Courtney]

Fix various cases that allow reading past `BIOS_MAX_SCAN_LEN` when
scanning the VBIOS.

Fix bug where `read_more_at_offset` would unnecessarily read more data.
This happens when the window to read has some part cached and some part
not. It would read `len` bytes instead of just the uncached portion,
which could read past `BIOS_MAX_SCAN_LEN`.

Also add more checked arithmetic to catch potential overflows.
`read_bios_image_at_offset` is called with a length from the VBIOS
header, so we should be more defensive here.

Fixes: 6fda04e7f0cd ("gpu: nova-core: vbios: Add base support for VBIOS construction and iteration")
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index ebda28e596c5..6de7e58e0da0 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -132,17 +132,14 @@ fn read_more(&mut self, len: usize) -> Result {
 
     /// Read bytes at a specific offset, filling any gap.
     fn read_more_at_offset(&mut self, offset: usize, len: usize) -> Result {
-        if offset > BIOS_MAX_SCAN_LEN {
+        let end = offset.checked_add(len).ok_or(EINVAL)?;
+
+        if end > BIOS_MAX_SCAN_LEN {
             dev_err!(self.dev, "Error: exceeded BIOS scan limit.\n");
             return Err(EINVAL);
         }
 
-        // If `offset` is beyond current data size, fill the gap first.
-        let current_len = self.data.len();
-        let gap_bytes = offset.saturating_sub(current_len);
-
-        // Now read the requested bytes at the offset.
-        self.read_more(gap_bytes + len)
+        self.read_more(end.saturating_sub(self.data.len()))
     }
 
     /// Read a BIOS image at a specific offset and create a [`BiosImage`] from it.
@@ -155,8 +152,9 @@ fn read_bios_image_at_offset(
         len: usize,
         context: &str,
     ) -> Result<BiosImage> {
+        let end = offset.checked_add(len).ok_or(EINVAL)?;
         let data_len = self.data.len();
-        if offset + len > data_len {
+        if end > data_len {
             self.read_more_at_offset(offset, len).inspect_err(|e| {
                 dev_err!(
                     self.dev,
@@ -167,7 +165,7 @@ fn read_bios_image_at_offset(
             })?;
         }
 
-        BiosImage::new(self.dev, &self.data[offset..offset + len]).inspect_err(|err| {
+        BiosImage::new(self.dev, &self.data[offset..end]).inspect_err(|err| {
             dev_err!(
                 self.dev,
                 "Failed to {} at offset {:#x}: {:?}\n",
@@ -189,7 +187,7 @@ fn next(&mut self) -> Option<Self::Item> {
             return None;
         }
 
-        if self.current_offset > BIOS_MAX_SCAN_LEN {
+        if self.current_offset >= BIOS_MAX_SCAN_LEN {
             dev_err!(self.dev, "Error: exceeded BIOS scan limit, stopping scan\n");
             return None;
         }

-- 
2.53.0

[PATCH 2/5] gpu: nova-core: vbios: limit `BitToken` entry reads

[Eliot Courtney]

If `header.token_size` is smaller than `BitToken`, then we currently can
read past the end of `image.base.data`. Check that the token size is at
least as big as `BitToken`.

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 | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 6de7e58e0da0..de856000de23 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -423,31 +423,31 @@ impl BitToken {
     /// Find a BIT token entry by BIT ID in a PciAtBiosImage
     fn from_id(image: &PciAtBiosImage, token_id: u8) -> Result<Self> {
         let header = &image.bit_header;
+        let entry_size = usize::from(header.token_size);
+
+        if entry_size < size_of::<BitToken>() {
+            return Err(EINVAL);
+        }
 
         // Offset to the first token entry
         let tokens_start = image.bit_offset + usize::from(header.header_size);
 
         for i in 0..usize::from(header.token_entries) {
-            let entry_offset = tokens_start + (i * usize::from(header.token_size));
-
-            // Make sure we don't go out of bounds
-            if entry_offset + usize::from(header.token_size) > image.base.data.len() {
-                return Err(EINVAL);
-            }
+            let entry_offset = tokens_start + (i * entry_size);
+            let entry = image
+                .base
+                .data
+                .get(entry_offset..)
+                .and_then(|data| data.get(..entry_size))
+                .ok_or(EINVAL)?;
 
             // Check if this token has the requested ID
-            if image.base.data[entry_offset] == token_id {
+            if entry[0] == token_id {
                 return Ok(BitToken {
-                    id: image.base.data[entry_offset],
-                    data_version: image.base.data[entry_offset + 1],
-                    data_size: u16::from_le_bytes([
-                        image.base.data[entry_offset + 2],
-                        image.base.data[entry_offset + 3],
-                    ]),
-                    data_offset: u16::from_le_bytes([
-                        image.base.data[entry_offset + 4],
-                        image.base.data[entry_offset + 5],
-                    ]),
+                    id: entry[0],
+                    data_version: entry[1],
+                    data_size: u16::from_le_bytes([entry[2], entry[3]]),
+                    data_offset: u16::from_le_bytes([entry[4], entry[5]]),
                 });
             }
         }

-- 
2.53.0

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

[Eliot Courtney]

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

-- 
2.53.0

[PATCH 4/5] gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header`

[Eliot Courtney]

Use checked access in `FwSecBiosImage::header` for getting the header
version since the value is firmware derived.

Fixes: 47c4846e4319 ("gpu: nova-core: vbios: Add support for FWSEC ucode extraction")
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 2b0dc1a9125d..3bd3ac3a69f2 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -995,14 +995,16 @@ pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
         // Get the falcon ucode offset that was found in setup_falcon_data.
         let falcon_ucode_offset = self.falcon_ucode_offset;
 
+        let data = self.base.data.get(falcon_ucode_offset..).ok_or(EINVAL)?;
+
         // Read the first 4 bytes to get the version.
-        let hdr_bytes: [u8; 4] = self.base.data[falcon_ucode_offset..falcon_ucode_offset + 4]
+        let hdr_bytes: [u8; 4] = data
+            .get(..4)
+            .ok_or(EINVAL)?
             .try_into()
             .map_err(|_| EINVAL)?;
         let hdr = u32::from_le_bytes(hdr_bytes);
         let ver = (hdr & 0xff00) >> 8;
-
-        let data = self.base.data.get(falcon_ucode_offset..).ok_or(EINVAL)?;
         match ver {
             2 => {
                 let v2 = FalconUCodeDescV2::from_bytes_copy_prefix(data)

-- 
2.53.0

[PATCH 5/5] gpu: nova-core: vbios: use checked ops and accesses in `FwSecBiosImage::ucode`

[Eliot Courtney]

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")
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 3bd3ac3a69f2..b509cd8407a5 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -1027,16 +1027,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!(

-- 
2.53.0

Re: [PATCH 1/5] gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN`

[Joel Fernandes]

On 4/10/2026 4:38 AM, Eliot Courtney wrote:
> Fix various cases that allow reading past `BIOS_MAX_SCAN_LEN` when
> scanning the VBIOS.
> 
> Fix bug where `read_more_at_offset` would unnecessarily read more data.
> This happens when the window to read has some part cached and some part
> not. It would read `len` bytes instead of just the uncached portion,
> which could read past `BIOS_MAX_SCAN_LEN`.
> 
> Also add more checked arithmetic to catch potential overflows.
> `read_bios_image_at_offset` is called with a length from the VBIOS
> header, so we should be more defensive here.
> 
> Fixes: 6fda04e7f0cd ("gpu: nova-core: vbios: Add base support for VBIOS construction and iteration")
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/vbios.rs | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index ebda28e596c5..6de7e58e0da0 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -132,17 +132,14 @@ fn read_more(&mut self, len: usize) -> Result {
>  
>      /// Read bytes at a specific offset, filling any gap.
>      fn read_more_at_offset(&mut self, offset: usize, len: usize) -> Result {
> -        if offset > BIOS_MAX_SCAN_LEN {
> +        let end = offset.checked_add(len).ok_or(EINVAL)?;
> +
> +        if end > BIOS_MAX_SCAN_LEN {
>              dev_err!(self.dev, "Error: exceeded BIOS scan limit.\n");
>              return Err(EINVAL);
>          }
>  
> -        // If `offset` is beyond current data size, fill the gap first.
> -        let current_len = self.data.len();
> -        let gap_bytes = offset.saturating_sub(current_len);
> -
> -        // Now read the requested bytes at the offset.
> -        self.read_more(gap_bytes + len)
> +        self.read_more(end.saturating_sub(self.data.len()))
>      }
>  
>      /// Read a BIOS image at a specific offset and create a [`BiosImage`] from it.
> @@ -155,8 +152,9 @@ fn read_bios_image_at_offset(
>          len: usize,
>          context: &str,
>      ) -> Result<BiosImage> {
> +        let end = offset.checked_add(len).ok_or(EINVAL)?;
>          let data_len = self.data.len();
> -        if offset + len > data_len {
> +        if end > data_len {
>              self.read_more_at_offset(offset, len).inspect_err(|e| {
>                  dev_err!(
>                      self.dev,
> @@ -167,7 +165,7 @@ fn read_bios_image_at_offset(
>              })?;
>          }
>  
> -        BiosImage::new(self.dev, &self.data[offset..offset + len]).inspect_err(|err| {
> +        BiosImage::new(self.dev, &self.data[offset..end]).inspect_err(|err| {
>              dev_err!(
>                  self.dev,
>                  "Failed to {} at offset {:#x}: {:?}\n",
> @@ -189,7 +187,7 @@ fn next(&mut self) -> Option<Self::Item> {
>              return None;
>          }
>  
> -        if self.current_offset > BIOS_MAX_SCAN_LEN {
> +        if self.current_offset >= BIOS_MAX_SCAN_LEN {
>              dev_err!(self.dev, "Error: exceeded BIOS scan limit, stopping scan\n");
>              return None;
>          }
Very nice!

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

thanks,

--
Joel Fernandes

Re: [PATCH 2/5] gpu: nova-core: vbios: limit `BitToken` entry reads

[Joel Fernandes]

On 4/10/2026 4:38 AM, Eliot Courtney wrote:
> If `header.token_size` is smaller than `BitToken`, then we currently can
> read past the end of `image.base.data`. Check that the token size is at
> least as big as `BitToken`.
> 
> 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 | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index 6de7e58e0da0..de856000de23 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -423,31 +423,31 @@ impl BitToken {
>      /// Find a BIT token entry by BIT ID in a PciAtBiosImage
>      fn from_id(image: &PciAtBiosImage, token_id: u8) -> Result<Self> {
>          let header = &image.bit_header;
> +        let entry_size = usize::from(header.token_size);
> +
> +        if entry_size < size_of::<BitToken>() {
> +            return Err(EINVAL);
> +        }
>  
>          // Offset to the first token entry
>          let tokens_start = image.bit_offset + usize::from(header.header_size);
>  
>          for i in 0..usize::from(header.token_entries) {
> -            let entry_offset = tokens_start + (i * usize::from(header.token_size));
> -
> -            // Make sure we don't go out of bounds
> -            if entry_offset + usize::from(header.token_size) > image.base.data.len() {
> -                return Err(EINVAL);
> -            }
> +            let entry_offset = tokens_start + (i * entry_size);
> +            let entry = image
> +                .base
> +                .data
> +                .get(entry_offset..)
> +                .and_then(|data| data.get(..entry_size))
> +                .ok_or(EINVAL)?;
>  
>              // Check if this token has the requested ID
> -            if image.base.data[entry_offset] == token_id {
> +            if entry[0] == token_id {
>                  return Ok(BitToken {
> -                    id: image.base.data[entry_offset],
> -                    data_version: image.base.data[entry_offset + 1],
> -                    data_size: u16::from_le_bytes([
> -                        image.base.data[entry_offset + 2],
> -                        image.base.data[entry_offset + 3],
> -                    ]),
> -                    data_offset: u16::from_le_bytes([
> -                        image.base.data[entry_offset + 4],
> -                        image.base.data[entry_offset + 5],
> -                    ]),
> +                    id: entry[0],
> +                    data_version: entry[1],
> +                    data_size: u16::from_le_bytes([entry[2], entry[3]]),
> +                    data_offset: u16::from_le_bytes([entry[4], entry[5]]),
>                  });
>              }
>          }
> 

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

thanks,

--
Joel Fernandes

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

[Joel Fernandes]

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

Re: [PATCH 4/5] gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header`

[Joel Fernandes]

On 4/10/2026 4:38 AM, Eliot Courtney wrote:
> Use checked access in `FwSecBiosImage::header` for getting the header
> version since the value is firmware derived.
> 
> Fixes: 47c4846e4319 ("gpu: nova-core: vbios: Add support for FWSEC ucode extraction")
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/vbios.rs | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index 2b0dc1a9125d..3bd3ac3a69f2 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -995,14 +995,16 @@ pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
>          // Get the falcon ucode offset that was found in setup_falcon_data.
>          let falcon_ucode_offset = self.falcon_ucode_offset;
>  
> +        let data = self.base.data.get(falcon_ucode_offset..).ok_or(EINVAL)?;
> +
>          // Read the first 4 bytes to get the version.
> -        let hdr_bytes: [u8; 4] = self.base.data[falcon_ucode_offset..falcon_ucode_offset + 4]
> +        let hdr_bytes: [u8; 4] = data
> +            .get(..4)
> +            .ok_or(EINVAL)?
>              .try_into()
>              .map_err(|_| EINVAL)?;
>          let hdr = u32::from_le_bytes(hdr_bytes);
>          let ver = (hdr & 0xff00) >> 8;
> -
> -        let data = self.base.data.get(falcon_ucode_offset..).ok_or(EINVAL)?;
>          match ver {
>              2 => {
>                  let v2 = FalconUCodeDescV2::from_bytes_copy_prefix(data)
> 

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

thanks,

--
Joel Fernandes

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

[Joel Fernandes]

Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com> , one comment below

On 4/10/2026 4:38 AM, 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")
> 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 3bd3ac3a69f2..b509cd8407a5 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -1027,16 +1027,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))

It might be worth adding something like:

data.get_slice(start, size) -> Result

in R4L longer term if the data.get(start..).and_then(|data|
data.get(..size)) pattern is common. It seems to be so in this series.

Claude review: gpu: nova-core: vbios: harden various array accesses

[Claude Code Review Bot]

Overall Series Review

Subject: gpu: nova-core: vbios: harden various array accesses
Author: Eliot Courtney <ecourtney@nvidia.com>
Patches: 11
Reviewed: 2026-04-12T10:18:45.552282

---

This is a well-motivated 5-patch series that hardens VBIOS parsing in the nova-core Rust driver against malformed or malicious firmware data. The changes systematically replace unchecked arithmetic (`offset + len`, direct subtraction) and unchecked array indexing with their checked/fallible equivalents (`checked_add`, `checked_sub`, `.get()`). Since all the hardened values originate from firmware (VBIOS headers, token entries, falcon descriptors), these are legitimate trust boundaries where defensive programming is warranted.

The series is clean, well-structured, and each patch is focused on a specific function or code path. The diffs are small and easy to verify. Overall this looks good and ready to merge with minor comments below.

**Verdict: Series looks good. A few minor observations but nothing blocking.**

---

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN`

[Claude Code Review Bot]

Patch Review

This patch fixes three issues:

1. **`read_more_at_offset` bounds check**: The old code checked `offset > BIOS_MAX_SCAN_LEN` but didn't account for `len`, so `offset + len` could exceed the limit. The fix computes `end = offset.checked_add(len)` and checks `end > BIOS_MAX_SCAN_LEN`.

2. **`read_more_at_offset` over-read**: The old logic computed `gap_bytes + len` which would read `len` additional bytes even when some were already cached. The new `end.saturating_sub(self.data.len())` correctly reads only the uncached portion. This is a genuine bug fix.

3. **`read_bios_image_at_offset` overflow**: Similarly replaces `offset + len` with `checked_add`.

4. **Iterator boundary**: Changes `>` to `>=` for `current_offset >= BIOS_MAX_SCAN_LEN`.

All four changes are correct. One observation:

```rust
+        let end = offset.checked_add(len).ok_or(EINVAL)?;
+
+        if end > BIOS_MAX_SCAN_LEN {
```

The `read_more` function requires `len` to be a multiple of 4. After the fix, `read_more_at_offset` calls `self.read_more(end.saturating_sub(self.data.len()))` — if `end` and `self.data.len()` don't have matching alignment, this could pass a non-multiple-of-4 to `read_more`, which would then return `EINVAL`. This was also possible with the old code (it depended on the caller), so it's not a regression — just worth noting that callers must ensure alignment-compatible `offset` and `len` values.

**Reviewed-by assessment: Good.**

---

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: vbios: limit `BitToken` entry reads

[Claude Code Review Bot]

Patch Review

This patch adds a check that `header.token_size >= size_of::<BitToken>()` and replaces direct array indexing with `.get()` + `.ok_or(EINVAL)?`.

```rust
+        if entry_size < size_of::<BitToken>() {
+            return Err(EINVAL);
```

This is a correct and important check. If the firmware reports a `token_size` smaller than the `BitToken` struct (6 bytes), the old code would index out of bounds within each entry.

The refactored loop body:

```rust
+            let entry = image
+                .base
+                .data
+                .get(entry_offset..)
+                .and_then(|data| data.get(..entry_size))
+                .ok_or(EINVAL)?;
```

This is cleaner than the old bounds check + direct indexing. The subsequent access to `entry[0]` through `entry[5]` is safe because `entry_size >= size_of::<BitToken>()` (which is 6) was verified up front.

Minor: No error message or `dev_err!` is emitted when `entry_size < size_of::<BitToken>()`. The existing code also didn't emit a message for out-of-bounds, so this is consistent. A `dev_err!` might be helpful for debugging bad firmware in the field, but it's not mandatory.

**Reviewed-by assessment: Good.**

---

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: vbios: use checked accesses in `setup_falcon_data`

[Claude Code Review Bot]

Patch Review

Two changes here:

1. **PMU lookup table data slicing**: The `pmu_in_first_fwsec` branch keeps `&first_fwsec.base.data[offset..]` as unchecked (which is fine since `offset < first_fwsec.base.data.len()` is guaranteed by the `if` above), while the `else` branch is changed to use `.get(offset..).ok_or(EINVAL)?`.

```rust
+        let pmu_lookup_data = if pmu_in_first_fwsec {
+            &first_fwsec.base.data[offset..]
         } else {
-            self.pmu_lookup_table = Some(PmuLookupTable::new(
-                &self.base.dev,
-                &self.base.data[offset..],
-            )?);
-        }
+            self.base.data.get(offset..).ok_or(EINVAL)?
+        };
```

This is a good refactor — it also deduplicates the `PmuLookupTable::new` call. The asymmetry between the two branches (checked vs unchecked) is acceptable since the `pmu_in_first_fwsec` branch has the earlier `offset < first_fwsec.base.data.len()` guard. However, for consistency, it might be worth using `.get(offset..)` on both branches — but this is stylistic, not a correctness issue.

2. **`ucode_offset` subtraction**: The old code did `ucode_offset -= pci_at_image.base.data.len()` which could underflow (wrapping in release, panicking in debug). The fix uses `checked_sub`:

```rust
+                let mut ucode_offset = usize::from_safe_cast(entry.data)
+                    .checked_sub(pci_at_image.base.data.len())
+                    .ok_or(EINVAL)?;
```

This is correct. Note the same unchecked subtraction pattern exists 6 lines earlier in the function (`offset -= pci_at_image.base.data.len()` at the top of `setup_falcon_data`, line ~926 of the original). That subtraction is **not** fixed by this patch. This could be a follow-up, though the call context may provide guarantees — worth mentioning to the author.

**Reviewed-by assessment: Good, but the unchecked `offset -= pci_at_image.base.data.len()` earlier in the same function (line ~926 in the original) appears to have the same vulnerability and should probably also use `checked_sub`.**

---

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header`

[Claude Code Review Bot]

Patch Review

This patch moves the `data.get(falcon_ucode_offset..)` call to happen first, then uses `data.get(..4)` instead of direct slicing to read the version header:

```rust
+        let data = self.base.data.get(falcon_ucode_offset..).ok_or(EINVAL)?;
+
         // Read the first 4 bytes to get the version.
-        let hdr_bytes: [u8; 4] = self.base.data[falcon_ucode_offset..falcon_ucode_offset + 4]
+        let hdr_bytes: [u8; 4] = data
+            .get(..4)
+            .ok_or(EINVAL)?
             .try_into()
             .map_err(|_| EINVAL)?;
```

Looking at the original code, the `.get(falcon_ucode_offset..)` call already existed but was placed *after* the unchecked `self.base.data[falcon_ucode_offset..falcon_ucode_offset + 4]` — so the old code would panic on an out-of-bounds `falcon_ucode_offset` before ever reaching the checked access. This patch correctly reorders the operations and makes both accesses checked.

Note: the `.try_into().map_err(|_| EINVAL)?` after `.get(..4).ok_or(EINVAL)?` is now technically redundant since `.get(..4)` on a slice guarantees exactly 4 bytes (or `None`). But it's harmless and the compiler will optimize it away, so no issue.

**Reviewed-by assessment: Good.**

---

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: vbios: use checked ops and accesses in `FwSecBiosImage::ucode`

[Claude Code Review Bot]

Patch Review

This patch hardens the `ucode()` function:

1. **`falcon_ucode_offset` bounds check**: Uses `.get(self.falcon_ucode_offset..)` instead of computing `ucode_data_offset = falcon_ucode_offset + desc.size()` which could overflow.

2. **Size computation**: The old `desc.imem_load_size() + desc.dmem_load_size()` (both `u32`) could overflow. The fix uses `checked_add`:

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

3. **Data slicing**: Instead of computing `ucode_data_offset..ucode_data_offset + size` (two potential overflows), the fix chains two `.get()` calls:

```rust
+        data.get(desc.size()..)
+            .and_then(|data| data.get(..size))
             .ok_or(ERANGE)
```

This avoids all intermediate arithmetic overflow and is correct.

The change from `EINVAL` in other patches to `ERANGE` here is consistent with the original code for this function — it already used `ERANGE` for this error case.

**Reviewed-by assessment: Good.**

---

**Summary**: All 5 patches are correct and improve robustness against malformed firmware data. The only item worth raising with the author is the unchecked `offset -= pci_at_image.base.data.len()` at the top of `setup_falcon_data` (around line 926 in the original), which has the same subtraction-underflow pattern that patch 3 fixes for `ucode_offset` but was not addressed.

---
Generated by Claude Code Patch Reviewer