* [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* 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
* [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* 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
* [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* 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
* [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* 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
* [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 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