[tip: x86/bugs] Revert "x86/retpoline: Ensure default return thunk isn't used at runtime"

tip-bot2 for Borislav Petkov (AMD) posted 1 patch 2 years, 1 month ago
arch/x86/lib/retpoline.S | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
[tip: x86/bugs] Revert "x86/retpoline: Ensure default return thunk isn't used at runtime"
Posted by tip-bot2 for Borislav Petkov (AMD) 2 years, 1 month ago
The following commit has been merged into the x86/bugs branch of tip:

Commit-ID:     08ec7e82c1e3ebcd79ab8d2d0d11faad0f07e71c
Gitweb:        https://git.kernel.org/tip/08ec7e82c1e3ebcd79ab8d2d0d11faad0f07e71c
Author:        Borislav Petkov (AMD) <bp@alien8.de>
AuthorDate:    Thu, 19 Oct 2023 11:04:27 +02:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Thu, 19 Oct 2023 11:08:22 +02:00

Revert "x86/retpoline: Ensure default return thunk isn't used at runtime"

This reverts commit 91174087dcc7565d8bf0d576544e42d5b1de6f39.

It turns out that raising an undefined opcode exception due to unpatched
return thunks is not visible to users in every possible scenario (not
being able to catch dmesg, slow console, etc.).

Thus, it is not very friendly to them when the box explodes without even
saying why.

Revert for now until a better solution has been devised.

Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Cc: David Kaplan <david.kaplan@amd.com>
Link: https://lore.kernel.org/r/20231018175531.GEZTAcE2p92U1AuVp1@fat_crate.local
---
 arch/x86/lib/retpoline.S | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/x86/lib/retpoline.S b/arch/x86/lib/retpoline.S
index fe05c13..6376d01 100644
--- a/arch/x86/lib/retpoline.S
+++ b/arch/x86/lib/retpoline.S
@@ -356,17 +356,15 @@ SYM_FUNC_END(call_depth_return_thunk)
  * This function name is magical and is used by -mfunction-return=thunk-extern
  * for the compiler to generate JMPs to it.
  *
- * This code is only used during kernel boot.  All
+ * This code is only used during kernel boot or module init.  All
  * 'JMP __x86_return_thunk' sites are changed to something else by
  * apply_returns().
- *
- * This thunk is turned into a ud2 to ensure it is never used at runtime.
- * Alternative instructions are applied after apply_returns().
  */
 SYM_CODE_START(__x86_return_thunk)
 	UNWIND_HINT_FUNC
 	ANNOTATE_NOENDBR
-	ALTERNATIVE __stringify(ANNOTATE_UNRET_SAFE;ret),"ud2", X86_FEATURE_ALWAYS
+	ANNOTATE_UNRET_SAFE
+	ret
 	int3
 SYM_CODE_END(__x86_return_thunk)
 EXPORT_SYMBOL(__x86_return_thunk)
Re: [tip: x86/bugs] Revert "x86/retpoline: Ensure default return thunk isn't used at runtime"
Posted by Peter Zijlstra 1 year, 2 months ago
On Thu, Oct 19, 2023 at 09:40:43AM -0000, tip-bot2 for Borislav Petkov (AMD) wrote:
> The following commit has been merged into the x86/bugs branch of tip:
> 
> Commit-ID:     08ec7e82c1e3ebcd79ab8d2d0d11faad0f07e71c
> Gitweb:        https://git.kernel.org/tip/08ec7e82c1e3ebcd79ab8d2d0d11faad0f07e71c
> Author:        Borislav Petkov (AMD) <bp@alien8.de>
> AuthorDate:    Thu, 19 Oct 2023 11:04:27 +02:00
> Committer:     Borislav Petkov (AMD) <bp@alien8.de>
> CommitterDate: Thu, 19 Oct 2023 11:08:22 +02:00
> 
> Revert "x86/retpoline: Ensure default return thunk isn't used at runtime"
> 
> This reverts commit 91174087dcc7565d8bf0d576544e42d5b1de6f39.
> 
> It turns out that raising an undefined opcode exception due to unpatched
> return thunks is not visible to users in every possible scenario (not
> being able to catch dmesg, slow console, etc.).
> 
> Thus, it is not very friendly to them when the box explodes without even
> saying why.

This is what we have __bug_table for...

Turns out asm/bug.h doesn't currently have nice helpers for __ASSMEBLY__
so I botched it a bit...

---
diff --git a/arch/x86/entry/entry.S b/arch/x86/entry/entry.S
index d9feadffa972..003379049924 100644
--- a/arch/x86/entry/entry.S
+++ b/arch/x86/entry/entry.S
@@ -10,8 +10,6 @@
 #include <asm/segment.h>
 #include <asm/cache.h>
 
-#include "calling.h"
-
 .pushsection .noinstr.text, "ax"
 
 SYM_FUNC_START(entry_ibpb)
@@ -45,4 +43,3 @@ EXPORT_SYMBOL_GPL(mds_verw_sel);
 
 .popsection
 
-THUNK warn_thunk_thunk, __warn_thunk
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index ff5f1ecc7d1e..547cde3db276 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -387,8 +387,6 @@ extern void clear_bhb_loop(void);
 
 extern void (*x86_return_thunk)(void);
 
-extern void __warn_thunk(void);
-
 #ifdef CONFIG_MITIGATION_CALL_DEPTH_TRACKING
 extern void call_depth_return_thunk(void);
 
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index d1915427b4ff..b86048f31a0c 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -3025,8 +3025,3 @@ ssize_t cpu_show_reg_file_data_sampling(struct device *dev, struct device_attrib
 	return cpu_show_common(dev, attr, buf, X86_BUG_RFDS);
 }
 #endif
-
-void __warn_thunk(void)
-{
-	WARN_ONCE(1, "Unpatched return thunk in use. This should not happen!\n");
-}
diff --git a/arch/x86/lib/retpoline.S b/arch/x86/lib/retpoline.S
index 391059b2c6fb..469bf27287a1 100644
--- a/arch/x86/lib/retpoline.S
+++ b/arch/x86/lib/retpoline.S
@@ -12,9 +12,39 @@
 #include <asm/percpu.h>
 #include <asm/frame.h>
 #include <asm/nops.h>
+#include <linux/objtool.h>
 
-	.section .text..__x86.indirect_thunk
+// this should probably go in asm/bug.h
+
+#ifdef CONFIG_X86_32
+#define __BUG_REL(val)	.long	val
+#else
+#define __BUG_REL(val)	.long	val - .
+#endif
+
+#ifdef CONFIG_DEBUG_BUGVERBOSE
+#define __BUG_VERBOSE()						\
+	__BUG_REL(0) ;						\
+	.word 0 ;
+#else
+#define __BUG_VERBOSE()
+#endif
 
+#define _BUG_FLAGS(flags)					\
+	1: ;							\
+	.pushsection __bug_table, "aw" ;			\
+	2: __BUG_REL(1b) ;					\
+	__BUG_VERBOSE() ;					\
+	.word flags ;						\
+	.org 2b+(6+6*IS_ENABLED(CONFIG_DEBUG_BUGVERBOSE)) ;	\
+	.popsection
+
+#define WARN_ONCE						\
+	_BUG_FLAGS(3) ;						\
+	ALTERNATIVE "", "ud2", X86_FEATURE_ALWAYS ;		\
+	REACHABLE
+
+	.section .text..__x86.indirect_thunk
 
 .macro POLINE reg
 	ANNOTATE_INTRA_FUNCTION_CALL
@@ -37,9 +67,15 @@ SYM_INNER_LABEL(__x86_indirect_thunk_\reg, SYM_L_GLOBAL)
 	UNWIND_HINT_UNDEFINED
 	ANNOTATE_NOENDBR
 
+#ifdef CONFIG_X86_32
 	ALTERNATIVE_2 __stringify(RETPOLINE \reg), \
 		      __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *%\reg; int3), X86_FEATURE_RETPOLINE_LFENCE, \
 		      __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *%\reg), ALT_NOT(X86_FEATURE_RETPOLINE)
+#else
+	WARN_ONCE
+	ANNOTATE_RETPOLINE_SAFE
+	jmp *%\reg
+#endif
 
 .endm
 
@@ -382,16 +418,15 @@ SYM_FUNC_END(call_depth_return_thunk)
 SYM_CODE_START(__x86_return_thunk)
 	UNWIND_HINT_FUNC
 	ANNOTATE_NOENDBR
-#if defined(CONFIG_MITIGATION_UNRET_ENTRY) || \
-    defined(CONFIG_MITIGATION_SRSO) || \
-    defined(CONFIG_MITIGATION_CALL_DEPTH_TRACKING)
-	ALTERNATIVE __stringify(ANNOTATE_UNRET_SAFE; ret), \
-		   "jmp warn_thunk_thunk", X86_FEATURE_ALWAYS
-#else
+
+#ifdef CONFIG_X86_64
+	WARN_ONCE
+#endif
+
 	ANNOTATE_UNRET_SAFE
 	ret
-#endif
 	int3
+
 SYM_CODE_END(__x86_return_thunk)
 EXPORT_SYMBOL(__x86_return_thunk)