From: David Kaplan <[email protected]>
The call to cc_platform_has() triggers a fault and system crash if call depth
tracking is active because the GS segment has been reset by load_segments() and
GS_BASE is now 0 but call depth tracking uses per-CPU variables to operate.
Call cc_platform_has() earlier in the function when GS is still valid.
[ bp: Massage. ]
Signed-off-by: David Kaplan <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
---
arch/x86/kernel/machine_kexec_64.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index b180d8e497c3..90b1946c06e5 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -295,8 +295,15 @@ void machine_kexec_cleanup(struct kimage *image)
void machine_kexec(struct kimage *image)
{
unsigned long page_list[PAGES_NR];
- void *control_page;
+ unsigned int host_mem_enc_active;
int save_ftrace_enabled;
+ void *control_page;
+
+ /*
+ * This must be done before load_segments() since if call depth tracking
+ * is used then GS must be valid to make any function calls.
+ */
+ host_mem_enc_active = cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT);
#ifdef CONFIG_KEXEC_JUMP
if (image->preserve_context)
@@ -358,7 +365,7 @@ void machine_kexec(struct kimage *image)
(unsigned long)page_list,
image->start,
image->preserve_context,
- cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT));
+ host_mem_enc_active);
#ifdef CONFIG_KEXEC_JUMP
if (image->preserve_context)
--
2.43.0
On 6/3/24 03:30, Borislav Petkov wrote:
> From: David Kaplan <[email protected]>
>
> The call to cc_platform_has() triggers a fault and system crash if call depth
> tracking is active because the GS segment has been reset by load_segments() and
> GS_BASE is now 0 but call depth tracking uses per-CPU variables to operate.
>
> Call cc_platform_has() earlier in the function when GS is still valid.
>
> [ bp: Massage. ]
>
> Signed-off-by: David Kaplan <[email protected]>
> Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Minor nit below, otherwise:
Reviewed-by: Tom Lendacky <[email protected]>
Does it need a Fixes: tag for call depth tracking (before the change to
cc_platform_has() it was a call to sme_active())?
> ---
> arch/x86/kernel/machine_kexec_64.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> index b180d8e497c3..90b1946c06e5 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -295,8 +295,15 @@ void machine_kexec_cleanup(struct kimage *image)
> void machine_kexec(struct kimage *image)
> {
> unsigned long page_list[PAGES_NR];
> - void *control_page;
> + unsigned int host_mem_enc_active;
Looks like spaces used here instead of tabs.
Thanks,
Tom
> int save_ftrace_enabled;
> + void *control_page;
> +
> + /*
> + * This must be done before load_segments() since if call depth tracking
> + * is used then GS must be valid to make any function calls.
> + */
> + host_mem_enc_active = cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT);
>
> #ifdef CONFIG_KEXEC_JUMP
> if (image->preserve_context)
> @@ -358,7 +365,7 @@ void machine_kexec(struct kimage *image)
> (unsigned long)page_list,
> image->start,
> image->preserve_context,
> - cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT));
> + host_mem_enc_active);
>
> #ifdef CONFIG_KEXEC_JUMP
> if (image->preserve_context)
On Mon, Jun 03, 2024 at 08:30:26AM -0500, Tom Lendacky wrote:
> Does it need a Fixes: tag for call depth tracking (before the change to
> cc_platform_has() it was a call to sme_active())?
Yeah, something like
Fixes: 5d8213864ade ("x86/retbleed: Add SKL return thunk")
I guess.
> Looks like spaces used here instead of tabs.
I was fixing those and forgot to refresh before sending - it is fixed
here locally.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: 93c1800b3799f17375989b0daf76497dd3e80922
Gitweb: https://git.kernel.org/tip/93c1800b3799f17375989b0daf76497dd3e80922
Author: David Kaplan <[email protected]>
AuthorDate: Sun, 02 Jun 2024 13:19:09 -05:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Mon, 03 Jun 2024 17:19:03 +02:00
x86/kexec: Fix bug with call depth tracking
The call to cc_platform_has() triggers a fault and system crash if call depth
tracking is active because the GS segment has been reset by load_segments() and
GS_BASE is now 0 but call depth tracking uses per-CPU variables to operate.
Call cc_platform_has() earlier in the function when GS is still valid.
[ bp: Massage. ]
Fixes: 5d8213864ade ("x86/retbleed: Add SKL return thunk")
Signed-off-by: David Kaplan <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Reviewed-by: Tom Lendacky <[email protected]>
Cc: <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/machine_kexec_64.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index b180d8e..cc0f7f7 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -295,8 +295,15 @@ void machine_kexec_cleanup(struct kimage *image)
void machine_kexec(struct kimage *image)
{
unsigned long page_list[PAGES_NR];
- void *control_page;
+ unsigned int host_mem_enc_active;
int save_ftrace_enabled;
+ void *control_page;
+
+ /*
+ * This must be done before load_segments() since if call depth tracking
+ * is used then GS must be valid to make any function calls.
+ */
+ host_mem_enc_active = cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT);
#ifdef CONFIG_KEXEC_JUMP
if (image->preserve_context)
@@ -358,7 +365,7 @@ void machine_kexec(struct kimage *image)
(unsigned long)page_list,
image->start,
image->preserve_context,
- cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT));
+ host_mem_enc_active);
#ifdef CONFIG_KEXEC_JUMP
if (image->preserve_context)