Skip to content

Commit d43490d

Browse files
Peter Zijlstrabp3tk0v
authored andcommitted
x86/cpu: Clean up SRSO return thunk mess
Use the existing configurable return thunk. There is absolute no justification for having created this __x86_return_thunk alternative. To clarify, the whole thing looks like: Zen3/4 does: srso_alias_untrain_ret: nop2 lfence jmp srso_alias_return_thunk int3 srso_alias_safe_ret: // aliasses srso_alias_untrain_ret just so add $8, %rsp ret int3 srso_alias_return_thunk: call srso_alias_safe_ret ud2 While Zen1/2 does: srso_untrain_ret: movabs $foo, %rax lfence call srso_safe_ret (jmp srso_return_thunk ?) int3 srso_safe_ret: // embedded in movabs instruction add $8,%rsp ret int3 srso_return_thunk: call srso_safe_ret ud2 While retbleed does: zen_untrain_ret: test $0xcc, %bl lfence jmp zen_return_thunk int3 zen_return_thunk: // embedded in the test instruction ret int3 Where Zen1/2 flush the BTB entry using the instruction decoder trick (test,movabs) Zen3/4 use BTB aliasing. SRSO adds a return sequence (srso_safe_ret()) which forces the function return instruction to speculate into a trap (UD2). This RET will then mispredict and execution will continue at the return site read from the top of the stack. Pick one of three options at boot (evey function can only ever return once). [ bp: Fixup commit message uarch details and add them in a comment in the code too. Add a comment about the srso_select_mitigation() dependency on retbleed_select_mitigation(). Add moar ifdeffery for 32-bit builds. Add a dummy srso_untrain_ret_alias() definition for 32-bit alternatives needing the symbol. ] Fixes: fb3bd91 ("x86/srso: Add a Speculative RAS Overflow mitigation") Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de> Link: https://lore.kernel.org/r/20230814121148.842775684@infradead.org
1 parent 095b830 commit d43490d

File tree

5 files changed

+62
-20
lines changed

5 files changed

+62
-20
lines changed

arch/x86/include/asm/nospec-branch.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,9 +347,14 @@ extern void __x86_return_thunk(void);
347347
static inline void __x86_return_thunk(void) {}
348348
#endif
349349

350+
extern void zen_return_thunk(void);
351+
extern void srso_return_thunk(void);
352+
extern void srso_alias_return_thunk(void);
353+
350354
extern void zen_untrain_ret(void);
351355
extern void srso_untrain_ret(void);
352356
extern void srso_untrain_ret_alias(void);
357+
353358
extern void entry_ibpb(void);
354359

355360
extern void (*x86_return_thunk)(void);

arch/x86/kernel/cpu/bugs.c

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,11 @@ void __init cpu_select_mitigations(void)
167167
md_clear_select_mitigation();
168168
srbds_select_mitigation();
169169
l1d_flush_select_mitigation();
170+
171+
/*
172+
* srso_select_mitigation() depends and must run after
173+
* retbleed_select_mitigation().
174+
*/
170175
srso_select_mitigation();
171176
gds_select_mitigation();
172177
}
@@ -1037,6 +1042,9 @@ static void __init retbleed_select_mitigation(void)
10371042
setup_force_cpu_cap(X86_FEATURE_RETHUNK);
10381043
setup_force_cpu_cap(X86_FEATURE_UNRET);
10391044

1045+
if (IS_ENABLED(CONFIG_RETHUNK))
1046+
x86_return_thunk = zen_return_thunk;
1047+
10401048
if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD &&
10411049
boot_cpu_data.x86_vendor != X86_VENDOR_HYGON)
10421050
pr_err(RETBLEED_UNTRAIN_MSG);
@@ -2453,10 +2461,13 @@ static void __init srso_select_mitigation(void)
24532461
*/
24542462
setup_force_cpu_cap(X86_FEATURE_RETHUNK);
24552463

2456-
if (boot_cpu_data.x86 == 0x19)
2464+
if (boot_cpu_data.x86 == 0x19) {
24572465
setup_force_cpu_cap(X86_FEATURE_SRSO_ALIAS);
2458-
else
2466+
x86_return_thunk = srso_alias_return_thunk;
2467+
} else {
24592468
setup_force_cpu_cap(X86_FEATURE_SRSO);
2469+
x86_return_thunk = srso_return_thunk;
2470+
}
24602471
srso_mitigation = SRSO_MITIGATION_SAFE_RET;
24612472
} else {
24622473
pr_err("WARNING: kernel not compiled with CPU_SRSO.\n");

arch/x86/kernel/vmlinux.lds.S

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,7 @@ INIT_PER_CPU(irq_stack_backing_store);
521521
#endif
522522

523523
#ifdef CONFIG_RETHUNK
524-
. = ASSERT((__ret & 0x3f) == 0, "__ret not cacheline-aligned");
524+
. = ASSERT((zen_return_thunk & 0x3f) == 0, "zen_return_thunk not cacheline-aligned");
525525
. = ASSERT((srso_safe_ret & 0x3f) == 0, "srso_safe_ret not cacheline-aligned");
526526
#endif
527527

arch/x86/lib/retpoline.S

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -151,57 +151,69 @@ SYM_CODE_END(__x86_indirect_jump_thunk_array)
151151
.section .text..__x86.rethunk_untrain
152152

153153
SYM_START(srso_untrain_ret_alias, SYM_L_GLOBAL, SYM_A_NONE)
154+
UNWIND_HINT_FUNC
154155
ANNOTATE_NOENDBR
155156
ASM_NOP2
156157
lfence
157-
jmp __x86_return_thunk
158+
jmp srso_alias_return_thunk
158159
SYM_FUNC_END(srso_untrain_ret_alias)
159160
__EXPORT_THUNK(srso_untrain_ret_alias)
160161

161162
.section .text..__x86.rethunk_safe
163+
#else
164+
/* dummy definition for alternatives */
165+
SYM_START(srso_untrain_ret_alias, SYM_L_GLOBAL, SYM_A_NONE)
166+
ANNOTATE_UNRET_SAFE
167+
ret
168+
int3
169+
SYM_FUNC_END(srso_untrain_ret_alias)
162170
#endif
163171

164-
/* Needs a definition for the __x86_return_thunk alternative below. */
165172
SYM_START(srso_safe_ret_alias, SYM_L_GLOBAL, SYM_A_NONE)
166-
#ifdef CONFIG_CPU_SRSO
167173
lea 8(%_ASM_SP), %_ASM_SP
168174
UNWIND_HINT_FUNC
169-
#endif
170175
ANNOTATE_UNRET_SAFE
171176
ret
172177
int3
173178
SYM_FUNC_END(srso_safe_ret_alias)
174179

175180
.section .text..__x86.return_thunk
176181

182+
SYM_CODE_START(srso_alias_return_thunk)
183+
UNWIND_HINT_FUNC
184+
ANNOTATE_NOENDBR
185+
call srso_safe_ret_alias
186+
ud2
187+
SYM_CODE_END(srso_alias_return_thunk)
188+
177189
/*
178190
* Safety details here pertain to the AMD Zen{1,2} microarchitecture:
179-
* 1) The RET at __x86_return_thunk must be on a 64 byte boundary, for
191+
* 1) The RET at zen_return_thunk must be on a 64 byte boundary, for
180192
* alignment within the BTB.
181193
* 2) The instruction at zen_untrain_ret must contain, and not
182194
* end with, the 0xc3 byte of the RET.
183195
* 3) STIBP must be enabled, or SMT disabled, to prevent the sibling thread
184196
* from re-poisioning the BTB prediction.
185197
*/
186198
.align 64
187-
.skip 64 - (__ret - zen_untrain_ret), 0xcc
199+
.skip 64 - (zen_return_thunk - zen_untrain_ret), 0xcc
188200
SYM_START(zen_untrain_ret, SYM_L_GLOBAL, SYM_A_NONE)
189201
ANNOTATE_NOENDBR
190202
/*
191203
* As executed from zen_untrain_ret, this is:
192204
*
193205
* TEST $0xcc, %bl
194206
* LFENCE
195-
* JMP __x86_return_thunk
207+
* JMP zen_return_thunk
196208
*
197209
* Executing the TEST instruction has a side effect of evicting any BTB
198210
* prediction (potentially attacker controlled) attached to the RET, as
199-
* __x86_return_thunk + 1 isn't an instruction boundary at the moment.
211+
* zen_return_thunk + 1 isn't an instruction boundary at the moment.
200212
*/
201213
.byte 0xf6
202214

203215
/*
204-
* As executed from __x86_return_thunk, this is a plain RET.
216+
* As executed from zen_return_thunk, this is a plain RET.
205217
*
206218
* As part of the TEST above, RET is the ModRM byte, and INT3 the imm8.
207219
*
@@ -213,13 +225,13 @@ SYM_START(zen_untrain_ret, SYM_L_GLOBAL, SYM_A_NONE)
213225
* With SMT enabled and STIBP active, a sibling thread cannot poison
214226
* RET's prediction to a type of its choice, but can evict the
215227
* prediction due to competitive sharing. If the prediction is
216-
* evicted, __x86_return_thunk will suffer Straight Line Speculation
228+
* evicted, zen_return_thunk will suffer Straight Line Speculation
217229
* which will be contained safely by the INT3.
218230
*/
219-
SYM_INNER_LABEL(__ret, SYM_L_GLOBAL)
231+
SYM_INNER_LABEL(zen_return_thunk, SYM_L_GLOBAL)
220232
ret
221233
int3
222-
SYM_CODE_END(__ret)
234+
SYM_CODE_END(zen_return_thunk)
223235

224236
/*
225237
* Ensure the TEST decoding / BTB invalidation is complete.
@@ -230,7 +242,7 @@ SYM_CODE_END(__ret)
230242
* Jump back and execute the RET in the middle of the TEST instruction.
231243
* INT3 is for SLS protection.
232244
*/
233-
jmp __ret
245+
jmp zen_return_thunk
234246
int3
235247
SYM_FUNC_END(zen_untrain_ret)
236248
__EXPORT_THUNK(zen_untrain_ret)
@@ -251,24 +263,38 @@ SYM_START(srso_untrain_ret, SYM_L_GLOBAL, SYM_A_NONE)
251263
ANNOTATE_NOENDBR
252264
.byte 0x48, 0xb8
253265

266+
/*
267+
* This forces the function return instruction to speculate into a trap
268+
* (UD2 in srso_return_thunk() below). This RET will then mispredict
269+
* and execution will continue at the return site read from the top of
270+
* the stack.
271+
*/
254272
SYM_INNER_LABEL(srso_safe_ret, SYM_L_GLOBAL)
255273
lea 8(%_ASM_SP), %_ASM_SP
256274
ret
257275
int3
258276
int3
277+
/* end of movabs */
259278
lfence
260279
call srso_safe_ret
261280
ud2
262281
SYM_CODE_END(srso_safe_ret)
263282
SYM_FUNC_END(srso_untrain_ret)
264283
__EXPORT_THUNK(srso_untrain_ret)
265284

266-
SYM_CODE_START(__x86_return_thunk)
285+
SYM_CODE_START(srso_return_thunk)
267286
UNWIND_HINT_FUNC
268287
ANNOTATE_NOENDBR
269-
ALTERNATIVE_2 "jmp __ret", "call srso_safe_ret", X86_FEATURE_SRSO, \
270-
"call srso_safe_ret_alias", X86_FEATURE_SRSO_ALIAS
288+
call srso_safe_ret
271289
ud2
290+
SYM_CODE_END(srso_return_thunk)
291+
292+
SYM_CODE_START(__x86_return_thunk)
293+
UNWIND_HINT_FUNC
294+
ANNOTATE_NOENDBR
295+
ANNOTATE_UNRET_SAFE
296+
ret
297+
int3
272298
SYM_CODE_END(__x86_return_thunk)
273299
EXPORT_SYMBOL(__x86_return_thunk)
274300

tools/objtool/arch/x86/decode.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,6 @@ bool arch_is_rethunk(struct symbol *sym)
829829

830830
bool arch_is_embedded_insn(struct symbol *sym)
831831
{
832-
return !strcmp(sym->name, "__ret") ||
832+
return !strcmp(sym->name, "zen_return_thunk") ||
833833
!strcmp(sym->name, "srso_safe_ret");
834834
}

0 commit comments

Comments
 (0)