2023-10-10 17:11:26

by Kaplan, David

[permalink] [raw]
Subject: [PATCH 0/3] Ensure default return thunk isn't used at runtime

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.

Patch 3 enforces this by modifying __x86_return_thunk to be a ud2 after
alternatives are applied.

Patch 1 reverts a recent commit which resulted in retpoline sequences not being
annotated as containing returns, which was leaving them using the default return
thunk.

Patch 2 fixes an issue where functions in vdso32-setup were using the default
return thunk because objtool was not being run on them in some cases.

David Kaplan (3):
Revert "x86/retpoline: Remove .text..__x86.return_thunk section"
x86/vdso: Run objtool on vdso32-setup
x86/retpoline: Ensure default return thunk isn't used at runtime

arch/x86/entry/vdso/Makefile | 3 ++-
arch/x86/kernel/vmlinux.lds.S | 3 +++
arch/x86/lib/retpoline.S | 10 +++++++---
3 files changed, 12 insertions(+), 4 deletions(-)

--
2.25.1


2023-10-10 17:11:28

by Kaplan, David

[permalink] [raw]
Subject: [PATCH 2/3] x86/vdso: Run objtool on vdso32-setup

vdso32-setup.c is part of the main kernel image and should not be
excluded from objtool. Objtool is necessary in part for ensuring that
returns in this file are correctly patched to the appropriate return
thunk at runtime.

Signed-off-by: David Kaplan <[email protected]>
---
arch/x86/entry/vdso/Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index 6a1821bd7d5e..83c0afb7c741 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -42,7 +42,8 @@ vdso_img-$(VDSO64-y) += 64
vdso_img-$(VDSOX32-y) += x32
vdso_img-$(VDSO32-y) += 32

-obj-$(VDSO32-y) += vdso32-setup.o
+obj-$(VDSO32-y) += vdso32-setup.o
+OBJECT_FILES_NON_STANDARD_vdso32-setup.o := n

vobjs := $(foreach F,$(vobjs-y),$(obj)/$F)
vobjs32 := $(foreach F,$(vobjs32-y),$(obj)/$F)
--
2.25.1

2023-10-10 17:52:24

by Peter Zijlstra

[permalink] [raw]
Subject: 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:

> David Kaplan (3):
> Revert "x86/retpoline: Remove .text..__x86.return_thunk section"
> x86/vdso: Run objtool on vdso32-setup
> x86/retpoline: Ensure default return thunk isn't used at runtime
>
> arch/x86/entry/vdso/Makefile | 3 ++-
> arch/x86/kernel/vmlinux.lds.S | 3 +++
> arch/x86/lib/retpoline.S | 10 +++++++---
> 3 files changed, 12 insertions(+), 4 deletions(-)

Acked-by: Peter Zijlstra (Intel) <[email protected]>

That said, I'm afraid we might have broken rethunk for i386 somewhere
along the SRSO series.

I suspect the easiest fix is to make CONFIG_RETHUNK hard depend on
x86_64 or something.

Subject: [tip: x86/bugs] x86/vdso: Run objtool on vdso32-setup.o

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

Commit-ID: 99b5bf0276d4ae5028ab9743053c6d16044009ea
Gitweb: https://git.kernel.org/tip/99b5bf0276d4ae5028ab9743053c6d16044009ea
Author: David Kaplan <[email protected]>
AuthorDate: Tue, 10 Oct 2023 12:10:19 -05:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Thu, 12 Oct 2023 19:44:07 +02:00

x86/vdso: Run objtool on vdso32-setup.o

vdso32-setup.c is part of the main kernel image and should not be
excluded from objtool. Objtool is necessary in part for ensuring that
returns in this file are correctly patched to the appropriate return
thunk at runtime.

Signed-off-by: David Kaplan <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Acked-by: Borislav Petkov (AMD) <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/entry/vdso/Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index 6a1821b..83c0afb 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -42,7 +42,8 @@ vdso_img-$(VDSO64-y) += 64
vdso_img-$(VDSOX32-y) += x32
vdso_img-$(VDSO32-y) += 32

-obj-$(VDSO32-y) += vdso32-setup.o
+obj-$(VDSO32-y) += vdso32-setup.o
+OBJECT_FILES_NON_STANDARD_vdso32-setup.o := n

vobjs := $(foreach F,$(vobjs-y),$(obj)/$F)
vobjs32 := $(foreach F,$(vobjs32-y),$(obj)/$F)

2023-10-20 11:29:15

by Borislav Petkov

[permalink] [raw]
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)" <[email protected]>
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 <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
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

Subject: [tip: x86/bugs] x86/retpoline: Document some thunk handling aspects

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

Commit-ID: 9d9c22cc444af01ce254872b729af26864c43a3a
Gitweb: https://git.kernel.org/tip/9d9c22cc444af01ce254872b729af26864c43a3a
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Fri, 20 Oct 2023 13:17:14 +02:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Fri, 20 Oct 2023 13:17:14 +02:00

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 <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
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 d410aba..a48077c 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

Subject: [tip: x86/bugs] x86/vdso: Run objtool on vdso32-setup.o

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

Commit-ID: b587fef124f98f3ab1322dba8e37cdff660acd8c
Gitweb: https://git.kernel.org/tip/b587fef124f98f3ab1322dba8e37cdff660acd8c
Author: David Kaplan <[email protected]>
AuthorDate: Tue, 10 Oct 2023 12:10:19 -05:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Fri, 20 Oct 2023 12:58:27 +02:00

x86/vdso: Run objtool on vdso32-setup.o

vdso32-setup.c is part of the main kernel image and should not be
excluded from objtool. Objtool is necessary in part for ensuring that
returns in this file are correctly patched to the appropriate return
thunk at runtime.

Signed-off-by: David Kaplan <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Acked-by: Borislav Petkov (AMD) <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/entry/vdso/Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index 6a1821b..83c0afb 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -42,7 +42,8 @@ vdso_img-$(VDSO64-y) += 64
vdso_img-$(VDSOX32-y) += x32
vdso_img-$(VDSO32-y) += 32

-obj-$(VDSO32-y) += vdso32-setup.o
+obj-$(VDSO32-y) += vdso32-setup.o
+OBJECT_FILES_NON_STANDARD_vdso32-setup.o := n

vobjs := $(foreach F,$(vobjs-y),$(obj)/$F)
vobjs32 := $(foreach F,$(vobjs32-y),$(obj)/$F)