| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20260131111619.1360414-1-zijing.zhang@ry.rs>
Date: Sat, 31 Jan 2026 11:16:19 +0000
From: Zijing Zhang <zijing.zhang@...rs>
To: dakr@...nel.org,
acourbot@...dia.com
Cc: aliceryhl@...gle.com,
airlied@...il.com,
simona@...ll.ch,
nouveau@...ts.freedesktop.org,
dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org,
Zijing Zhang <zijing.zhang@...rs>
Subject: [PATCH] gpu: nova-core: vbios: harden falcon pointer parsing
Use checked arithmetic and slice get() to avoid overflow/underflow and out-of-bounds access when parsing Falcon data pointers and FWSEC payloads.
This also clarifies the error message for pointers that still fall within the PciAt image.
Signed-off-by: Zijing Zhang <zijing.zhang@...rs>
---
drivers/gpu/nova-core/vbios.rs | 95 ++++++++++++++++++++++++----------
1 file changed, 69 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 3e3fa5b72524..45bbc8894281 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -785,22 +785,31 @@ fn get_bit_token(&self, token_id: u8) -> Result<BitToken> {
fn falcon_data_ptr(&self) -> Result<u32> {
let token = self.get_bit_token(BIT_TOKEN_ID_FALCON_DATA)?;
- // Make sure we don't go out of bounds
- if usize::from(token.data_offset) + 4 > self.base.data.len() {
- return Err(EINVAL);
- }
-
- // read the 4 bytes at the offset specified in the token
let offset = usize::from(token.data_offset);
- let bytes: [u8; 4] = self.base.data[offset..offset + 4].try_into().map_err(|_| {
- dev_err!(self.base.dev, "Failed to convert data slice to array\n");
- EINVAL
- })?;
+ let end = offset.checked_add(4).ok_or(EINVAL)?;
+
+ // Read the 4 bytes at the offset specified in the token.
+ let bytes: [u8; 4] = self
+ .base
+ .data
+ .get(offset..end)
+ .ok_or(EINVAL)?
+ .try_into()
+ .map_err(|_| {
+ dev_err!(self.base.dev, "Failed to convert data slice to array\n");
+ EINVAL
+ })?;
let data_ptr = u32::from_le_bytes(bytes);
- if (usize::from_safe_cast(data_ptr)) < self.base.data.len() {
- dev_err!(self.base.dev, "Falcon data pointer out of bounds\n");
+ // The BIT Falcon data pointer is expected to point outside the PciAt image (into a later
+ // image in the ROM chain). Reject pointers that still fall within this PciAt image, since
+ // downstream code will subtract `self.base.data.len()` from this offset.
+ if usize::from_safe_cast(data_ptr) < self.base.data.len() {
+ dev_err!(
+ self.base.dev,
+ "Falcon data pointer points inside PciAt image\n"
+ );
return Err(EINVAL);
}
@@ -921,14 +930,17 @@ fn setup_falcon_data(
pci_at_image: &PciAtBiosImage,
first_fwsec: &FwSecBiosBuilder,
) -> Result {
- let mut offset = usize::from_safe_cast(pci_at_image.falcon_data_ptr()?);
+ let data_ptr = pci_at_image.falcon_data_ptr()?;
+ let mut offset = usize::from_safe_cast(data_ptr);
let mut pmu_in_first_fwsec = false;
// The falcon data pointer assumes that the PciAt and FWSEC images
// are contiguous in memory. However, testing shows the EFI image sits in
// between them. So calculate the offset from the end of the PciAt image
// rather than the start of it. Compensate.
- offset -= pci_at_image.base.data.len();
+ offset = offset
+ .checked_sub(pci_at_image.base.data.len())
+ .ok_or(EINVAL)?;
// The offset is now from the start of the first Fwsec image, however
// the offset points to a location in the second Fwsec image. Since
@@ -938,7 +950,9 @@ fn setup_falcon_data(
if offset < first_fwsec.base.data.len() {
pmu_in_first_fwsec = true;
} else {
- offset -= first_fwsec.base.data.len();
+ offset = offset
+ .checked_sub(first_fwsec.base.data.len())
+ .ok_or(EINVAL)?;
}
self.falcon_data_offset = Some(offset);
@@ -946,12 +960,16 @@ fn setup_falcon_data(
if pmu_in_first_fwsec {
self.pmu_lookup_table = Some(PmuLookupTable::new(
&self.base.dev,
- &first_fwsec.base.data[offset..],
+ first_fwsec.base.data.get(offset..).ok_or(EINVAL)?,
)?);
} else {
+ if offset >= self.base.data.len() {
+ dev_err!(self.base.dev, "Falcon data pointer out of bounds\n");
+ return Err(EINVAL);
+ }
self.pmu_lookup_table = Some(PmuLookupTable::new(
&self.base.dev,
- &self.base.data[offset..],
+ self.base.data.get(offset..).ok_or(EINVAL)?,
)?);
}
@@ -963,12 +981,20 @@ fn setup_falcon_data(
{
Ok(entry) => {
let mut ucode_offset = usize::from_safe_cast(entry.data);
- ucode_offset -= pci_at_image.base.data.len();
+ ucode_offset = ucode_offset
+ .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);
}
- ucode_offset -= first_fwsec.base.data.len();
+ ucode_offset = ucode_offset
+ .checked_sub(first_fwsec.base.data.len())
+ .ok_or(EINVAL)?;
+ if ucode_offset >= self.base.data.len() {
+ dev_err!(self.base.dev, "Falcon Ucode offset out of bounds.\n");
+ return Err(EINVAL);
+ }
self.falcon_ucode_offset = Some(ucode_offset);
}
Err(e) => {
@@ -1007,7 +1033,11 @@ pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
let falcon_ucode_offset = self.falcon_ucode_offset;
// 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] = self
+ .base
+ .data
+ .get(falcon_ucode_offset..falcon_ucode_offset.checked_add(4).ok_or(EINVAL)?)
+ .ok_or(EINVAL)?
.try_into()
.map_err(|_| EINVAL)?;
let hdr = u32::from_le_bytes(hdr_bytes);
@@ -1039,13 +1069,20 @@ 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 ucode_data_offset = falcon_ucode_offset
+ .checked_add(desc.size())
+ .ok_or(ERANGE)?;
+ let size_u32 = desc
+ .imem_load_size()
+ .checked_add(desc.dmem_load_size())
+ .ok_or(ERANGE)?;
+ let size = usize::from_safe_cast(size_u32);
+ let ucode_data_end = ucode_data_offset.checked_add(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)
+ .get(ucode_data_offset..ucode_data_end)
.ok_or(ERANGE)
.inspect_err(|_| {
dev_err!(
@@ -1062,12 +1099,18 @@ pub(crate) fn sigs(&self, desc: &FalconUCodeDesc) -> Result<&[Bcrt30Rsa3kSignatu
FalconUCodeDesc::V3(_v3) => core::mem::size_of::<FalconUCodeDescV3>(),
};
// The signatures data follows the descriptor.
- let sigs_data_offset = self.falcon_ucode_offset + hdr_size;
+ let sigs_data_offset = self
+ .falcon_ucode_offset
+ .checked_add(hdr_size)
+ .ok_or(ERANGE)?;
let sigs_count = usize::from(desc.signature_count());
- let sigs_size = sigs_count * core::mem::size_of::<Bcrt30Rsa3kSignature>();
+ let sigs_size = sigs_count
+ .checked_mul(core::mem::size_of::<Bcrt30Rsa3kSignature>())
+ .ok_or(ERANGE)?;
// Make sure the data is within bounds.
- if sigs_data_offset + sigs_size > self.base.data.len() {
+ let end = sigs_data_offset.checked_add(sigs_size).ok_or(ERANGE)?;
+ if end > self.base.data.len() {
dev_err!(
self.base.dev,
"fwsec signatures data not contained within BIOS bounds\n"
--
2.52.0
Powered by blists - more mailing lists