-
Notifications
You must be signed in to change notification settings - Fork 61
Description
The current variant-determination logic in sev contains several fallback behaviors that are either incorrect or no longer justified by current and future hardware. These fallbacks should be removed in favor of explicit errors when the input cannot be interpreted unambiguously.
This issue focuses on sev itself; a related issue will be filed separately for the downstream snpguest.
1. Attestation Report version fallback
Currently, Attestation Report versions other than 2, 3, 4, or 5 are treated as falling back to version 5 (ReportVariant::V5). This is unsafe.
sev/src/firmware/guest/types/snp.rs
Lines 216 to 226 in 6afb12c
| fn decode(reader: &mut impl Read, _: ()) -> Result<Self, std::io::Error> { | |
| let version: u32 = reader.read_bytes()?; | |
| Ok(match version { | |
| 0 | 1 => return Err(std::io::ErrorKind::Unsupported.into()), | |
| 2 => Self::V2, | |
| 3 | 4 => Self::V3, | |
| 5 => Self::V5, | |
| _ => Self::V5, | |
| }) | |
| } | |
| } |
- The known set of report versions is {2, 3, 4, 5}. (Note: No information about V4 report has been disclosed by AMD...)
- Any version outside this set should be considered invalid or unsupported input.
- Falling back to V5 silently risks mis-parsing an unknown format.
Proposed change
If version is not 2, 3, 4 or 5, return an error instead of falling back to V5.
2. TCB Version / processor model fallback for V2 report
For ReportVariant::V3 or later, the processor model can be determined reliably from CPU_FAM_ID and CPU_MOD_ID, and this approach is already used downstream snpguest. However, for ReportVariant::V2, since V2 reports have neither CPU_FAM_ID nor CPU_MOD_ID, the current code relies on heuristics and fallbacks that are no longer valid.
sev/src/firmware/guest/types/snp.rs
Lines 448 to 467 in 6afb12c
| fn decode(reader: &mut impl Read, _: ()) -> Result<Self, std::io::Error> { | |
| let mut bytes = vec![0u8; ATT_REP_FW_LEN]; | |
| reader.read_exact(&mut bytes)?; | |
| let variant = ReportVariant::from_bytes(&bytes[0..4])?; | |
| let generation = match variant { | |
| ReportVariant::V2 => { | |
| if Self::chip_id_is_turin_like(&bytes[CHIP_ID_RANGE])? { | |
| Generation::Turin | |
| } else { | |
| Generation::Genoa | |
| } | |
| } | |
| _ => { | |
| let family = &bytes[CPUID_FAMILY_ID_BYTES]; | |
| let model = &bytes[CPUID_MODEL_ID_BYTES]; | |
| Generation::identify_cpu(*family, *model)? | |
| } | |
| }; |
Turin heuristic
The code assumes that:
- Turin truncates CPUID to 8 bytes with zero-fill
- If the trailing 54 bytes are zero (so-called "Turin-like"), the CPU is Turin
This assumption is now broken:
- Venice (the successor to Turin) exists
- So "Turin-like" no longer refers to Turin
Genoa fallback
If the CPU is determined to be non–Turin-like, the code falls back to Genoa. This fallback has no clear technical justification and risks misclassifying hardware.
Proposed change
- Remove the Turin-like heuristic for
ReportVariant::V2 - (Remove the fallback to Genoa)
- If the processor model cannot be determined unambiguously, return an error