Re: [PATCH v12 13/22] gpu: nova-core: Hopper/Blackwell: add FMC signature extraction

[Alexandre Courbot]

On Tue Jun 2, 2026 at 12:32 PM JST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The patch uses `field: expr` instead of `field <- expr` inside `init!`, violating Rust subsystem guidelines and defeating the goal of avoiding stack allocation.
> --
>
> commit 7149f8afdf16eef886b0d72d3c2139493024923e
> Author: John Hubbard <jhubbard@xxxxxxxxxx>
>
> gpu: nova-core: Hopper/Blackwell: add FMC signature extraction
>     
> This commit extracts the SHA-384 hash, RSA public key, and RSA signature
> from FMC ELF32 firmware sections. The signatures are stored in a
> heap-allocated structure used for FSP Chain of Trust verification.
>
>> diff --git a/drivers/gpu/nova-core/firmware/fsp.rs b/drivers/gpu/nova-core/firmware/fsp.rs
>> index 011be1e571c25..db61905eac9de 100644
>> --- a/drivers/gpu/nova-core/firmware/fsp.rs
>> +++ b/drivers/gpu/nova-core/firmware/fsp.rs
>
> [ ... ]
>
>> @@ -41,7 +63,73 @@ pub(crate) fn new(
>>  
>>          Ok(Self {
>>              fmc_image,
>> -            fmc_elf: fw,
>> +            fmc_sigs: Self::extract_fmc_signatures(&fw, dev)?,
>>          })
>>      }
>> +
>> +    /// Extract FMC firmware signatures for Chain of Trust verification.
>
> [ ... ]
>
>> +        // Initialize the signatures in place to avoid building the large `FmcSignatures` on the
>> +        // stack, then fill each section from the firmware.
>> +        let signatures = KBox::init(
>> +            init!(FmcSignatures {
>> +                hash384: [0; _],
>> +                public_key: [0; _],
>> +                signature: [0; _],
>> +            })
>
> [Severity: Medium]
> Does this init! block allocate these arrays on the stack before moving them 
> to the heap?
>
> According to the Rust subsystem coding guidelines, fields that require in-place
> initialization should use the `field <- expr` syntax rather than `field: expr`. 
>
> Because `hash384: [0; _]` and the other fields use the colon syntax, the
> arrays (which are up to 384 bytes each) might be evaluated as temporaries on
> the stack, which defeats the goal mentioned in the comment above.
>
> Could these be updated to use the in-place initialization syntax, such as 
> `hash384 <- kernel::init::zeroed()` or similar?

That's a good observation. One can also derive `Zeroable` on
`FmcSignatures` to simplify further:

--- a/drivers/gpu/nova-core/firmware/fsp.rs
+++ b/drivers/gpu/nova-core/firmware/fsp.rs
@@ -29,7 +29,7 @@
 /// Structure to hold FMC signatures.
 ///
 /// C representation is used because this type is used for communication with the FSP.
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, Zeroable)]
 #[repr(C)]
 pub(crate) struct FmcSignatures {
     pub(crate) hash384: [u8; FSP_HASH_SIZE],
@@ -113,12 +113,7 @@ fn extract_fmc_signatures(
         // Initialize the signatures in place to avoid building the large `FmcSignatures` on the
         // stack, then fill each section from the firmware.
         let signatures = KBox::init(
-            init!(FmcSignatures {
-                hash384: [0; _],
-                public_key: [0; _],
-                signature: [0; _],
-            })
-            .chain(|sigs| {
+            pin_init::init_zeroed::<FmcSignatures>().chain(|sigs| {
                 // PANIC: src and dst lengths are both FSP_HASH_SIZE (verified above).
                 sigs.hash384.copy_from_slice(hash_section);
                 // PANIC: dst is sliced to src.len(); src.len() <= FSP_PKEY_SIZE per `get_section`.

I will apply this chunk when pushing.