On Tue, Aug 23, 2022 at 11:07:34AM -0500, Michael Roth wrote:

In some cases bootloaders will leave boot_params->cc_blob_address uninitialized rather than zero'ing it out. This field is only meant to be set by the boot/compressed kernel to pass information to the uncompressed kernel when SEV-SNP support is enabled, so there are no cases where the bootloader-provided values should be treated as anything other than garbage. Otherwise, the uncompressed kernel may attempt to access this bogus address, leading to a crash during early boot.

Normally sanitize_boot_params() would be used to clear out such fields, but that happens too late: sev_enable() may have already initialized it to a valid value that should not be zero'd out. Instead, have sev_enable() zero it out unconditionally beforehand.

Also ensure this happens for !CONFIG_AMD_MEM_ENCRYPT as well by also including this handling in the sev_enable() stub function.

Fixes: b190a043c49a ("x86/sev: Add SEV-SNP feature detection/setup") Cc: stable@vger.kernel.org Reported-by: Jeremi Piotrowski jpiotrowski@linux.microsoft.com Reported-by: watnuss@gmx.de Link: https://bugzilla.kernel.org/show_bug.cgi?id=216387 Signed-off-by: Michael Roth michael.roth@amd.com

---

arch/x86/boot/compressed/misc.h | 11 ++++++++++- arch/x86/boot/compressed/sev.c | 8 ++++++++ 2 files changed, 18 insertions(+), 1 deletion(-)

I extended the stub comment too so that it is clear that yes, the stub is supposed to do it too and not someone later wonders why there's code in the stub function.

--- From 4065a25f2b287e42642fe31509304cfd0a1ee125 Mon Sep 17 00:00:00 2001 From: Michael Roth michael.roth@amd.com Date: Tue, 23 Aug 2022 11:07:34 -0500 Subject: [PATCH] x86/boot: Don't propagate uninitialized boot_params->cc_blob_address

In some cases, bootloaders will leave boot_params->cc_blob_address uninitialized rather than zeroing it out. This field is only meant to be set by the boot/compressed kernel in order to pass information to the uncompressed kernel when SEV-SNP support is enabled.

Therefore, there are no cases where the bootloader-provided values should be treated as anything other than garbage. Otherwise, the uncompressed kernel may attempt to access this bogus address, leading to a crash during early boot.

Normally, sanitize_boot_params() would be used to clear out such fields but that happens too late: sev_enable() may have already initialized it to a valid value that should not be zeroed out. Instead, have sev_enable() zero it out unconditionally beforehand.

Also ensure this happens for !CONFIG_AMD_MEM_ENCRYPT as well by also including this handling in the sev_enable() stub function.

[ bp: Massage commit message and comments. ]

Fixes: b190a043c49a ("x86/sev: Add SEV-SNP feature detection/setup") Reported-by: Jeremi Piotrowski jpiotrowski@linux.microsoft.com Reported-by: watnuss@gmx.de Signed-off-by: Michael Roth michael.roth@amd.com Signed-off-by: Borislav Petkov bp@suse.de Cc: stable@vger.kernel.org Link: https://bugzilla.kernel.org/show_bug.cgi?id=216387 Link: https://lore.kernel.org/r/20220823160734.89036-1-michael.roth@amd.com --- arch/x86/boot/compressed/misc.h | 12 +++++++++++- arch/x86/boot/compressed/sev.c | 8 ++++++++ 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h index 4910bf230d7b..62208ec04ca4 100644 --- a/arch/x86/boot/compressed/misc.h +++ b/arch/x86/boot/compressed/misc.h @@ -132,7 +132,17 @@ void snp_set_page_private(unsigned long paddr); void snp_set_page_shared(unsigned long paddr); void sev_prep_identity_maps(unsigned long top_level_pgt); #else -static inline void sev_enable(struct boot_params *bp) { } +static inline void sev_enable(struct boot_params *bp) +{ + /* + * bp->cc_blob_address should only be set by boot/compressed kernel. + * Initialize it to 0 unconditionally (thus here in this stub too) to + * ensure that uninitialized values from buggy bootloaders aren't + * propagated. + */ + if (bp) + bp->cc_blob_address = 0; +} static inline void sev_es_shutdown_ghcb(void) { } static inline bool sev_es_check_ghcb_fault(unsigned long address) { diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c index 52f989f6acc2..c93930d5ccbd 100644 --- a/arch/x86/boot/compressed/sev.c +++ b/arch/x86/boot/compressed/sev.c @@ -276,6 +276,14 @@ void sev_enable(struct boot_params *bp) struct msr m; bool snp;

+ /* + * bp->cc_blob_address should only be set by boot/compressed kernel. + * Initialize it to 0 to ensure that uninitialized values from + * buggy bootloaders aren't propagated. + */ + if (bp) + bp->cc_blob_address = 0; + /* * Setup/preliminary detection of SNP. This will be sanity-checked * against CPUID/MSR values later.