[PATCH v2 0/5] gpu: nova-core: use checked arithmetic for firmware parsing robustness

[PATCH v2 0/5] gpu: nova-core: use checked arithmetic for firmware parsing robustness

[Joel Fernandes]

Changes from v1 to v2:
- Added Reviewed-by tags from Zhi
- Fixed comment formatting nits raised by Dirk/Zhi

This series adds checked arithmetic throughout nova-core's firmware parsing
code to guard rust code against integer overflow from corrupt firmware.

Without checked arithmetic, firmware could cause integer overflow when
computing offsets. The danger is not just wrapping to a huge value (which may
fail validation in other paths), but potentially wrapping to a small plausible
offset that accesses entirely wrong data, causing silent corruption or security
issues.

This series has been rebased on drm-rust-next. If possible, I would like us to
consider merging for the upcoming merge window to avoid future conflicts.
Tested probing with GPU name printed in dmesg on my GA102 (Ampere).

The git tree with all patches can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git (tag: nova-checked-arith-v2-20260126)

Link to v1: https://lore.kernel.org/all/20260124231830.3088323-1-joelagnelf@nvidia.com/

Joel Fernandes (5):
  gpu: nova-core: use checked arithmetic in FWSEC firmware parsing
  gpu: nova-core: use checked arithmetic in Booter signature parsing
  gpu: nova-core: use checked arithmetic in frombytes_at helper
  gpu: nova-core: use checked arithmetic in BinFirmware::data
  gpu: nova-core: use checked arithmetic in RISC-V firmware parsing

 drivers/gpu/nova-core/firmware.rs        |  3 +-
 drivers/gpu/nova-core/firmware/booter.rs | 22 ++++++---
 drivers/gpu/nova-core/firmware/fwsec.rs  | 60 ++++++++++++++----------
 drivers/gpu/nova-core/firmware/riscv.rs  |  6 ++-
 4 files changed, 57 insertions(+), 34 deletions(-)


base-commit: cea7b66a80412e2a5b74627b89ae25f1d0110a4b
--
2.34.1

[PATCH v2 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing

[Joel Fernandes]

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 <zhiw@nvidia.com>
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.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..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`.
         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 = imem_load_size + 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

Re: [PATCH v2 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing

[Alexandre Courbot]

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 <zhiw@nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.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..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.

Re: [PATCH v2 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing

[Alexandre Courbot]

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 <zhiw@nvidia.com>
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.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..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.

Re: [PATCH v2 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing

[Joel Fernandes]

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 <zhiw@nvidia.com>
>>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.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..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

Re: [PATCH v2 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing

[Danilo Krummrich]

On Mon Jan 26, 2026 at 9:23 PM CET, Joel Fernandes wrote:
> @@ -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.

I do get the idea behind those comments, but are we sure that's really a good
idea? How do we ensure to keep them up to date in case we have to change the
code?

If we really want this, I'd at least chose a common syntax, e.g.

	// CALC: `imem_load_size + interface_offset`

without the variable name the resulting value is assigned to.

But I'd rather prefer to just drop those comments.

> +        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) }?;

Re: [PATCH v2 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing

[Joel Fernandes]

On 1/28/2026 5:53 AM, Danilo Krummrich wrote:
> On Mon Jan 26, 2026 at 9:23 PM CET, Joel Fernandes wrote:
>> @@ -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.
> 
> I do get the idea behind those comments, but are we sure that's really a good
> idea? How do we ensure to keep them up to date in case we have to change the
> code?
> 
> If we really want this, I'd at least chose a common syntax, e.g.
> 
> 	// CALC: `imem_load_size + interface_offset`
> 
> without the variable name the resulting value is assigned to.
> 
> But I'd rather prefer to just drop those comments.
The idea of adding these comments was to improve readability. However, I 
can drop them in the v3, that's fine with me.

Do you want me to wait for additional comments on this series, or should 
I make the update and repost it?  Thanks,

--
Joel Fernandes

Re: [PATCH v2 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing

[Danilo Krummrich]

On Wed Jan 28, 2026 at 4:14 PM CET, Joel Fernandes wrote:
> On 1/28/2026 5:53 AM, Danilo Krummrich wrote:
>> On Mon Jan 26, 2026 at 9:23 PM CET, Joel Fernandes wrote:
>>> @@ -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.
>> 
>> I do get the idea behind those comments, but are we sure that's really a good
>> idea? How do we ensure to keep them up to date in case we have to change the
>> code?
>> 
>> If we really want this, I'd at least chose a common syntax, e.g.
>> 
>> 	// CALC: `imem_load_size + interface_offset`
>> 
>> without the variable name the resulting value is assigned to.
>> 
>> But I'd rather prefer to just drop those comments.
> The idea of adding these comments was to improve readability. However, I 
> can drop them in the v3, that's fine with me.

Yeah, that's why I wrote "I get the idea". :) But as I write above, I'm
concerned about the comments getting outdated or inconsistent over time.

Besides that, it more seems like something your favorite editor should help with
instead.

> Do you want me to wait for additional comments on this series, or should 
> I make the update and repost it?  Thanks,

As mentioned, I tend to think we should just drop them, but I'm happy to hear
some more opinions on this if any.

Re: [PATCH v2 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing

[Alexandre Courbot]

On Thu Jan 29, 2026 at 9:20 AM JST, Danilo Krummrich wrote:
> On Wed Jan 28, 2026 at 4:14 PM CET, Joel Fernandes wrote:
>> On 1/28/2026 5:53 AM, Danilo Krummrich wrote:
>>> On Mon Jan 26, 2026 at 9:23 PM CET, Joel Fernandes wrote:
>>>> @@ -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.
>>> 
>>> I do get the idea behind those comments, but are we sure that's really a good
>>> idea? How do we ensure to keep them up to date in case we have to change the
>>> code?
>>> 
>>> If we really want this, I'd at least chose a common syntax, e.g.
>>> 
>>> 	// CALC: `imem_load_size + interface_offset`
>>> 
>>> without the variable name the resulting value is assigned to.
>>> 
>>> But I'd rather prefer to just drop those comments.
>> The idea of adding these comments was to improve readability. However, I 
>> can drop them in the v3, that's fine with me.
>
> Yeah, that's why I wrote "I get the idea". :) But as I write above, I'm
> concerned about the comments getting outdated or inconsistent over time.
>
> Besides that, it more seems like something your favorite editor should help with
> instead.
>
>> Do you want me to wait for additional comments on this series, or should 
>> I make the update and repost it?  Thanks,
>
> As mentioned, I tend to think we should just drop them, but I'm happy to hear
> some more opinions on this if any.

For safety I would keep something like the 

  // CALC: `imem_load_size + interface_offset`

you suggested. From simple operations yes, the code would be obvious,
but there are also more involved computations where order matters and it
is good to have a reference. These shouldn't change often anyway, and
the `CALC:` header catches the attention of anyone who would update
them, similarly to a `SAFETY:` comment.

If Joel agrees, I will amend the comments accordingly in my staging
branch.

Re: [PATCH v2 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing

[Joel Fernandes]

On 1/28/2026 7:36 PM, Alexandre Courbot wrote:
> On Thu Jan 29, 2026 at 9:20 AM JST, Danilo Krummrich wrote:
>> On Wed Jan 28, 2026 at 4:14 PM CET, Joel Fernandes wrote:
>>> On 1/28/2026 5:53 AM, Danilo Krummrich wrote:
>>>> On Mon Jan 26, 2026 at 9:23 PM CET, Joel Fernandes wrote:
>>>>> @@ -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.
>>>>
>>>> I do get the idea behind those comments, but are we sure that's really a good
>>>> idea? How do we ensure to keep them up to date in case we have to change the
>>>> code?
>>>>
>>>> If we really want this, I'd at least chose a common syntax, e.g.
>>>>
>>>> 	// CALC: `imem_load_size + interface_offset`
>>>>
>>>> without the variable name the resulting value is assigned to.
>>>>
>>>> But I'd rather prefer to just drop those comments.
>>> The idea of adding these comments was to improve readability. However, I 
>>> can drop them in the v3, that's fine with me.
>>
>> Yeah, that's why I wrote "I get the idea". :) But as I write above, I'm
>> concerned about the comments getting outdated or inconsistent over time.
>>
>> Besides that, it more seems like something your favorite editor should help with
>> instead.
>>
>>> Do you want me to wait for additional comments on this series, or should 
>>> I make the update and repost it?  Thanks,
>>
>> As mentioned, I tend to think we should just drop them, but I'm happy to hear
>> some more opinions on this if any.
> 
> For safety I would keep something like the 
> 
>   // CALC: `imem_load_size + interface_offset`
> 
> you suggested. From simple operations yes, the code would be obvious,
> but there are also more involved computations where order matters and it
> is good to have a reference. These shouldn't change often anyway, and
> the `CALC:` header catches the attention of anyone who would update
> them, similarly to a `SAFETY:` comment.
> 
> If Joel agrees, I will amend the comments accordingly in my staging
> branch.

This approach sounds good to me. I am of the opinion, this "pseudocode comment"
should not change as long as the actual code's changes does not cause arithmetic
changes.

Whatever we decide, thanks for fixing it up Alex.

--
Joel Fernandes

Re: [PATCH v2 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing

[John Hubbard]

On 1/28/26 4:20 PM, Danilo Krummrich wrote:
> On Wed Jan 28, 2026 at 4:14 PM CET, Joel Fernandes wrote:
>> On 1/28/2026 5:53 AM, Danilo Krummrich wrote:
>>> On Mon Jan 26, 2026 at 9:23 PM CET, Joel Fernandes wrote:
>>>> @@ -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.
>>>
>>> I do get the idea behind those comments, but are we sure that's really a good
>>> idea? How do we ensure to keep them up to date in case we have to change the
>>> code?
>>>
>>> If we really want this, I'd at least chose a common syntax, e.g.
>>>
>>> 	// CALC: `imem_load_size + interface_offset`
>>>
>>> without the variable name the resulting value is assigned to.
>>>
>>> But I'd rather prefer to just drop those comments.
>> The idea of adding these comments was to improve readability. However, I
>> can drop them in the v3, that's fine with me.
> 
> Yeah, that's why I wrote "I get the idea". :) But as I write above, I'm
> concerned about the comments getting outdated or inconsistent over time.
> 
> Besides that, it more seems like something your favorite editor should help with
> instead.
> 
>> Do you want me to wait for additional comments on this series, or should
>> I make the update and repost it?  Thanks,
> 
> As mentioned, I tend to think we should just drop them, but I'm happy to hear
> some more opinions on this if any.

Yes, please drop the comments. They were just echoing the code for
the most part, so the code itself will be easier to read without
them.

I think this is something that, when you are writing the code and
comments, it's really hard to see. But when you come back a while
later (or you are reviewing someone else's patch), then it is much
easier to spot.

thanks,
-- 
John Hubbard

Re: [PATCH v2 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing

[Alexandre Courbot]

On Thu Jan 29, 2026 at 9:58 AM JST, John Hubbard wrote:
> On 1/28/26 4:20 PM, Danilo Krummrich wrote:
>> On Wed Jan 28, 2026 at 4:14 PM CET, Joel Fernandes wrote:
>>> On 1/28/2026 5:53 AM, Danilo Krummrich wrote:
>>>> On Mon Jan 26, 2026 at 9:23 PM CET, Joel Fernandes wrote:
>>>>> @@ -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.
>>>>
>>>> I do get the idea behind those comments, but are we sure that's really a good
>>>> idea? How do we ensure to keep them up to date in case we have to change the
>>>> code?
>>>>
>>>> If we really want this, I'd at least chose a common syntax, e.g.
>>>>
>>>> 	// CALC: `imem_load_size + interface_offset`
>>>>
>>>> without the variable name the resulting value is assigned to.
>>>>
>>>> But I'd rather prefer to just drop those comments.
>>> The idea of adding these comments was to improve readability. However, I
>>> can drop them in the v3, that's fine with me.
>> 
>> Yeah, that's why I wrote "I get the idea". :) But as I write above, I'm
>> concerned about the comments getting outdated or inconsistent over time.
>> 
>> Besides that, it more seems like something your favorite editor should help with
>> instead.
>> 
>>> Do you want me to wait for additional comments on this series, or should
>>> I make the update and repost it?  Thanks,
>> 
>> As mentioned, I tend to think we should just drop them, but I'm happy to hear
>> some more opinions on this if any.
>
> Yes, please drop the comments. They were just echoing the code for
> the most part, so the code itself will be easier to read without
> them.

I agree that if the operation is a simple `checked_add`, then comments
are not necessarily useful.

However, we also have stuff like 

  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))?))

Where the order of operation matters, and for these I think it is safer
to have a quick confirmation.

Thus for anything non-trivial, I'd like to keep a `// CALC: ` header
describing the intended operation. I also noticed that LLMs check that
the code is in accordance with comments, which provides an additional
layer of checking.

Re: [PATCH v2 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing

[Miguel Ojeda]

On Tue, Feb 3, 2026 at 11:25 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> Thus for anything non-trivial, I'd like to keep a `// CALC: ` header
> describing the intended operation. I also noticed that LLMs check that
> the code is in accordance with comments, which provides an additional
> layer of checking.

Yeah, it is the same reason why documentation as well as other tagged
comments like `// SAFETY:` comments enable to catch mistakes even if
they may be redundant in a certain sense.

I wouldn't mind having those tagged comments after a certain
complexity -- perhaps it could be possible to define a heuristic for a
threshold where such a comment is required, and get Clippy to warn
about it (we are trying to get other tagged comments implemented, so
it is a good opportunity).

I guess a fancy IDE could perhaps render the math expression on hover as well.

Cheers,
Miguel

Re: [PATCH v2 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing

[Joel Fernandes]

> On Feb 4, 2026, at 1:54 PM, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
> 
> On Tue, Feb 3, 2026 at 11:25 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>> 
>> Thus for anything non-trivial, I'd like to keep a `// CALC: ` header
>> describing the intended operation. I also noticed that LLMs check that
>> the code is in accordance with comments, which provides an additional
>> layer of checking.
> 
> Yeah, it is the same reason why documentation as well as other tagged
> comments like `// SAFETY:` comments enable to catch mistakes even if
> they may be redundant in a certain sense.
> 
> I wouldn't mind having those tagged comments after a certain
> complexity -- perhaps it could be possible to define a heuristic for a
> threshold where such a comment is required, and get Clippy to warn
> about it (we are trying to get other tagged comments implemented, so
> it is a good opportunity).
> 
> I guess a fancy IDE could perhaps render the math expression on hover as well.

All of these suggestions sound good to me!

Thanks,

Joel Fernandes


> 
> Cheers,
> Miguel

[PATCH v2 2/5] gpu: nova-core: use checked arithmetic in Booter signature parsing

[Joel Fernandes]

Use checked_add() when computing signature offsets from firmware-
provided values in signatures_iter().

Without checked arithmetic, overflow could wrap to a small plausible
offset that points to entirely wrong data.

Reviewed-by: Zhi Wang <zhiw@nvidia.com>
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 drivers/gpu/nova-core/firmware/booter.rs | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware/booter.rs b/drivers/gpu/nova-core/firmware/booter.rs
index 86556cee8e67..40ac7e66d228 100644
--- a/drivers/gpu/nova-core/firmware/booter.rs
+++ b/drivers/gpu/nova-core/firmware/booter.rs
@@ -119,14 +119,23 @@ fn signatures_iter(&'a self) -> Result<impl Iterator<Item = BooterSignature<'a>>
             Some(sig_size) => {
                 let patch_sig =
                     frombytes_at::<u32>(self.fw, self.hdr.patch_sig_offset.into_safe_cast())?;
-                let signatures_start = usize::from_safe_cast(self.hdr.sig_prod_offset + patch_sig);
+
+                // Compute signatures_start = sig_prod_offset + patch_sig.
+                let signatures_start = self
+                    .hdr
+                    .sig_prod_offset
+                    .checked_add(patch_sig)
+                    .map(usize::from_safe_cast)
+                    .ok_or(EINVAL)?;
+
+                // Compute signatures_end = signatures_start + sig_prod_size.
+                let signatures_end = signatures_start
+                    .checked_add(usize::from_safe_cast(self.hdr.sig_prod_size))
+                    .ok_or(EINVAL)?;
 
                 self.fw
                     // Get signatures range.
-                    .get(
-                        signatures_start
-                            ..signatures_start + usize::from_safe_cast(self.hdr.sig_prod_size),
-                    )
+                    .get(signatures_start..signatures_end)
                     .ok_or(EINVAL)?
                     .chunks_exact(sig_size.into_safe_cast())
             }
-- 
2.34.1

[PATCH v2 3/5] gpu: nova-core: use checked arithmetic in frombytes_at helper

[Joel Fernandes]

Use checked_add() when computing the end offset in the frombytes_at()
helper function. This function is called with firmware-provided offsets.

Reviewed-by: Zhi Wang <zhiw@nvidia.com>
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 drivers/gpu/nova-core/firmware/booter.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/nova-core/firmware/booter.rs b/drivers/gpu/nova-core/firmware/booter.rs
index 40ac7e66d228..bae24c756853 100644
--- a/drivers/gpu/nova-core/firmware/booter.rs
+++ b/drivers/gpu/nova-core/firmware/booter.rs
@@ -43,8 +43,9 @@
 /// Local convenience function to return a copy of `S` by reinterpreting the bytes starting at
 /// `offset` in `slice`.
 fn frombytes_at<S: FromBytes + Sized>(slice: &[u8], offset: usize) -> Result<S> {
+    let end = offset.checked_add(size_of::<S>()).ok_or(EINVAL)?;
     slice
-        .get(offset..offset + size_of::<S>())
+        .get(offset..end)
         .and_then(S::from_bytes_copy)
         .ok_or(EINVAL)
 }
-- 
2.34.1

[PATCH v2 4/5] gpu: nova-core: use checked arithmetic in BinFirmware::data

[Joel Fernandes]

Use checked_add() when computing the firmware data end offset in the
BinFirmware::data() method. The data_offset and data_size fields come
from the BinHdr structure parsed from the firmware file header.

Reviewed-by: Zhi Wang <zhiw@nvidia.com>
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 drivers/gpu/nova-core/firmware.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index 68779540aa28..4f57a270e142 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -394,8 +394,9 @@ fn new(fw: &'a firmware::Firmware) -> Result<Self> {
     fn data(&self) -> Option<&[u8]> {
         let fw_start = usize::from_safe_cast(self.hdr.data_offset);
         let fw_size = usize::from_safe_cast(self.hdr.data_size);
+        let fw_end = fw_start.checked_add(fw_size)?;
 
-        self.fw.get(fw_start..fw_start + fw_size)
+        self.fw.get(fw_start..fw_end)
     }
 }
 
-- 
2.34.1

[PATCH v2 5/5] gpu: nova-core: use checked arithmetic in RISC-V firmware parsing

[Joel Fernandes]

Use checked_add() when computing offsets from firmware-provided values
in the RISC-V firmware parsing code. These values come from the BinHdr
structure parsed from the firmware file header.

Reviewed-by: Zhi Wang <zhiw@nvidia.com>
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 drivers/gpu/nova-core/firmware/riscv.rs | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
index 28dfef63657a..97030bdd9991 100644
--- a/drivers/gpu/nova-core/firmware/riscv.rs
+++ b/drivers/gpu/nova-core/firmware/riscv.rs
@@ -47,10 +47,11 @@ impl RmRiscvUCodeDesc {
     /// Fails if the header pointed at by `bin_fw` is not within the bounds of the firmware image.
     fn new(bin_fw: &BinFirmware<'_>) -> Result<Self> {
         let offset = usize::from_safe_cast(bin_fw.hdr.header_offset);
+        let end = offset.checked_add(size_of::<Self>()).ok_or(EINVAL)?;
 
         bin_fw
             .fw
-            .get(offset..offset + size_of::<Self>())
+            .get(offset..end)
             .and_then(Self::from_bytes_copy)
             .ok_or(EINVAL)
     }
@@ -80,8 +81,9 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, fw: &Firmware) -> Result<
         let ucode = {
             let start = usize::from_safe_cast(bin_fw.hdr.data_offset);
             let len = usize::from_safe_cast(bin_fw.hdr.data_size);
+            let end = start.checked_add(len).ok_or(EINVAL)?;
 
-            DmaObject::from_data(dev, fw.data().get(start..start + len).ok_or(EINVAL)?)?
+            DmaObject::from_data(dev, fw.data().get(start..end).ok_or(EINVAL)?)?
         };
 
         Ok(Self {
-- 
2.34.1

Re: [PATCH v2 0/5] gpu: nova-core: use checked arithmetic for firmware parsing robustness

[Gary Guo]

On Mon Jan 26, 2026 at 8:23 PM GMT, Joel Fernandes wrote:
> Changes from v1 to v2:
> - Added Reviewed-by tags from Zhi
> - Fixed comment formatting nits raised by Dirk/Zhi
>
> This series adds checked arithmetic throughout nova-core's firmware parsing
> code to guard rust code against integer overflow from corrupt firmware.
>
> Without checked arithmetic, firmware could cause integer overflow when
> computing offsets. The danger is not just wrapping to a huge value (which may
> fail validation in other paths), but potentially wrapping to a small plausible
> offset that accesses entirely wrong data, causing silent corruption or security
> issues.
>
> This series has been rebased on drm-rust-next. If possible, I would like us to
> consider merging for the upcoming merge window to avoid future conflicts.
> Tested probing with GPU name printed in dmesg on my GA102 (Ampere).
>
> The git tree with all patches can be found at:
> git://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git (tag: nova-checked-arith-v2-20260126)
>
> Link to v1: https://lore.kernel.org/all/20260124231830.3088323-1-joelagnelf@nvidia.com/
>
> Joel Fernandes (5):
>   gpu: nova-core: use checked arithmetic in FWSEC firmware parsing
>   gpu: nova-core: use checked arithmetic in Booter signature parsing
>   gpu: nova-core: use checked arithmetic in frombytes_at helper
>   gpu: nova-core: use checked arithmetic in BinFirmware::data
>   gpu: nova-core: use checked arithmetic in RISC-V firmware parsing
>
>  drivers/gpu/nova-core/firmware.rs        |  3 +-
>  drivers/gpu/nova-core/firmware/booter.rs | 22 ++++++---
>  drivers/gpu/nova-core/firmware/fwsec.rs  | 60 ++++++++++++++----------
>  drivers/gpu/nova-core/firmware/riscv.rs  |  6 ++-
>  4 files changed, 57 insertions(+), 34 deletions(-)
>

Reviewed-by: Gary Guo <gary@garyguo.net>

>
> base-commit: cea7b66a80412e2a5b74627b89ae25f1d0110a4b
> --
> 2.34.1

Re: [PATCH v2 0/5] gpu: nova-core: use checked arithmetic for firmware parsing robustness

[Alexandre Courbot]

On Tue Jan 27, 2026 at 5:23 AM JST, Joel Fernandes wrote:
> Changes from v1 to v2:
> - Added Reviewed-by tags from Zhi
> - Fixed comment formatting nits raised by Dirk/Zhi
>
> This series adds checked arithmetic throughout nova-core's firmware parsing
> code to guard rust code against integer overflow from corrupt firmware.
>
> Without checked arithmetic, firmware could cause integer overflow when
> computing offsets. The danger is not just wrapping to a huge value (which may
> fail validation in other paths), but potentially wrapping to a small plausible
> offset that accesses entirely wrong data, causing silent corruption or security
> issues.
>
> This series has been rebased on drm-rust-next. If possible, I would like us to
> consider merging for the upcoming merge window to avoid future conflicts.
> Tested probing with GPU name printed in dmesg on my GA102 (Ampere).
>
> The git tree with all patches can be found at:
> git://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git (tag: nova-checked-arith-v2-20260126)
>
> Link to v1: https://lore.kernel.org/all/20260124231830.3088323-1-joelagnelf@nvidia.com/
>
> Joel Fernandes (5):
>   gpu: nova-core: use checked arithmetic in FWSEC firmware parsing
>   gpu: nova-core: use checked arithmetic in Booter signature parsing
>   gpu: nova-core: use checked arithmetic in frombytes_at helper
>   gpu: nova-core: use checked arithmetic in BinFirmware::data
>   gpu: nova-core: use checked arithmetic in RISC-V firmware parsing

Looking good, thanks! I'm staging these into a local branch and will
push as soon as `drm-rust-next` reopens.

Re: [PATCH v2 0/5] gpu: nova-core: use checked arithmetic for firmware parsing robustness

[Alexandre Courbot]

On Wed Jan 28, 2026 at 4:59 PM JST, Alexandre Courbot wrote:
> On Tue Jan 27, 2026 at 5:23 AM JST, Joel Fernandes wrote:
>> Changes from v1 to v2:
>> - Added Reviewed-by tags from Zhi
>> - Fixed comment formatting nits raised by Dirk/Zhi
>>
>> This series adds checked arithmetic throughout nova-core's firmware parsing
>> code to guard rust code against integer overflow from corrupt firmware.
>>
>> Without checked arithmetic, firmware could cause integer overflow when
>> computing offsets. The danger is not just wrapping to a huge value (which may
>> fail validation in other paths), but potentially wrapping to a small plausible
>> offset that accesses entirely wrong data, causing silent corruption or security
>> issues.
>>
>> This series has been rebased on drm-rust-next. If possible, I would like us to
>> consider merging for the upcoming merge window to avoid future conflicts.
>> Tested probing with GPU name printed in dmesg on my GA102 (Ampere).
>>
>> The git tree with all patches can be found at:
>> git://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git (tag: nova-checked-arith-v2-20260126)
>>
>> Link to v1: https://lore.kernel.org/all/20260124231830.3088323-1-joelagnelf@nvidia.com/
>>
>> Joel Fernandes (5):
>>   gpu: nova-core: use checked arithmetic in FWSEC firmware parsing
>>   gpu: nova-core: use checked arithmetic in Booter signature parsing
>>   gpu: nova-core: use checked arithmetic in frombytes_at helper
>>   gpu: nova-core: use checked arithmetic in BinFirmware::data
>>   gpu: nova-core: use checked arithmetic in RISC-V firmware parsing
>
> Looking good, thanks! I'm staging these into a local branch and will
> push as soon as `drm-rust-next` reopens.

Pushed into `drm-rust-next`.