Re: [PATCH] x86/kernel: Validate ROM before DMI scanning when SEV-SNP is active

[Kevin Loughlin]

On Tue, Feb 13, 2024 at 12:03 PM Michael Roth <michael.roth@xxxxxxx> wrote:
>
> Quoting Kevin Loughlin (2024-02-12 22:07:46)
> > SEV-SNP requires encrypted memory to be validated before access. The
> > kernel is responsible for validating the ROM memory range because the
> > range is not part of the e820 table and therefore not pre-validated by
> > the BIOS.
> >
> > While the current SEV-SNP code attempts to validate the ROM range in
> > probe_roms(), this does not suffice for all existing use cases. In
> > particular, if EFI_CONFIG_TABLES are not enabled and
> > CONFIG_DMI_SCAN_MACHINE_NON_EFI_FALLBACK is set, the kernel will
> > attempt to access the memory at SMBIOS_ENTRY_POINT_SCAN_START (which
> > falls in the ROM range) prior to validation. The specific problematic
> > call chain occurs during dmi_setup() -> dmi_scan_machine() and results
> > in a crash during boot if SEV-SNP is enabled under these conditions.
>
> AFAIK, QEMU doesn't actually include any legacy ROMs as part of the initial
> encrypted guest image, and I'm not aware of any VMM implementations that
> do this either.

I'm using a VMM implementation that uses (non-EFI) Oak stage0 firmware [0].

[0] https://github.com/project-oak/oak/tree/main/stage0_bin

> If dmi_setup() similarly scans these ranges, it seems likely the same
> issue would be present: the validated/private regions would only contain
> ciphertext rather than the expected ROM data. Does that agree with the
> behavior you are seeing?
>
> If so, maybe instead probe_roms should just be skipped in the case of SNP?

If probe_roms() is skipped, SEV-SNP guest boot also currently crashes;
I just quickly tried that (though admittedly haven't looked into why).
Apparently though, the fix for early ROM range accesses is not as
simple as just skipping probe_roms() if SEV-SNP is enabled.
Furthermore, skipping probe_roms() was also *not* the route taken in
the initial attempt that prevents this issue for EFI use cases [1].

[1] https://lore.kernel.org/lkml/20220307213356.2797205-21-brijesh.singh@xxxxxxx/

> And perhaps dmi_setup() should similarly skip the legacy ROM ranges for
> the kernel configs in question?

Given (a) non-EFI firmware is supported in other SME/SEV boot code
patches [2], (b) this patch does not seem to introduce significant
complexity (it just moves [1] to earlier in the boot process to
additionally handle the non-EFI case), and (c) skipping
probe_roms()+dmi_setup() doesn't work without additional changes, I'm
currently still inclined to simply validate the legacy ROM ranges
early enough to prevent this issue (as is already done when using EFI
firmware).

[2] https://lore.kernel.org/lkml/CAMj1kXFZKM5wU8djcVBxDmnCJwV4Xpest6u1EbE=7wyLUUeUUQ@xxxxxxxxxxxxxx/