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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAGdbjmKSpQSh=xr7MguoSgP85VTR5eu1xUeUXD8RtvZn-=MW3A@mail.gmail.com>
Date: Thu, 26 Dec 2024 10:56:19 -0500
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, 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)?

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?

[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.