Skip to content

Commit 833fd80

Browse files
petrpavlubp3tk0v
authored andcommitted
x86/retpoline,kprobes: Skip optprobe check for indirect jumps with retpolines and IBT
The kprobes optimization check can_optimize() calls insn_is_indirect_jump() to detect indirect jump instructions in a target function. If any is found, creating an optprobe is disallowed in the function because the jump could be from a jump table and could potentially land in the middle of the target optprobe. With retpolines, insn_is_indirect_jump() additionally looks for calls to indirect thunks which the compiler potentially used to replace original jumps. This extra check is however unnecessary because jump tables are disabled when the kernel is built with retpolines. The same is currently the case with IBT. Based on this observation, remove the logic to look for calls to indirect thunks and skip the check for indirect jumps altogether if the kernel is built with retpolines or IBT. Remove subsequently the symbols __indirect_thunk_start and __indirect_thunk_end which are no longer needed. Dropping this logic indirectly fixes a problem where the range [__indirect_thunk_start, __indirect_thunk_end] wrongly included also the return thunk. It caused that machines which used the return thunk as a mitigation and didn't have it patched by any alternative ended up not being able to use optprobes in any regular function. Fixes: 0b53c37 ("x86/retpoline: Use -mfunction-return") Suggested-by: Peter Zijlstra (Intel) <[email protected]> Suggested-by: Masami Hiramatsu (Google) <[email protected]> Signed-off-by: Petr Pavlu <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Signed-off-by: Borislav Petkov (AMD) <[email protected]> Acked-by: Masami Hiramatsu (Google) <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent 79cd2a1 commit 833fd80

File tree

4 files changed

+17
-32
lines changed

4 files changed

+17
-32
lines changed

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -478,9 +478,6 @@ enum ssb_mitigation {
478478
SPEC_STORE_BYPASS_SECCOMP,
479479
};
480480

481-
extern char __indirect_thunk_start[];
482-
extern char __indirect_thunk_end[];
483-
484481
static __always_inline
485482
void alternative_msr_write(unsigned int msr, u64 val, unsigned int feature)
486483
{

arch/x86/kernel/kprobes/opt.c

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ static int copy_optimized_instructions(u8 *dest, u8 *src, u8 *real)
226226
}
227227

228228
/* Check whether insn is indirect jump */
229-
static int __insn_is_indirect_jump(struct insn *insn)
229+
static int insn_is_indirect_jump(struct insn *insn)
230230
{
231231
return ((insn->opcode.bytes[0] == 0xff &&
232232
(X86_MODRM_REG(insn->modrm.value) & 6) == 4) || /* Jump */
@@ -260,26 +260,6 @@ static int insn_jump_into_range(struct insn *insn, unsigned long start, int len)
260260
return (start <= target && target <= start + len);
261261
}
262262

263-
static int insn_is_indirect_jump(struct insn *insn)
264-
{
265-
int ret = __insn_is_indirect_jump(insn);
266-
267-
#ifdef CONFIG_RETPOLINE
268-
/*
269-
* Jump to x86_indirect_thunk_* is treated as an indirect jump.
270-
* Note that even with CONFIG_RETPOLINE=y, the kernel compiled with
271-
* older gcc may use indirect jump. So we add this check instead of
272-
* replace indirect-jump check.
273-
*/
274-
if (!ret)
275-
ret = insn_jump_into_range(insn,
276-
(unsigned long)__indirect_thunk_start,
277-
(unsigned long)__indirect_thunk_end -
278-
(unsigned long)__indirect_thunk_start);
279-
#endif
280-
return ret;
281-
}
282-
283263
/* Decode whole function to ensure any instructions don't jump into target */
284264
static int can_optimize(unsigned long paddr)
285265
{
@@ -334,9 +314,21 @@ static int can_optimize(unsigned long paddr)
334314
/* Recover address */
335315
insn.kaddr = (void *)addr;
336316
insn.next_byte = (void *)(addr + insn.length);
337-
/* Check any instructions don't jump into target */
338-
if (insn_is_indirect_jump(&insn) ||
339-
insn_jump_into_range(&insn, paddr + INT3_INSN_SIZE,
317+
/*
318+
* Check any instructions don't jump into target, indirectly or
319+
* directly.
320+
*
321+
* The indirect case is present to handle a code with jump
322+
* tables. When the kernel uses retpolines, the check should in
323+
* theory additionally look for jumps to indirect thunks.
324+
* However, the kernel built with retpolines or IBT has jump
325+
* tables disabled so the check can be skipped altogether.
326+
*/
327+
if (!IS_ENABLED(CONFIG_RETPOLINE) &&
328+
!IS_ENABLED(CONFIG_X86_KERNEL_IBT) &&
329+
insn_is_indirect_jump(&insn))
330+
return 0;
331+
if (insn_jump_into_range(&insn, paddr + INT3_INSN_SIZE,
340332
DISP32_SIZE))
341333
return 0;
342334
addr += insn.length;

arch/x86/kernel/vmlinux.lds.S

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,8 @@ SECTIONS
133133
KPROBES_TEXT
134134
SOFTIRQENTRY_TEXT
135135
#ifdef CONFIG_RETPOLINE
136-
__indirect_thunk_start = .;
137136
*(.text..__x86.indirect_thunk)
138137
*(.text..__x86.return_thunk)
139-
__indirect_thunk_end = .;
140138
#endif
141139
STATIC_CALL_TEXT
142140

tools/perf/util/thread-stack.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,9 +1038,7 @@ static int thread_stack__trace_end(struct thread_stack *ts,
10381038

10391039
static bool is_x86_retpoline(const char *name)
10401040
{
1041-
const char *p = strstr(name, "__x86_indirect_thunk_");
1042-
1043-
return p == name || !strcmp(name, "__indirect_thunk_start");
1041+
return strstr(name, "__x86_indirect_thunk_") == name;
10441042
}
10451043

10461044
/*

0 commit comments

Comments
 (0)