All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/mm: Fix SME encryption stack ptr handling
@ 2017-08-27 16:39 Borislav Petkov
  2017-08-29  9:01 ` [tip:x86/mm] " tip-bot for Borislav Petkov
  0 siblings, 1 reply; 2+ messages in thread
From: Borislav Petkov @ 2017-08-27 16:39 UTC (permalink / raw)
  To: X86 ML; +Cc: Tom Lendacky, LKML, Brijesh Singh

From: Borislav Petkov <bp@suse.de>

sme_encrypt_execute() stashes the stack pointer on entry into %rbp
because it allocates a one-page stack in the non-encrypted area for the
encryption routine to use. When the latter is done, it restores it from
%rbp again, before returning.

However, it uses the FRAME_* macros partially but restores %rsp from
%rbp explicitly with a MOV. And this is fine as long as the macros
*actually* do something.

Unless, you do a !CONFIG_FRAME_POINTER build where those macros
are empty. Then, we still restore %rsp from %rbp but %rbp contains
*something* and this leads to a stack corruption. The manifestation
being a triple-fault during early boot when testing SME. Good luck to me
debugging this with the clumsy endless-loop-in-asm method and narrowing
it down gradually. :-(

So, long story short, open-code the frame macros so that there's no
monkey business and we avoid subtly breaking SME depending on the
.config.

Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
---
 arch/x86/mm/mem_encrypt_boot.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/mm/mem_encrypt_boot.S b/arch/x86/mm/mem_encrypt_boot.S
index b327e0472448..730e6d541df1 100644
--- a/arch/x86/mm/mem_encrypt_boot.S
+++ b/arch/x86/mm/mem_encrypt_boot.S
@@ -15,7 +15,6 @@
 #include <asm/page.h>
 #include <asm/processor-flags.h>
 #include <asm/msr-index.h>
-#include <asm/frame.h>
 
 	.text
 	.code64
@@ -33,7 +32,8 @@ ENTRY(sme_encrypt_execute)
 	 *    R8 - physcial address of the pagetables to use for encryption
 	 */
 
-	FRAME_BEGIN			/* RBP now has original stack pointer */
+	push	%rbp
+	movq	%rsp, %rbp		/* RBP now has original stack pointer */
 
 	/* Set up a one page stack in the non-encrypted memory area */
 	movq	%rcx, %rax		/* Workarea stack page */
@@ -64,7 +64,7 @@ ENTRY(sme_encrypt_execute)
 	pop	%r12
 
 	movq	%rbp, %rsp		/* Restore original stack pointer */
-	FRAME_END
+	pop	%rbp
 
 	ret
 ENDPROC(sme_encrypt_execute)
-- 
2.13.0

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [tip:x86/mm] x86/mm: Fix SME encryption stack ptr handling
  2017-08-27 16:39 [PATCH] x86/mm: Fix SME encryption stack ptr handling Borislav Petkov
@ 2017-08-29  9:01 ` tip-bot for Borislav Petkov
  0 siblings, 0 replies; 2+ messages in thread
From: tip-bot for Borislav Petkov @ 2017-08-29  9:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, brijesh.singh, hpa, thomas.lendacky, bp, tglx, linux-kernel

Commit-ID:  6e0b52d406f64d2bd65731968a072387b91b44d2
Gitweb:     http://git.kernel.org/tip/6e0b52d406f64d2bd65731968a072387b91b44d2
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Sun, 27 Aug 2017 18:39:24 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 29 Aug 2017 10:57:16 +0200

x86/mm: Fix SME encryption stack ptr handling

sme_encrypt_execute() stashes the stack pointer on entry into %rbp
because it allocates a one-page stack in the non-encrypted area for the
encryption routine to use. When the latter is done, it restores it from
%rbp again, before returning.

However, it uses the FRAME_* macros partially but restores %rsp from
%rbp explicitly with a MOV. And this is fine as long as the macros
*actually* do something.

Unless, you do a !CONFIG_FRAME_POINTER build where those macros
are empty. Then, we still restore %rsp from %rbp but %rbp contains
*something* and this leads to a stack corruption. The manifestation
being a triple-fault during early boot when testing SME. Good luck to me
debugging this with the clumsy endless-loop-in-asm method and narrowing
it down gradually. :-(

So, long story short, open-code the frame macros so that there's no
monkey business and we avoid subtly breaking SME depending on the
.config.

Fixes: 6ebcb060713f ("x86/mm: Add support to encrypt the kernel in-place")
Signed-off-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Link: http://lkml.kernel.org/r/20170827163924.25552-1-bp@alien8.de

---
 arch/x86/mm/mem_encrypt_boot.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/mm/mem_encrypt_boot.S b/arch/x86/mm/mem_encrypt_boot.S
index b327e04..730e6d5 100644
--- a/arch/x86/mm/mem_encrypt_boot.S
+++ b/arch/x86/mm/mem_encrypt_boot.S
@@ -15,7 +15,6 @@
 #include <asm/page.h>
 #include <asm/processor-flags.h>
 #include <asm/msr-index.h>
-#include <asm/frame.h>
 
 	.text
 	.code64
@@ -33,7 +32,8 @@ ENTRY(sme_encrypt_execute)
 	 *    R8 - physcial address of the pagetables to use for encryption
 	 */
 
-	FRAME_BEGIN			/* RBP now has original stack pointer */
+	push	%rbp
+	movq	%rsp, %rbp		/* RBP now has original stack pointer */
 
 	/* Set up a one page stack in the non-encrypted memory area */
 	movq	%rcx, %rax		/* Workarea stack page */
@@ -64,7 +64,7 @@ ENTRY(sme_encrypt_execute)
 	pop	%r12
 
 	movq	%rbp, %rsp		/* Restore original stack pointer */
-	FRAME_END
+	pop	%rbp
 
 	ret
 ENDPROC(sme_encrypt_execute)

^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-08-29  9:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-27 16:39 [PATCH] x86/mm: Fix SME encryption stack ptr handling Borislav Petkov
2017-08-29  9:01 ` [tip:x86/mm] " tip-bot for Borislav Petkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.