On 1/28/2026 3:08 AM, Alexandre Courbot wrote:
On Wed Jan 28, 2026 at 4:58 PM JST, Alexandre Courbot wrote:
On Tue Jan 27, 2026 at 5:23 AM JST, Joel Fernandes wrote:
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.
Reviewed-by: Zhi Wang <[email protected]>
Signed-off-by: Joel Fernandes <[email protected]>
---
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..71541b1f07d7 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`.
Missing empty line before the SAFETY comment (also in other places).
I will fix when applying, no need to resend.
I also got this clippy warning when building:
warning: unsafe block missing a safety comment
--> ../drivers/gpu/nova-core/firmware/fwsec.rs:303:17
|
303 | unsafe { transmute_mut(&mut dma_object,
dmem_mapper_offset) }?;
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider adding a safety comment on the
preceding line
= help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks
= note: requested on the command line with `-W
clippy::undocumented-unsafe-blocks`
warning: unsafe block missing a safety comment
--> ../drivers/gpu/nova-core/firmware/fwsec.rs:319:17
|
319 | unsafe { transmute_mut(&mut dma_object,
frts_cmd_offset) }?;
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider adding a safety comment on the
preceding line
= help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks
warning: 2 warnings emitted
Since the `unsafe` keyword has moved to a new line, its SAFETY comment needed
to be moved right above it, despite it still being part of the same statement.
I'll fix this as well.
Thanks Alex! Do you mind also dropping those "Compute .." comments that
Danilo mentioned. But come to think of it, I think those comments do
improve any loss of readability due to the checked_* calls.
--
Joel Fernandes
