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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGdbjmJAE5Uo5LA=hkMFwp+LdnNxxKgSwgkpdWZ69kwS-tLkFw@mail.gmail.com>
Date: Tue, 13 Feb 2024 15:10:46 -0800
From: Kevin Loughlin <kevinloughlin@...gle.com>
To: Michael Roth <michael.roth@....com>
Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Ard Biesheuvel <ardb@...nel.org>, 
	Baoquan He <bhe@...hat.com>, Borislav Petkov <bp@...en8.de>, Brijesh Singh <brijesh.singh@....com>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, Dionna Glaze <dionnaglaze@...gle.com>, 
	"H.Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>, Josh Poimboeuf <jpoimboe@...nel.org>, 
	Kai Huang <kai.huang@...el.com>, Peter Zijlstra <peterz@...radead.org>, 
	Ross Lagerwall <ross.lagerwall@...rix.com>, Thomas Gleixner <tglx@...utronix.de>, 
	Tom Lendacky <thomas.lendacky@....com>, Yuntao Wang <ytcoode@...il.com>, 
	linux-kernel@...r.kernel.org, x86@...nel.org, 
	Adam Dunlap <acdunlap@...gle.com>, Peter Gonda <pgonda@...gle.com>, Jacob Xu <jacobhxu@...gle.com>, 
	Sidharth Telang <sidtelang@...gle.com>, Conrad Grobler <grobler@...gle.com>, 
	Andri Saar <andrisaar@...gle.com>
Subject: Re: [PATCH] x86/kernel: Validate ROM before DMI scanning when SEV-SNP
 is active

On Tue, Feb 13, 2024 at 12:03 PM Michael Roth <michael.roth@....com> 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@amd.com/

> 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@mail.gmail.com/