linux-kernel - Re: [PATCH] sev-snp: parse MP tables for VMware hypervisor

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAGdbjm+vuSDO_Zm4CiJgVEx1vx+LAT1+fVQG8xj41uYhBY9qkA@mail.gmail.com>
Date: Fri, 17 Jan 2025 14:56:17 -0800
From: Kevin Loughlin <kevinloughlin@...gle.com>
To: Ajay Kaher <ajay.kaher@...adcom.com>
Cc: bp@...en8.de, bcm-kernel-feedback-list@...adcom.com, tglx@...utronix.de, 
	mingo@...hat.com, dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, 
	virtualization@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	ye.li@...adcom.com, bo.gan@...adcom.com, 
	vamsi-krishna.brahmajosyula@...adcom.com, alexey.makhalov@...adcom.com, 
	vasavi.sirnapalli@...adcom.com, florian.fainelli@...adcom.com
Subject: Re: [PATCH] sev-snp: parse MP tables for VMware hypervisor

On Thu, Jan 2, 2025 at 8:31 PM Ajay Kaher <ajay.kaher@...adcom.com> wrote:
>
> On Thu, Dec 26, 2024 at 9:26 PM Kevin Loughlin <kevinloughlin@...gle.com> wrote:
> >
> > On Thu, Dec 19, 2024 at 6:44 AM Ajay Kaher <ajay.kaher@...adcom.com> wrote:
> > >
> > > For VMware hypervisor, SEV-SNP enabled VM's could boot without UEFI.
> > > In this case, mpparse_find_mptable() has to be called to parse MP
> > > tables which contains boot information.
> > >
> > > Fixes: 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for SEV-SNP guests")
> > > Signed-off-by: Ajay Kaher <ajay.kaher@...adcom.com>
> > > Signed-off-by: Ye Li <ye.li@...adcom.com>
> > > Tested-by: Ye Li <ye.li@...adcom.com>
> > > ---
> > >  arch/x86/kernel/cpu/vmware.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> > > index 00189cd..3e2594d 100644
> > > --- a/arch/x86/kernel/cpu/vmware.c
> > > +++ b/arch/x86/kernel/cpu/vmware.c
> > > @@ -26,6 +26,7 @@
> > >  #include <linux/export.h>
> > >  #include <linux/clocksource.h>
> > >  #include <linux/cpu.h>
> > > +#include <linux/efi.h>
> > >  #include <linux/reboot.h>
> > >  #include <linux/static_call.h>
> > >  #include <asm/div64.h>
> > > @@ -35,6 +36,8 @@
> > >  #include <asm/apic.h>
> > >  #include <asm/vmware.h>
> > >  #include <asm/svm.h>
> > > +#include <asm/mem_encrypt.h>
> > > +#include <asm/efi.h>
> > >
> > >  #undef pr_fmt
> > >  #define pr_fmt(fmt)    "vmware: " fmt
> > > @@ -429,6 +432,10 @@ static void __init vmware_platform_setup(void)
> > >                 pr_warn("Failed to get TSC freq from the hypervisor\n");
> > >         }
> > >
> > > +       if (sev_status & MSR_AMD64_SEV_SNP_ENABLED &&
> > > +           !efi_enabled(EFI_BOOT))
> > > +               x86_init.mpparse.find_mptable = mpparse_find_mptable;
> >
> > As far I know, Linux itself currently doesn't PVALIDATE the address
> > ranges scanned in mpparse_find_mptable(), and Linux accesses these
> > addresses as encrypted during early boot. Given this, am I correct
> > that the guest firmware that you're using is doing the PVALIDATE of
> > these ranges before starting Linux (else there would be a crash upon
> > scan)? And then presumably the firmware is also making sure that this
> > memory is encrypted so that Linux isn't reading unencrypted data as
> > encrypted (i.e., garbage)?
>
> Yes, Linux (as a guest) receives valid data.
>
> > If so, does that mean all the ROM region scans removed in [0] are
> > permissible for SEV-SNP guests booting on whichever guest firmware
> > this is? But you only want/need the mptable info here?
>
> Yes, VMware firmware validates MPTABLE info when 'non-EFI guest boots
> on VMware Hypervisor'. The other ROM regions removed in [0] were not
> encrypted and validated. This is a specific use case for the VMware platform.
> Linux today assumes SNP VMs will be booted with UEFI.
>
> > [0] 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for
> > SEV-SNP guests")
> >
> > More broadly, ideally the guest firmware would communicate to Linux
> > that these ranges are safe to access (perhaps via the e820 table),
> > rather than Linux making the assumption that the ranges are safe for
> > non-EFI SEV-SNP guest boots in this scenario. However, since you're
> > only changing vmware_platform_setup() for such boots, I don't think we
> > need to hold up this patch on that generality.
>
> This is a VMware specific scenario. VMware Hypervisor makes the MPTABLE
> memory available in e820 when 'non-EFI guest boots on VMware Hypervisor'.
> So it makes sense to do the changes only for vmware_platform_setup().

Works for me. Just wanted to make sure we don't regress Linux to
making assumptions about the safety of accessing memory regions in
CoCo guests without having entries in the e820 table for them.

Reviewed-by: Kevin Loughlin <kevinloughlin@...gle.com>