public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] gpu: nova-core: vbios: harden various array accesses
@ 2026-04-10  8:38 Eliot Courtney
  2026-04-10  8:38 ` [PATCH 1/5] gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN` Eliot Courtney
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Eliot Courtney @ 2026-04-10  8:38 UTC (permalink / raw)
  To: Danilo Krummrich, Alice Ryhl, Alexandre Courbot, David Airlie,
	Simona Vetter, Joel Fernandes
  Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
	dri-devel, linux-kernel, 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>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/5] gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN`
  2026-04-10  8:38 [PATCH 0/5] gpu: nova-core: vbios: harden various array accesses Eliot Courtney
@ 2026-04-10  8:38 ` Eliot Courtney
  2026-04-10 14:08   ` Joel Fernandes
  2026-04-10  8:38 ` [PATCH 2/5] gpu: nova-core: vbios: limit `BitToken` entry reads Eliot Courtney
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Eliot Courtney @ 2026-04-10  8:38 UTC (permalink / raw)
  To: Danilo Krummrich, Alice Ryhl, Alexandre Courbot, David Airlie,
	Simona Vetter, Joel Fernandes
  Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
	dri-devel, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/5] gpu: nova-core: vbios: limit `BitToken` entry reads
  2026-04-10  8:38 [PATCH 0/5] gpu: nova-core: vbios: harden various array accesses Eliot Courtney
  2026-04-10  8:38 ` [PATCH 1/5] gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN` Eliot Courtney
@ 2026-04-10  8:38 ` Eliot Courtney
  2026-04-10 14:30   ` Joel Fernandes
  2026-04-10  8:38 ` [PATCH 3/5] gpu: nova-core: vbios: use checked accesses in `setup_falcon_data` Eliot Courtney
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Eliot Courtney @ 2026-04-10  8:38 UTC (permalink / raw)
  To: Danilo Krummrich, Alice Ryhl, Alexandre Courbot, David Airlie,
	Simona Vetter, Joel Fernandes
  Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
	dri-devel, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/5] gpu: nova-core: vbios: use checked accesses in `setup_falcon_data`
  2026-04-10  8:38 [PATCH 0/5] gpu: nova-core: vbios: harden various array accesses Eliot Courtney
  2026-04-10  8:38 ` [PATCH 1/5] gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN` Eliot Courtney
  2026-04-10  8:38 ` [PATCH 2/5] gpu: nova-core: vbios: limit `BitToken` entry reads Eliot Courtney
@ 2026-04-10  8:38 ` Eliot Courtney
  2026-04-10 14:53   ` Joel Fernandes
  2026-04-10  8:38 ` [PATCH 4/5] gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header` Eliot Courtney
  2026-04-10  8:38 ` [PATCH 5/5] gpu: nova-core: vbios: use checked ops and accesses in `FwSecBiosImage::ucode` Eliot Courtney
  4 siblings, 1 reply; 11+ messages in thread
From: Eliot Courtney @ 2026-04-10  8:38 UTC (permalink / raw)
  To: Danilo Krummrich, Alice Ryhl, Alexandre Courbot, David Airlie,
	Simona Vetter, Joel Fernandes
  Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
	dri-devel, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/5] gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header`
  2026-04-10  8:38 [PATCH 0/5] gpu: nova-core: vbios: harden various array accesses Eliot Courtney
                   ` (2 preceding siblings ...)
  2026-04-10  8:38 ` [PATCH 3/5] gpu: nova-core: vbios: use checked accesses in `setup_falcon_data` Eliot Courtney
@ 2026-04-10  8:38 ` Eliot Courtney
  2026-04-10 15:00   ` Joel Fernandes
  2026-04-10  8:38 ` [PATCH 5/5] gpu: nova-core: vbios: use checked ops and accesses in `FwSecBiosImage::ucode` Eliot Courtney
  4 siblings, 1 reply; 11+ messages in thread
From: Eliot Courtney @ 2026-04-10  8:38 UTC (permalink / raw)
  To: Danilo Krummrich, Alice Ryhl, Alexandre Courbot, David Airlie,
	Simona Vetter, Joel Fernandes
  Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
	dri-devel, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/5] gpu: nova-core: vbios: use checked ops and accesses in `FwSecBiosImage::ucode`
  2026-04-10  8:38 [PATCH 0/5] gpu: nova-core: vbios: harden various array accesses Eliot Courtney
                   ` (3 preceding siblings ...)
  2026-04-10  8:38 ` [PATCH 4/5] gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header` Eliot Courtney
@ 2026-04-10  8:38 ` Eliot Courtney
  2026-04-10 15:05   ` Joel Fernandes
  4 siblings, 1 reply; 11+ messages in thread
From: Eliot Courtney @ 2026-04-10  8:38 UTC (permalink / raw)
  To: Danilo Krummrich, Alice Ryhl, Alexandre Courbot, David Airlie,
	Simona Vetter, Joel Fernandes
  Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
	dri-devel, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/5] gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN`
  2026-04-10  8:38 ` [PATCH 1/5] gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN` Eliot Courtney
@ 2026-04-10 14:08   ` Joel Fernandes
  0 siblings, 0 replies; 11+ messages in thread
From: Joel Fernandes @ 2026-04-10 14:08 UTC (permalink / raw)
  To: Eliot Courtney, Danilo Krummrich, Alice Ryhl, Alexandre Courbot,
	David Airlie, Simona Vetter
  Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
	dri-devel, linux-kernel



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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/5] gpu: nova-core: vbios: limit `BitToken` entry reads
  2026-04-10  8:38 ` [PATCH 2/5] gpu: nova-core: vbios: limit `BitToken` entry reads Eliot Courtney
@ 2026-04-10 14:30   ` Joel Fernandes
  0 siblings, 0 replies; 11+ messages in thread
From: Joel Fernandes @ 2026-04-10 14:30 UTC (permalink / raw)
  To: Eliot Courtney, Danilo Krummrich, Alice Ryhl, Alexandre Courbot,
	David Airlie, Simona Vetter
  Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
	dri-devel, linux-kernel



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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/5] gpu: nova-core: vbios: use checked accesses in `setup_falcon_data`
  2026-04-10  8:38 ` [PATCH 3/5] gpu: nova-core: vbios: use checked accesses in `setup_falcon_data` Eliot Courtney
@ 2026-04-10 14:53   ` Joel Fernandes
  0 siblings, 0 replies; 11+ messages in thread
From: Joel Fernandes @ 2026-04-10 14:53 UTC (permalink / raw)
  To: Eliot Courtney, Danilo Krummrich, Alice Ryhl, Alexandre Courbot,
	David Airlie, Simona Vetter
  Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
	dri-devel, linux-kernel

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/5] gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header`
  2026-04-10  8:38 ` [PATCH 4/5] gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header` Eliot Courtney
@ 2026-04-10 15:00   ` Joel Fernandes
  0 siblings, 0 replies; 11+ messages in thread
From: Joel Fernandes @ 2026-04-10 15:00 UTC (permalink / raw)
  To: Eliot Courtney, Danilo Krummrich, Alice Ryhl, Alexandre Courbot,
	David Airlie, Simona Vetter
  Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
	dri-devel, linux-kernel



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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 5/5] gpu: nova-core: vbios: use checked ops and accesses in `FwSecBiosImage::ucode`
  2026-04-10  8:38 ` [PATCH 5/5] gpu: nova-core: vbios: use checked ops and accesses in `FwSecBiosImage::ucode` Eliot Courtney
@ 2026-04-10 15:05   ` Joel Fernandes
  0 siblings, 0 replies; 11+ messages in thread
From: Joel Fernandes @ 2026-04-10 15:05 UTC (permalink / raw)
  To: Eliot Courtney, Danilo Krummrich, Alice Ryhl, Alexandre Courbot,
	David Airlie, Simona Vetter
  Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
	dri-devel, linux-kernel

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.


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-04-10 15:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-10  8:38 [PATCH 0/5] gpu: nova-core: vbios: harden various array accesses Eliot Courtney
2026-04-10  8:38 ` [PATCH 1/5] gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN` Eliot Courtney
2026-04-10 14:08   ` Joel Fernandes
2026-04-10  8:38 ` [PATCH 2/5] gpu: nova-core: vbios: limit `BitToken` entry reads Eliot Courtney
2026-04-10 14:30   ` Joel Fernandes
2026-04-10  8:38 ` [PATCH 3/5] gpu: nova-core: vbios: use checked accesses in `setup_falcon_data` Eliot Courtney
2026-04-10 14:53   ` Joel Fernandes
2026-04-10  8:38 ` [PATCH 4/5] gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header` Eliot Courtney
2026-04-10 15:00   ` Joel Fernandes
2026-04-10  8:38 ` [PATCH 5/5] gpu: nova-core: vbios: use checked ops and accesses in `FwSecBiosImage::ucode` Eliot Courtney
2026-04-10 15:05   ` Joel Fernandes

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox