linux-kernel - Subject: [PATCH] x86/retpoline: Document some thunk handling aspects (was: Re: [PATCH 0/3] Ensure default return thunk isn't used at runtime)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231020112843.GEZTJkax6LRw+x8qZj@fat_crate.local>
Date:   Fri, 20 Oct 2023 13:28:43 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     David Kaplan <david.kaplan@....com>,
        Josh Poimboeuf <jpoimboe@...nel.org>
Cc:     x86@...nel.org, luto@...nel.org, linux-kernel@...r.kernel.org
Subject: Subject: [PATCH] x86/retpoline: Document some thunk handling aspects
 (was: Re: [PATCH 0/3] Ensure default return thunk isn't used at runtime)

On Tue, Oct 10, 2023 at 12:10:17PM -0500, David Kaplan wrote:
> Several CPU side-channel mitigations require the use of a special return thunk.
> The necessary return thunk is installed at runtime via apply_returns(), after
> which point the default return thunk (__x86_return_thunk) should never be used.

Ok, mingo was right when suggesting that reverting those commits is not
really the right thing to do because it would break bisection if the
bisection point lands before the reverts. Yeah, yeah, it is unlikely but
better safe than sorry. So I went and rebased the whole tip:x86/bugs
branch into a clean state.

I've left, I hope enough, breadcrumbs in there for future improvements
in the form of the following patch:

---
From: "Borislav Petkov (AMD)" <bp@...en8.de>
Date: Fri, 20 Oct 2023 13:17:14 +0200
Subject: [PATCH] x86/retpoline: Document some thunk handling aspects

After a lot of experimenting (see thread Link points to) document for
now the issues and requirements for future improvements to the thunk
handling and potential issuing of a diagnostic when the default thunk
hasn't been patched out.

This documentation is only temporary and that close before the merge
window it is only a placeholder for those future improvements.

Suggested-by: Ingo Molnar <mingo@...nel.org>
Signed-off-by: Borislav Petkov (AMD) <bp@...en8.de>
Link: https://lore.kernel.org/r/20231010171020.462211-1-david.kaplan@amd.com
---
 arch/x86/lib/retpoline.S | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/x86/lib/retpoline.S b/arch/x86/lib/retpoline.S
index d410abacbf88..a48077c5ca61 100644
--- a/arch/x86/lib/retpoline.S
+++ b/arch/x86/lib/retpoline.S
@@ -129,6 +129,13 @@ SYM_CODE_END(__x86_indirect_jump_thunk_array)
 
 #ifdef CONFIG_RETHUNK
 
+/*
+ * Be careful here: that label cannot really be removed because in
+ * some configurations and toolchains, the JMP __x86_return_thunk the
+ * compiler issues is either a short one or the compiler doesn't use
+ * relocations for same-section JMPs and that breaks the returns
+ * detection logic in apply_returns() and in objtool.
+ */
 	.section .text..__x86.return_thunk
 
 #ifdef CONFIG_CPU_SRSO
@@ -361,6 +368,14 @@ SYM_FUNC_END(call_depth_return_thunk)
  * 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 should be converted eventually to call a warning function which
+ * should scream loudly when the default return thunk is called after
+ * alternatives have been applied.
+ *
+ * That warning function cannot BUG() because the bug splat cannot be
+ * displayed in all possible configurations, leading to users not really
+ * knowing why the machine froze.
  */
 SYM_CODE_START(__x86_return_thunk)
 	UNWIND_HINT_FUNC
-- 
2.42.0.rc0.25.ga82fb66fed25

-- 
Regards/Gruss,
    Boris.

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