On 2/7/23 16:48, Jason A. Donenfeld wrote:
From: Dov Murik <dovmu...@linux.ibm.com>Modifying the cmdline by appending setup_data breaks measured boot with SEV and OVMF, and possibly signed boot. Previously this was disabled when appending to the kernel image, but with eac7a7791bb6 ("x86: don't let decompressed kernel image clobber setup_data"), this was changed to the cmdline file instead, with the sev_enabled() check left out. Fixes: eac7a7791bb6 ("x86: don't let decompressed kernel image clobber setup_data") Reported-by: Tom Lendacky <thomas.lenda...@amd.com> Signed-off-by: Dov Murik <dovmu...@linux.ibm.com> Signed-off-by: Jason A. Donenfeld <ja...@zx2c4.com> --- hw/i386/x86.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/i386/x86.c b/hw/i386/x86.c index c6d7bf6db2..80a1678acd 100644 --- a/hw/i386/x86.c +++ b/hw/i386/x86.c @@ -1079,7 +1079,7 @@ void x86_load_linux(X86MachineState *x86ms, fclose(f);/* append dtb to kernel */- if (dtb_filename) { + if (dtb_filename && !sev_enabled()) {
Is this change necessary? From an SEV point of view, the DTB file should be "constant" and so a guest owner will be able to use that to correctly verify the SEV LAUNCH measurement. Additionally, it won't change from the time it was measured to the time OVMF validates everything. Of course, I don't really anticipate that a DTB file would be used with an SEV guest, anyway.
@Dov, does that make sense? Thanks, Tom
if (protocol < 0x209) { fprintf(stderr, "qemu: Linux kernel too old to load a dtb\n"); exit(1); @@ -1103,7 +1103,7 @@ void x86_load_linux(X86MachineState *x86ms, load_image_size(dtb_filename, setup_data->data, dtb_size); }- if (!legacy_no_rng_seed && protocol >= 0x209) {+ if (!legacy_no_rng_seed && protocol >= 0x209 && !sev_enabled()) { setup_data_offset = cmdline_size; cmdline_size += sizeof(SetupData) + RNG_SEED_LENGTH; kernel_cmdline = g_realloc(kernel_cmdline, cmdline_size);
