Hi John, On 12/3/2025 12:59 AM, John Hubbard wrote: > Add extract_fmc_signatures_static() to parse cryptographic signatures > from FMC ELF firmware sections. This extracts the SHA-384 hash, RSA > public key, and signature needed for Chain of Trust verification. > > Also exposes the elf_section() helper from firmware.rs for use by FSP. > > Signed-off-by: John Hubbard <[email protected]> > --- > drivers/gpu/nova-core/firmware.rs | 4 +- > drivers/gpu/nova-core/fsp.rs | 104 ++++++++++++++++++++++++++++++ > 2 files changed, 107 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/nova-core/firmware.rs > b/drivers/gpu/nova-core/firmware.rs > index 5cbb8be7434f..7f8d62f9ceba 100644 > --- a/drivers/gpu/nova-core/firmware.rs > +++ b/drivers/gpu/nova-core/firmware.rs > @@ -23,6 +23,8 @@ > }, > }; > > +pub(crate) use elf::elf_section; > + > pub(crate) mod booter; > pub(crate) mod fsp; > pub(crate) mod fwsec; > @@ -419,7 +421,7 @@ fn elf32_section<'a>(elf: &'a [u8], name: &str) -> > Option<&'a [u8]> { > } > > /// Automatically detects ELF32 vs ELF64 based on the ELF header. > - pub(super) fn elf_section<'a>(elf: &'a [u8], name: &str) -> Option<&'a > [u8]> { > + pub(crate) fn elf_section<'a>(elf: &'a [u8], name: &str) -> Option<&'a > [u8]> { > // Check ELF magic. > if elf.len() < 5 || elf.get(0..4)? != b"\x7fELF" { > return None; > diff --git a/drivers/gpu/nova-core/fsp.rs b/drivers/gpu/nova-core/fsp.rs > index 389c43bfd538..311b6d4c6011 100644 > --- a/drivers/gpu/nova-core/fsp.rs > +++ b/drivers/gpu/nova-core/fsp.rs > @@ -256,4 +256,108 @@ pub(crate) fn wait_secure_boot( > }) > .map(|_| ()) > } > + > + /// Extract FMC firmware signatures for Chain of Trust verification. > + /// > + /// Extracts real cryptographic signatures from FMC ELF32 firmware > sections. > + /// Returns signatures in a heap-allocated structure to prevent stack > overflow. > + pub(crate) fn extract_fmc_signatures_static( > + dev: &device::Device<device::Bound>, > + fmc_fw_data: &[u8], > + ) -> Result<KBox<FmcSignatures>> { > + dev_dbg!(dev, "FMC firmware size: {} bytes\n", fmc_fw_data.len());
Let us remove these? I think we discussed [1] that once things are working, we'd not want these and can add it on-demand if needed. [1] https://lore.kernel.org/all/[email protected]/ > + > + // Extract hash section (SHA-384) > + let hash_section = crate::firmware::elf_section(fmc_fw_data, "hash") > + .ok_or(EINVAL) > + .inspect_err(|_| dev_err!(dev, "FMC firmware missing 'hash' > section\n"))?; > + > + // Extract public key section (RSA public key) > + let pkey_section = crate::firmware::elf_section(fmc_fw_data, > "publickey") > + .ok_or(EINVAL) > + .inspect_err(|_| dev_err!(dev, "FMC firmware missing 'publickey' > section\n"))?; > + > + // Extract signature section (RSA signature) > + let sig_section = crate::firmware::elf_section(fmc_fw_data, > "signature") > + .ok_or(EINVAL) > + .inspect_err(|_| dev_err!(dev, "FMC firmware missing 'signature' > section\n"))?; > + > + dev_dbg!( > + dev, > + "FMC ELF sections: hash={} bytes, pkey={} bytes, sig={} bytes\n", > + hash_section.len(), > + pkey_section.len(), > + sig_section.len() > + ); > + Here as well. > + // Validate section sizes - hash must be exactly 48 bytes > + if hash_section.len() != FSP_HASH_SIZE { > + dev_err!( > + dev, > + "FMC hash section size {} != expected {}\n", > + hash_section.len(), > + FSP_HASH_SIZE > + ); > + return Err(EINVAL); > + } > + > + // Public key and signature can be smaller than the fixed array sizes > + if pkey_section.len() > FSP_PKEY_SIZE * 4 { > + dev_err!( > + dev, > + "FMC publickey section size {} > maximum {}\n", > + pkey_section.len(), > + FSP_PKEY_SIZE * 4 > + ); > + return Err(EINVAL); > + } > + > + if sig_section.len() > FSP_SIG_SIZE * 4 { > + dev_err!( > + dev, > + "FMC signature section size {} > maximum {}\n", > + sig_section.len(), > + FSP_SIG_SIZE * 4 > + ); > + return Err(EINVAL); > + } > + > + // Allocate signature structure on heap to avoid stack overflow > + let mut signatures = KBox::new(FmcSignatures::default(), > GFP_KERNEL)?; > + > + // Copy hash section directly as bytes (48 bytes exactly) > + // SAFETY: hash384 is a [u32; 12] array (48 bytes), and we create a > byte slice of > + // exactly FSP_HASH_SIZE (48) bytes. The pointer is valid and > properly aligned. > + let hash_bytes = unsafe { > + core::slice::from_raw_parts_mut( > + signatures.hash384.as_mut_ptr().cast::<u8>(), > + FSP_HASH_SIZE, > + ) > + }; > + hash_bytes.copy_from_slice(hash_section); > + > + // Copy public key section (up to 388 bytes, zero-padded) > + // SAFETY: public_key is a [u32; 96] array (384 bytes), and we > create a byte slice of > + // FSP_PKEY_SIZE * 4 bytes. The pointer is valid and properly > aligned. > + let pkey_bytes = unsafe { > + core::slice::from_raw_parts_mut( > + signatures.public_key.as_mut_ptr().cast::<u8>(), > + FSP_PKEY_SIZE * 4, > + ) > + }; > + pkey_bytes[..pkey_section.len()].copy_from_slice(pkey_section); Even if this works in practice, I believe it's UB as the `from_raw_parts_mut()` should have the entire slice range to be valid memory (see [2]), but FSP_PKEY_SIZE * 4 is 388 bytes while public_key is only 384 bytes ([u32; 96]). This is vulnerable as the KBox holding the signature may not have the extra space even if it does now. Can we create a slice with exactly the bytes we need? something like: let pkey_bytes = unsafe { core::slice::from_raw_parts_mut( signatures.public_key.as_mut_ptr().cast::<u8>(), pkey_section.len(), ) }; pkey_bytes.copy_from_slice(pkey_section); Another reason for doing this is, the code is more fragile left as is, as there is a risk of unrelated memory leaking into the slice and accessed by new/future code. [2] "Behavior is undefined if any of the following conditions are violated" https://doc.rust-lang.org/std/slice/fn.from_raw_parts_mut.html thanks, - Joel > + > + // Copy signature section (up to 384 bytes, zero-padded) > + // SAFETY: signature is a [u32; 96] array (384 bytes), and we create > a byte slice of > + // FSP_SIG_SIZE * 4 bytes. The pointer is valid and properly aligned. > + let sig_bytes = unsafe { > + core::slice::from_raw_parts_mut( > + signatures.signature.as_mut_ptr().cast::<u8>(), > + FSP_SIG_SIZE * 4, > + ) > + }; > + sig_bytes[..sig_section.len()].copy_from_slice(sig_section); > + > + Ok(signatures) > + } > }
