| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f73512e9-085c-4cb3-be23-325c69ec85dc@nvidia.com>
Date: Wed, 3 Dec 2025 23:55:06 -0800
From: John Hubbard <jhubbard@...dia.com>
To: Joel Fernandes <joelagnelf@...dia.com>, Danilo Krummrich <dakr@...nel.org>
Cc: Alexandre Courbot <acourbot@...dia.com>, Timur Tabi <ttabi@...dia.com>,
Alistair Popple <apopple@...dia.com>, Edwin Peer <epeer@...dia.com>,
Zhi Wang <zhiw@...dia.com>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, 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>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
nouveau@...ts.freedesktop.org, rust-for-linux@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 21/31] gpu: nova-core: Hopper/Blackwell: add FMC signature
extraction
On 12/3/25 7:45 AM, Joel Fernandes wrote:
> 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 <jhubbard@...dia.com>
>> ---
>> 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.
Yes, absolutely. Thanks for checking on this, I was having trouble
drawing the line at the right amount of output--I'm sure there is
still too much, now that you point it out.
>
> [1] https://lore.kernel.org/all/d6c9c7f2-098e-4b55-b754-4287b698fc1c@nvidia.com/
>
>> +
>> + // 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.
Yes.
>
>> + // 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.
Sure, I'll go in this direction, thanks for spotting that.
thanks,
--
John Hubbard
>
> [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)
>> + }
>> }
>
Powered by blists - more mailing lists