linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/boot: Don't propagate uninitialized boot_params->cc_blob_address
@ 2022-08-23 16:07 Michael Roth
  2022-08-23 17:11 ` Borislav Petkov
  2022-08-24  7:55 ` [tip: x86/urgent] " tip-bot2 for Michael Roth
  0 siblings, 2 replies; 3+ messages in thread
From: Michael Roth @ 2022-08-23 16:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: stable, x86, watnuss, Jeremi Piotrowski, Tom Lendacky, Borislav Petkov

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(-)

diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 4910bf230d7b..aa7889751abc 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -132,7 +132,16 @@ 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 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.
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] x86/boot: Don't propagate uninitialized boot_params->cc_blob_address
  2022-08-23 16:07 [PATCH] x86/boot: Don't propagate uninitialized boot_params->cc_blob_address Michael Roth
@ 2022-08-23 17:11 ` Borislav Petkov
  2022-08-24  7:55 ` [tip: x86/urgent] " tip-bot2 for Michael Roth
  1 sibling, 0 replies; 3+ messages in thread
From: Borislav Petkov @ 2022-08-23 17:11 UTC (permalink / raw)
  To: Michael Roth
  Cc: linux-kernel, stable, x86, watnuss, Jeremi Piotrowski, Tom Lendacky

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

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* [tip: x86/urgent] x86/boot: Don't propagate uninitialized boot_params->cc_blob_address
  2022-08-23 16:07 [PATCH] x86/boot: Don't propagate uninitialized boot_params->cc_blob_address Michael Roth
  2022-08-23 17:11 ` Borislav Petkov
@ 2022-08-24  7:55 ` tip-bot2 for Michael Roth
  1 sibling, 0 replies; 3+ messages in thread
From: tip-bot2 for Michael Roth @ 2022-08-24  7:55 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Jeremi Piotrowski, watnuss, Michael Roth, Borislav Petkov,
	stable, x86, linux-kernel

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     4b1c742407571eff58b6de9881889f7ca7c4b4dc
Gitweb:        https://git.kernel.org/tip/4b1c742407571eff58b6de9881889f7ca7c4b4dc
Author:        Michael Roth <michael.roth@amd.com>
AuthorDate:    Tue, 23 Aug 2022 11:07:34 -05:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Wed, 24 Aug 2022 09:03:04 +02:00

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 4910bf2..62208ec 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 52f989f..c93930d 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -277,6 +277,14 @@ void sev_enable(struct boot_params *bp)
 	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.
 	 */

^ permalink raw reply related	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-24  7:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-23 16:07 [PATCH] x86/boot: Don't propagate uninitialized boot_params->cc_blob_address Michael Roth
2022-08-23 17:11 ` Borislav Petkov
2022-08-24  7:55 ` [tip: x86/urgent] " tip-bot2 for Michael Roth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).