From f149d75d7f97d28f27cfe87965c9c3b79ad41642 Mon Sep 17 00:00:00 2001 From: Brennan Tracy Date: Tue, 10 Nov 2026 18:19:47 +0000 Subject: [PATCH] util: stop using uninitialized memory Initialize memory with zeroes before reading data into it. Read raw bytes only into types that accept every bit pattern. The crate constructed values from uninitialized memory. This causes undefined behavior. Signed-off-by: Brennan Tracy --- src/certs/sev/ca/cert/v1.rs | 3 ++ src/certs/sev/sev/cert/mod.rs | 5 +++ src/certs/sev/sev/cert/v1/mod.rs | 8 +++++ src/firmware/host/mod.rs | 11 +++--- src/firmware/host/types/sev.rs | 3 ++ src/launch/sev.rs | 10 ++++++ src/util/mod.rs | 60 ++++++++++++++++++++++++++++---- src/vmsa/mod.rs | 3 ++ 8 files changed, 90 insertions(+), 13 deletions(-) diff --git a/src/certs/sev/ca/cert/v1.rs b/src/certs/sev/ca/cert/v1.rs index db02db1c..39284bfe 100644 --- a/src/certs/sev/ca/cert/v1.rs +++ b/src/certs/sev/ca/cert/v1.rs @@ -187,6 +187,9 @@ impl PartialEq for Certificate { } } +// SAFETY: All fields of Data, recursively, are integers or arrays of integers. +unsafe impl crate::util::AnyBitPattern for Data {} + impl Decoder<()> for Certificate { fn decode(reader: &mut impl Read, _: ()) -> Result { let p = Preamble { diff --git a/src/certs/sev/sev/cert/mod.rs b/src/certs/sev/sev/cert/mod.rs index ef16ac45..52e12aaf 100644 --- a/src/certs/sev/sev/cert/mod.rs +++ b/src/certs/sev/sev/cert/mod.rs @@ -74,6 +74,11 @@ impl> PartialEq for Certificate { } } +// SAFETY: Certificate is a union of a u32 and a v1 certificate. All fields +// of the v1 certificate, recursively, are integers, arrays, or unions of +// these. +unsafe impl crate::util::AnyBitPattern for Certificate {} + impl Decoder<()> for Certificate { fn decode(reader: &mut impl Read, params: ()) -> Result { Ok(match u32::from_le(reader.load()?) { diff --git a/src/certs/sev/sev/cert/v1/mod.rs b/src/certs/sev/sev/cert/v1/mod.rs index 486e5a57..a08f9739 100644 --- a/src/certs/sev/sev/cert/v1/mod.rs +++ b/src/certs/sev/sev/cert/v1/mod.rs @@ -29,6 +29,14 @@ impl Certificate { } } +// SAFETY: All fields of Data, recursively, are integers, arrays, or unions of +// these. +unsafe impl crate::util::AnyBitPattern for body::Data {} + +// SAFETY: All fields of Signature, recursively, are integers, arrays, or +// unions of these. +unsafe impl crate::util::AnyBitPattern for sig::Signature {} + impl Decoder<()> for Certificate { fn decode(reader: &mut impl Read, _: ()) -> Result { Ok(Self { diff --git a/src/firmware/host/mod.rs b/src/firmware/host/mod.rs index 712aa96c..e7f069fd 100644 --- a/src/firmware/host/mod.rs +++ b/src/firmware/host/mod.rs @@ -31,7 +31,7 @@ use std::{ #[cfg(feature = "sev")] #[cfg(target_os = "linux")] -use std::mem::MaybeUninit; +use crate::util::AnyBitPattern; #[cfg(feature = "snp")] #[cfg(target_os = "linux")] @@ -131,8 +131,7 @@ impl Firmware { /// Request a signature for the PEK. #[cfg(feature = "sev")] pub fn pek_csr(&mut self) -> Result { - #[allow(clippy::uninit_assumed_init)] - let mut pek: Certificate = unsafe { MaybeUninit::uninit().assume_init() }; + let mut pek = Certificate::zeroed(); let mut csr = PekCsr::new(&mut pek); let mut cmd_buf = Command::from_mut(&mut csr); PEK_CSR @@ -155,10 +154,8 @@ impl Firmware { /// Export the SEV certificate chain. #[cfg(feature = "sev")] pub fn pdh_cert_export(&mut self) -> Result { - #[allow(clippy::uninit_assumed_init)] - let mut chain: [Certificate; 3] = unsafe { MaybeUninit::uninit().assume_init() }; - #[allow(clippy::uninit_assumed_init)] - let mut pdh: Certificate = unsafe { MaybeUninit::uninit().assume_init() }; + let mut chain = <[Certificate; 3]>::zeroed(); + let mut pdh = Certificate::zeroed(); let mut pdh_cert_export = PdhCertExport::new(&mut pdh, &mut chain); let mut cmd_buf = Command::from_mut(&mut pdh_cert_export); diff --git a/src/firmware/host/types/sev.rs b/src/firmware/host/types/sev.rs index 4c3978b2..c74dd1da 100644 --- a/src/firmware/host/types/sev.rs +++ b/src/firmware/host/types/sev.rs @@ -80,6 +80,9 @@ impl std::fmt::Display for Build { } } +// SAFETY: Build and Version contain only u8 fields. +unsafe impl crate::util::AnyBitPattern for Build {} + impl Decoder<()> for Build { fn decode(reader: &mut impl Read, _: ()) -> std::io::Result { reader.load() diff --git a/src/launch/sev.rs b/src/launch/sev.rs index 30daf00d..e401deb1 100644 --- a/src/launch/sev.rs +++ b/src/launch/sev.rs @@ -383,6 +383,10 @@ pub struct Start { pub session: Session, } +// SAFETY: All fields of Start, recursively, are integers, arrays, or unions +// of these. The bit flag types wrap integers. +unsafe impl crate::util::AnyBitPattern for Start {} + impl Decoder<()> for Start { fn decode(reader: &mut impl Read, _: ()) -> std::io::Result { reader.load() @@ -460,6 +464,9 @@ pub struct Secret { pub ciphertext: Vec, } +// SAFETY: Header contains bit flags over a u32 and byte arrays. +unsafe impl crate::util::AnyBitPattern for Header {} + impl Decoder<()> for Secret { fn decode(reader: &mut impl Read, _: ()) -> std::io::Result { let header = reader.load()?; @@ -488,6 +495,9 @@ pub struct Measurement { pub mnonce: [u8; 16], } +// SAFETY: Measurement contains only byte arrays. +unsafe impl crate::util::AnyBitPattern for Measurement {} + impl Decoder<()> for Measurement { fn decode(reader: &mut impl Read, _: ()) -> std::io::Result { reader.load() diff --git a/src/util/mod.rs b/src/util/mod.rs index 18168996..c5cb7c6c 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -14,14 +14,37 @@ use std::{ slice::{from_raw_parts, from_raw_parts_mut}, }; +/// A type for which every initialized bit pattern of `size_of::()` bytes +/// is a valid value. +/// +/// # Safety +/// +/// Implement this only for types whose fields, recursively, are all integers, +/// arrays, or unions of such types. Types that contain `bool`, `char`, enums, +/// references, pointers, or `NonZero*` values must not implement it. +pub unsafe trait AnyBitPattern: Copy { + /// Returns a value with every byte set to zero. + fn zeroed() -> Self { + // SAFETY: Self accepts every bit pattern, including all zeroes. + unsafe { MaybeUninit::zeroed().assume_init() } + } +} + +// SAFETY: Every bit pattern is a valid u32. +unsafe impl AnyBitPattern for u32 {} + +// SAFETY: An array is valid when each element is valid. +unsafe impl AnyBitPattern for [T; N] {} + pub trait TypeLoad: Read { - fn load(&mut self) -> Result { - #[allow(clippy::uninit_assumed_init)] - let mut t = unsafe { MaybeUninit::uninit().assume_init() }; - let p = &mut t as *mut T as *mut u8; - let s = unsafe { from_raw_parts_mut(p, size_of::()) }; + fn load(&mut self) -> Result { + let mut t = MaybeUninit::::zeroed(); + // SAFETY: t owns size_of::() bytes. zeroed() initialized all of + // them, which makes them valid u8 values. + let s = unsafe { from_raw_parts_mut(t.as_mut_ptr() as *mut u8, size_of::()) }; self.read_exact(s)?; - Ok(t) + // SAFETY: read_exact filled every byte, and T accepts any bit pattern. + Ok(unsafe { t.assume_init() }) } } @@ -35,3 +58,28 @@ pub trait TypeSave: Write { impl TypeLoad for T {} impl TypeSave for T {} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_load_reads_value() { + let mut reader: &[u8] = &[1, 2, 3, 4, 5, 6, 7, 8]; + let value: [u32; 2] = reader.load().unwrap(); + assert_eq!( + value, + [ + u32::from_ne_bytes([1, 2, 3, 4]), + u32::from_ne_bytes([5, 6, 7, 8]) + ] + ); + } + + #[test] + fn test_load_short_input() { + let mut reader: &[u8] = &[1, 2, 3]; + let err = reader.load::().unwrap_err(); + assert_eq!(err.kind(), std::io::ErrorKind::UnexpectedEof); + } +} diff --git a/src/vmsa/mod.rs b/src/vmsa/mod.rs index b705d0b0..08b3a64b 100644 --- a/src/vmsa/mod.rs +++ b/src/vmsa/mod.rs @@ -393,6 +393,9 @@ impl Default for Vmsa { } } +// SAFETY: Vmsa and VmcbSegment contain only integers and integer arrays. +unsafe impl crate::util::AnyBitPattern for Vmsa {} + impl Decoder<()> for Vmsa { fn decode(reader: &mut impl Read, _: ()) -> Result { reader.load()