[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20260124231830.3088323-2-joelagnelf@nvidia.com>
Date: Sat, 24 Jan 2026 18:18:26 -0500
From: Joel Fernandes <joelagnelf@...dia.com>
To: linux-kernel@...r.kernel.org,
Danilo Krummrich <dakr@...nel.org>,
Alexandre Courbot <acourbot@...dia.com>,
Alice Ryhl <aliceryhl@...gle.com>,
David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>
Cc: John Hubbard <jhubbard@...dia.com>,
Alistair Popple <apopple@...dia.com>,
Timur Tabi <ttabi@...dia.com>,
Edwin Peer <epeer@...dia.com>,
Zhi Wang <zhiw@...dia.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>,
Boqun Feng <boqun.feng@...il.com>,
Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <lossin@...nel.org>,
Andreas Hindborg <a.hindborg@...nel.org>,
Trevor Gross <tmgross@...ch.edu>,
nouveau@...ts.freedesktop.org,
dri-devel@...ts.freedesktop.org,
rust-for-linux@...r.kernel.org,
Joel Fernandes <joelagnelf@...dia.com>
Subject: [PATCH v1 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing
Use checked_add() and checked_mul() when computing offsets from
firmware-provided values in new_fwsec().
Without checked arithmetic, corrupt firmware could cause integer overflow. The
danger is not just wrapping to a huge value, but potentially wrapping to a
small plausible offset that passes validation yet accesses entirely wrong data,
causing silent corruption or security issues.
Signed-off-by: Joel Fernandes <joelagnelf@...dia.com>
---
drivers/gpu/nova-core/firmware/fwsec.rs | 60 ++++++++++++++-----------
1 file changed, 35 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index a8ec08a500ac..1a91bbbce3d5 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -46,10 +46,7 @@
Signed,
Unsigned, //
},
- num::{
- FromSafeCast,
- IntoSafeCast, //
- },
+ num::FromSafeCast,
vbios::Vbios,
};
@@ -267,7 +264,12 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re
let ucode = bios.fwsec_image().ucode(&desc)?;
let mut dma_object = DmaObject::from_data(dev, ucode)?;
- let hdr_offset = usize::from_safe_cast(desc.imem_load_size() + desc.interface_offset());
+ // Compute hdr_offset = imem_load_size + interface_offset.
+ let hdr_offset = desc
+ .imem_load_size()
+ .checked_add(desc.interface_offset())
+ .map(usize::from_safe_cast)
+ .ok_or(EINVAL)?;
// SAFETY: we have exclusive access to `dma_object`.
let hdr: &FalconAppifHdrV1 = unsafe { transmute(&dma_object, hdr_offset) }?;
@@ -277,26 +279,28 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re
// Find the DMEM mapper section in the firmware.
for i in 0..usize::from(hdr.entry_count) {
+ // Compute entry_offset = hdr_offset + header_size + i * entry_size.
+ let entry_offset = hdr_offset
+ .checked_add(usize::from(hdr.header_size))
+ .and_then(|o| o.checked_add(i.checked_mul(usize::from(hdr.entry_size))?))
+ .ok_or(EINVAL)?;
// SAFETY: we have exclusive access to `dma_object`.
- let app: &FalconAppifV1 = unsafe {
- transmute(
- &dma_object,
- hdr_offset + usize::from(hdr.header_size) + i * usize::from(hdr.entry_size),
- )
- }?;
+ let app: &FalconAppifV1 = unsafe { transmute(&dma_object, entry_offset) }?;
if app.id != NVFW_FALCON_APPIF_ID_DMEMMAPPER {
continue;
}
let dmem_base = app.dmem_base;
+ // Compute dmem_mapper_offset = imem_load_size + dmem_base.
+ let dmem_mapper_offset = desc
+ .imem_load_size()
+ .checked_add(dmem_base)
+ .map(usize::from_safe_cast)
+ .ok_or(EINVAL)?;
// SAFETY: we have exclusive access to `dma_object`.
- let dmem_mapper: &mut FalconAppifDmemmapperV3 = unsafe {
- transmute_mut(
- &mut dma_object,
- (desc.imem_load_size() + dmem_base).into_safe_cast(),
- )
- }?;
+ let dmem_mapper: &mut FalconAppifDmemmapperV3 =
+ unsafe { transmute_mut(&mut dma_object, dmem_mapper_offset) }?;
dmem_mapper.init_cmd = match cmd {
FwsecCommand::Frts { .. } => NVFW_FALCON_APPIF_DMEMMAPPER_CMD_FRTS,
@@ -304,13 +308,15 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re
};
let cmd_in_buffer_offset = dmem_mapper.cmd_in_buffer_offset;
+ // Compute frts_cmd_offset = imem_load_size + cmd_in_buffer_offset.
+ let frts_cmd_offset = desc
+ .imem_load_size()
+ .checked_add(cmd_in_buffer_offset)
+ .map(usize::from_safe_cast)
+ .ok_or(EINVAL)?;
// SAFETY: we have exclusive access to `dma_object`.
- let frts_cmd: &mut FrtsCmd = unsafe {
- transmute_mut(
- &mut dma_object,
- (desc.imem_load_size() + cmd_in_buffer_offset).into_safe_cast(),
- )
- }?;
+ let frts_cmd: &mut FrtsCmd =
+ unsafe { transmute_mut(&mut dma_object, frts_cmd_offset) }?;
frts_cmd.read_vbios = ReadVbios {
ver: 1,
@@ -356,8 +362,12 @@ pub(crate) fn new(
// Patch signature if needed.
let desc = bios.fwsec_image().header()?;
let ucode_signed = if desc.signature_count() != 0 {
- let sig_base_img =
- usize::from_safe_cast(desc.imem_load_size() + desc.pkc_data_offset());
+ // Compute sig_base_img = desc.imem_load_size + desc.pkc_data_offset.
+ let sig_base_img = desc
+ .imem_load_size()
+ .checked_add(desc.pkc_data_offset())
+ .map(usize::from_safe_cast)
+ .ok_or(EINVAL)?;
let desc_sig_versions = u32::from(desc.signature_versions());
let reg_fuse_version =
falcon.signature_reg_fuse_version(bar, desc.engine_id_mask(), desc.ucode_id())?;
--
2.34.1
Powered by blists - more mailing lists