[PATCH v2 0/2] x86/retpoline,kprobes: Fix the [__indirect_thunk_start, ..end] range

[PATCH v2 0/2] x86/retpoline,kprobes: Fix the [__indirect_thunk_start, ..end] range

[Petr Pavlu]

Fix problems with an output position of thunk sections and the
associated definition of range [__indirect_thunk_start,
__indirect_thunk_end] which affects the kprobes optimization.

Initial v1 of the series kept the mentioned range but it turns out the
logic which uses it is not necessary so it is removed altogether.

Changes since v1 [1]:
- Drop the patch which moved the return thunk out of
  [__indirect_thunk_start, ..end] and instead replace it with a removal
  of the kprobes optimization check which looked for calls to indirect
  thunks.
- Slightly adjust the commit message for the first patch, to better
  match the new second patch.

[1] https://lore.kernel.org/lkml/20230705081547.25130-1-petr.pavlu@suse.com/

Petr Pavlu (2):
  x86/retpoline,kprobes: Fix position of thunk sections with
    CONFIG_LTO_CLANG
  x86/retpoline,kprobes: Skip optprobe check for indirect jumps with
    retpolines and IBT

 arch/x86/include/asm/nospec-branch.h |  3 ---
 arch/x86/kernel/kprobes/opt.c        | 40 +++++++++++-----------------
 arch/x86/kernel/vmlinux.lds.S        |  4 +--
 arch/x86/lib/retpoline.S             |  4 +--
 tools/perf/util/thread-stack.c       |  4 +--
 5 files changed, 20 insertions(+), 35 deletions(-)

-- 
2.35.3

[PATCH v2 1/2] x86/retpoline,kprobes: Fix position of thunk sections with CONFIG_LTO_CLANG

[Petr Pavlu]

Linker script arch/x86/kernel/vmlinux.lds.S matches the thunk sections
".text.__x86.*" from arch/x86/lib/retpoline.S as follows:

.text {
  [...]
  TEXT_TEXT
  [...]
  __indirect_thunk_start = .;
  *(.text.__x86.*)
  __indirect_thunk_end = .;
  [...]
}

Macro TEXT_TEXT references TEXT_MAIN which normally expands to only
".text". However, with CONFIG_LTO_CLANG, TEXT_MAIN becomes
".text .text.[0-9a-zA-Z_]*" which wrongly matches also the thunk
sections. The output layout is then different than expected. For
instance, the currently defined range [__indirect_thunk_start,
__indirect_thunk_end] becomes empty.

Prevent the problem by using ".." as the first separator, for example,
".text..__x86.indirect_thunk". This pattern is utilized by other
explicit section names which start with one of the standard prefixes,
such as ".text" or ".data", and that need to be individually selected in
the linker script.

Fixes: dc5723b02e52 ("kbuild: add support for Clang LTO")
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/kernel/vmlinux.lds.S | 2 +-
 arch/x86/lib/retpoline.S      | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 03c885d3640f..a4cd04c458df 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -134,7 +134,7 @@ SECTIONS
 		SOFTIRQENTRY_TEXT
 #ifdef CONFIG_RETPOLINE
 		__indirect_thunk_start = .;
-		*(.text.__x86.*)
+		*(.text..__x86.*)
 		__indirect_thunk_end = .;
 #endif
 		STATIC_CALL_TEXT
diff --git a/arch/x86/lib/retpoline.S b/arch/x86/lib/retpoline.S
index 3fd066d42ec0..3bea96341d00 100644
--- a/arch/x86/lib/retpoline.S
+++ b/arch/x86/lib/retpoline.S
@@ -12,7 +12,7 @@
 #include <asm/percpu.h>
 #include <asm/frame.h>
 
-	.section .text.__x86.indirect_thunk
+	.section .text..__x86.indirect_thunk
 
 
 .macro POLINE reg
@@ -131,7 +131,7 @@ SYM_CODE_END(__x86_indirect_jump_thunk_array)
  */
 #ifdef CONFIG_RETHUNK
 
-	.section .text.__x86.return_thunk
+	.section .text..__x86.return_thunk
 
 /*
  * Safety details here pertain to the AMD Zen{1,2} microarchitecture:
-- 
2.35.3

[PATCH v2 2/2] x86/retpoline,kprobes: Skip optprobe check for indirect jumps with retpolines and IBT

[Petr Pavlu]

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: 0b53c374b9ef ("x86/retpoline: Use -mfunction-return")
Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
 arch/x86/include/asm/nospec-branch.h |  3 ---
 arch/x86/kernel/kprobes/opt.c        | 40 +++++++++++-----------------
 arch/x86/kernel/vmlinux.lds.S        |  2 --
 tools/perf/util/thread-stack.c       |  4 +--
 4 files changed, 17 insertions(+), 32 deletions(-)

diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 55388c9f7601..c5460be93fa7 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -461,9 +461,6 @@ enum ssb_mitigation {
 	SPEC_STORE_BYPASS_SECCOMP,
 };
 
-extern char __indirect_thunk_start[];
-extern char __indirect_thunk_end[];
-
 static __always_inline
 void alternative_msr_write(unsigned int msr, u64 val, unsigned int feature)
 {
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 57b0037d0a99..517821b48391 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -226,7 +226,7 @@ static int copy_optimized_instructions(u8 *dest, u8 *src, u8 *real)
 }
 
 /* Check whether insn is indirect jump */
-static int __insn_is_indirect_jump(struct insn *insn)
+static int insn_is_indirect_jump(struct insn *insn)
 {
 	return ((insn->opcode.bytes[0] == 0xff &&
 		(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)
 	return (start <= target && target <= start + len);
 }
 
-static int insn_is_indirect_jump(struct insn *insn)
-{
-	int ret = __insn_is_indirect_jump(insn);
-
-#ifdef CONFIG_RETPOLINE
-	/*
-	 * Jump to x86_indirect_thunk_* is treated as an indirect jump.
-	 * Note that even with CONFIG_RETPOLINE=y, the kernel compiled with
-	 * older gcc may use indirect jump. So we add this check instead of
-	 * replace indirect-jump check.
-	 */
-	if (!ret)
-		ret = insn_jump_into_range(insn,
-				(unsigned long)__indirect_thunk_start,
-				(unsigned long)__indirect_thunk_end -
-				(unsigned long)__indirect_thunk_start);
-#endif
-	return ret;
-}
-
 /* Decode whole function to ensure any instructions don't jump into target */
 static int can_optimize(unsigned long paddr)
 {
@@ -334,9 +314,21 @@ static int can_optimize(unsigned long paddr)
 		/* Recover address */
 		insn.kaddr = (void *)addr;
 		insn.next_byte = (void *)(addr + insn.length);
-		/* Check any instructions don't jump into target */
-		if (insn_is_indirect_jump(&insn) ||
-		    insn_jump_into_range(&insn, paddr + INT3_INSN_SIZE,
+		/*
+		 * Check any instructions don't jump into target, indirectly or
+		 * directly.
+		 *
+		 * The indirect case is present to handle a code with jump
+		 * tables. When the kernel uses retpolines, the check should in
+		 * theory additionally look for jumps to indirect thunks.
+		 * However, the kernel built with retpolines or IBT has jump
+		 * tables disabled so the check can be skipped altogether.
+		 */
+		if (!IS_ENABLED(CONFIG_RETPOLINE) &&
+		    !IS_ENABLED(CONFIG_X86_KERNEL_IBT) &&
+		    insn_is_indirect_jump(&insn))
+			return 0;
+		if (insn_jump_into_range(&insn, paddr + INT3_INSN_SIZE,
 					 DISP32_SIZE))
 			return 0;
 		addr += insn.length;
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index a4cd04c458df..dd5b0a68cf84 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -133,9 +133,7 @@ SECTIONS
 		KPROBES_TEXT
 		SOFTIRQENTRY_TEXT
 #ifdef CONFIG_RETPOLINE
-		__indirect_thunk_start = .;
 		*(.text..__x86.*)
-		__indirect_thunk_end = .;
 #endif
 		STATIC_CALL_TEXT
 
diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
index 374d142e7390..c6a0a27b12c2 100644
--- a/tools/perf/util/thread-stack.c
+++ b/tools/perf/util/thread-stack.c
@@ -1038,9 +1038,7 @@ static int thread_stack__trace_end(struct thread_stack *ts,
 
 static bool is_x86_retpoline(const char *name)
 {
-	const char *p = strstr(name, "__x86_indirect_thunk_");
-
-	return p == name || !strcmp(name, "__indirect_thunk_start");
+	return strstr(name, "__x86_indirect_thunk_") == name;
 }
 
 /*
-- 
2.35.3

Re: [PATCH v2 2/2] x86/retpoline,kprobes: Skip optprobe check for indirect jumps with retpolines and IBT

[Masami Hiramatsu]

On Tue, 11 Jul 2023 11:19:52 +0200
Petr Pavlu <petr.pavlu@suse.com> wrote:

> 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.

This looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thanks!

> 
> Fixes: 0b53c374b9ef ("x86/retpoline: Use -mfunction-return")
> Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
>  arch/x86/include/asm/nospec-branch.h |  3 ---
>  arch/x86/kernel/kprobes/opt.c        | 40 +++++++++++-----------------
>  arch/x86/kernel/vmlinux.lds.S        |  2 --
>  tools/perf/util/thread-stack.c       |  4 +--
>  4 files changed, 17 insertions(+), 32 deletions(-)
> 
> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> index 55388c9f7601..c5460be93fa7 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -461,9 +461,6 @@ enum ssb_mitigation {
>  	SPEC_STORE_BYPASS_SECCOMP,
>  };
>  
> -extern char __indirect_thunk_start[];
> -extern char __indirect_thunk_end[];
> -
>  static __always_inline
>  void alternative_msr_write(unsigned int msr, u64 val, unsigned int feature)
>  {
> diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
> index 57b0037d0a99..517821b48391 100644
> --- a/arch/x86/kernel/kprobes/opt.c
> +++ b/arch/x86/kernel/kprobes/opt.c
> @@ -226,7 +226,7 @@ static int copy_optimized_instructions(u8 *dest, u8 *src, u8 *real)
>  }
>  
>  /* Check whether insn is indirect jump */
> -static int __insn_is_indirect_jump(struct insn *insn)
> +static int insn_is_indirect_jump(struct insn *insn)
>  {
>  	return ((insn->opcode.bytes[0] == 0xff &&
>  		(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)
>  	return (start <= target && target <= start + len);
>  }
>  
> -static int insn_is_indirect_jump(struct insn *insn)
> -{
> -	int ret = __insn_is_indirect_jump(insn);
> -
> -#ifdef CONFIG_RETPOLINE
> -	/*
> -	 * Jump to x86_indirect_thunk_* is treated as an indirect jump.
> -	 * Note that even with CONFIG_RETPOLINE=y, the kernel compiled with
> -	 * older gcc may use indirect jump. So we add this check instead of
> -	 * replace indirect-jump check.
> -	 */
> -	if (!ret)
> -		ret = insn_jump_into_range(insn,
> -				(unsigned long)__indirect_thunk_start,
> -				(unsigned long)__indirect_thunk_end -
> -				(unsigned long)__indirect_thunk_start);
> -#endif
> -	return ret;
> -}
> -
>  /* Decode whole function to ensure any instructions don't jump into target */
>  static int can_optimize(unsigned long paddr)
>  {
> @@ -334,9 +314,21 @@ static int can_optimize(unsigned long paddr)
>  		/* Recover address */
>  		insn.kaddr = (void *)addr;
>  		insn.next_byte = (void *)(addr + insn.length);
> -		/* Check any instructions don't jump into target */
> -		if (insn_is_indirect_jump(&insn) ||
> -		    insn_jump_into_range(&insn, paddr + INT3_INSN_SIZE,
> +		/*
> +		 * Check any instructions don't jump into target, indirectly or
> +		 * directly.
> +		 *
> +		 * The indirect case is present to handle a code with jump
> +		 * tables. When the kernel uses retpolines, the check should in
> +		 * theory additionally look for jumps to indirect thunks.
> +		 * However, the kernel built with retpolines or IBT has jump
> +		 * tables disabled so the check can be skipped altogether.
> +		 */
> +		if (!IS_ENABLED(CONFIG_RETPOLINE) &&
> +		    !IS_ENABLED(CONFIG_X86_KERNEL_IBT) &&
> +		    insn_is_indirect_jump(&insn))
> +			return 0;
> +		if (insn_jump_into_range(&insn, paddr + INT3_INSN_SIZE,
>  					 DISP32_SIZE))
>  			return 0;
>  		addr += insn.length;
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index a4cd04c458df..dd5b0a68cf84 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -133,9 +133,7 @@ SECTIONS
>  		KPROBES_TEXT
>  		SOFTIRQENTRY_TEXT
>  #ifdef CONFIG_RETPOLINE
> -		__indirect_thunk_start = .;
>  		*(.text..__x86.*)
> -		__indirect_thunk_end = .;
>  #endif
>  		STATIC_CALL_TEXT
>  
> diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
> index 374d142e7390..c6a0a27b12c2 100644
> --- a/tools/perf/util/thread-stack.c
> +++ b/tools/perf/util/thread-stack.c
> @@ -1038,9 +1038,7 @@ static int thread_stack__trace_end(struct thread_stack *ts,
>  
>  static bool is_x86_retpoline(const char *name)
>  {
> -	const char *p = strstr(name, "__x86_indirect_thunk_");
> -
> -	return p == name || !strcmp(name, "__indirect_thunk_start");
> +	return strstr(name, "__x86_indirect_thunk_") == name;
>  }
>  
>  /*
> -- 
> 2.35.3
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH v2 0/2] x86/retpoline,kprobes: Fix the [__indirect_thunk_start, ..end] range

[Masami Hiramatsu]

Hi Peter,

Can you pick this series to tip tree?
I think, these affect to x86 arch code.

Thank you,

On Tue, 11 Jul 2023 11:19:50 +0200
Petr Pavlu <petr.pavlu@suse.com> wrote:

> Fix problems with an output position of thunk sections and the
> associated definition of range [__indirect_thunk_start,
> __indirect_thunk_end] which affects the kprobes optimization.
> 
> Initial v1 of the series kept the mentioned range but it turns out the
> logic which uses it is not necessary so it is removed altogether.
> 
> Changes since v1 [1]:
> - Drop the patch which moved the return thunk out of
>   [__indirect_thunk_start, ..end] and instead replace it with a removal
>   of the kprobes optimization check which looked for calls to indirect
>   thunks.
> - Slightly adjust the commit message for the first patch, to better
>   match the new second patch.
> 
> [1] https://lore.kernel.org/lkml/20230705081547.25130-1-petr.pavlu@suse.com/
> 
> Petr Pavlu (2):
>   x86/retpoline,kprobes: Fix position of thunk sections with
>     CONFIG_LTO_CLANG
>   x86/retpoline,kprobes: Skip optprobe check for indirect jumps with
>     retpolines and IBT
> 
>  arch/x86/include/asm/nospec-branch.h |  3 ---
>  arch/x86/kernel/kprobes/opt.c        | 40 +++++++++++-----------------
>  arch/x86/kernel/vmlinux.lds.S        |  4 +--
>  arch/x86/lib/retpoline.S             |  4 +--
>  tools/perf/util/thread-stack.c       |  4 +--
>  5 files changed, 20 insertions(+), 35 deletions(-)
> 
> -- 
> 2.35.3
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH v2 0/2] x86/retpoline,kprobes: Fix the [__indirect_thunk_start, ..end] range

[Peter Zijlstra]

On Sun, Jul 30, 2023 at 12:14:35AM +0900, Masami Hiramatsu wrote:
> Hi Peter,
> 
> Can you pick this series to tip tree?

Will do!

[tip: x86/core] x86/retpoline,kprobes: Skip optprobe check for indirect jumps with retpolines and IBT

[tip-bot2 for Petr Pavlu]

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

Commit-ID:     029239c5b0e6484e4443be90e5664fd0bf0f066b
Gitweb:        https://git.kernel.org/tip/029239c5b0e6484e4443be90e5664fd0bf0f066b
Author:        Petr Pavlu <petr.pavlu@suse.com>
AuthorDate:    Tue, 11 Jul 2023 11:19:52 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 02 Aug 2023 16:27:08 +02:00

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: 0b53c374b9ef ("x86/retpoline: Use -mfunction-return")
Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Link: https://lore.kernel.org/r/20230711091952.27944-3-petr.pavlu@suse.com
---
 arch/x86/include/asm/nospec-branch.h |  3 +--
 arch/x86/kernel/kprobes/opt.c        | 40 ++++++++++-----------------
 arch/x86/kernel/vmlinux.lds.S        |  2 +-
 tools/perf/util/thread-stack.c       |  4 +---
 4 files changed, 17 insertions(+), 32 deletions(-)

diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 1a65cf4..db460e6 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -465,9 +465,6 @@ enum ssb_mitigation {
 	SPEC_STORE_BYPASS_SECCOMP,
 };
 
-extern char __indirect_thunk_start[];
-extern char __indirect_thunk_end[];
-
 static __always_inline
 void alternative_msr_write(unsigned int msr, u64 val, unsigned int feature)
 {
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 57b0037..517821b 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -226,7 +226,7 @@ static int copy_optimized_instructions(u8 *dest, u8 *src, u8 *real)
 }
 
 /* Check whether insn is indirect jump */
-static int __insn_is_indirect_jump(struct insn *insn)
+static int insn_is_indirect_jump(struct insn *insn)
 {
 	return ((insn->opcode.bytes[0] == 0xff &&
 		(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)
 	return (start <= target && target <= start + len);
 }
 
-static int insn_is_indirect_jump(struct insn *insn)
-{
-	int ret = __insn_is_indirect_jump(insn);
-
-#ifdef CONFIG_RETPOLINE
-	/*
-	 * Jump to x86_indirect_thunk_* is treated as an indirect jump.
-	 * Note that even with CONFIG_RETPOLINE=y, the kernel compiled with
-	 * older gcc may use indirect jump. So we add this check instead of
-	 * replace indirect-jump check.
-	 */
-	if (!ret)
-		ret = insn_jump_into_range(insn,
-				(unsigned long)__indirect_thunk_start,
-				(unsigned long)__indirect_thunk_end -
-				(unsigned long)__indirect_thunk_start);
-#endif
-	return ret;
-}
-
 /* Decode whole function to ensure any instructions don't jump into target */
 static int can_optimize(unsigned long paddr)
 {
@@ -334,9 +314,21 @@ static int can_optimize(unsigned long paddr)
 		/* Recover address */
 		insn.kaddr = (void *)addr;
 		insn.next_byte = (void *)(addr + insn.length);
-		/* Check any instructions don't jump into target */
-		if (insn_is_indirect_jump(&insn) ||
-		    insn_jump_into_range(&insn, paddr + INT3_INSN_SIZE,
+		/*
+		 * Check any instructions don't jump into target, indirectly or
+		 * directly.
+		 *
+		 * The indirect case is present to handle a code with jump
+		 * tables. When the kernel uses retpolines, the check should in
+		 * theory additionally look for jumps to indirect thunks.
+		 * However, the kernel built with retpolines or IBT has jump
+		 * tables disabled so the check can be skipped altogether.
+		 */
+		if (!IS_ENABLED(CONFIG_RETPOLINE) &&
+		    !IS_ENABLED(CONFIG_X86_KERNEL_IBT) &&
+		    insn_is_indirect_jump(&insn))
+			return 0;
+		if (insn_jump_into_range(&insn, paddr + INT3_INSN_SIZE,
 					 DISP32_SIZE))
 			return 0;
 		addr += insn.length;
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index a4cd04c..dd5b0a6 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -133,9 +133,7 @@ SECTIONS
 		KPROBES_TEXT
 		SOFTIRQENTRY_TEXT
 #ifdef CONFIG_RETPOLINE
-		__indirect_thunk_start = .;
 		*(.text..__x86.*)
-		__indirect_thunk_end = .;
 #endif
 		STATIC_CALL_TEXT
 
diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
index 374d142..c6a0a27 100644
--- a/tools/perf/util/thread-stack.c
+++ b/tools/perf/util/thread-stack.c
@@ -1038,9 +1038,7 @@ static int thread_stack__trace_end(struct thread_stack *ts,
 
 static bool is_x86_retpoline(const char *name)
 {
-	const char *p = strstr(name, "__x86_indirect_thunk_");
-
-	return p == name || !strcmp(name, "__indirect_thunk_start");
+	return strstr(name, "__x86_indirect_thunk_") == name;
 }
 
 /*

[tip: x86/core] x86/retpoline,kprobes: Fix position of thunk sections with CONFIG_LTO_CLANG

[tip-bot2 for Petr Pavlu]

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

Commit-ID:     973ab2d61f33dc85212c486e624af348c4eeb5c9
Gitweb:        https://git.kernel.org/tip/973ab2d61f33dc85212c486e624af348c4eeb5c9
Author:        Petr Pavlu <petr.pavlu@suse.com>
AuthorDate:    Tue, 11 Jul 2023 11:19:51 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 02 Aug 2023 16:27:07 +02:00

x86/retpoline,kprobes: Fix position of thunk sections with CONFIG_LTO_CLANG

Linker script arch/x86/kernel/vmlinux.lds.S matches the thunk sections
".text.__x86.*" from arch/x86/lib/retpoline.S as follows:

.text {
  [...]
  TEXT_TEXT
  [...]
  __indirect_thunk_start = .;
  *(.text.__x86.*)
  __indirect_thunk_end = .;
  [...]
}

Macro TEXT_TEXT references TEXT_MAIN which normally expands to only
".text". However, with CONFIG_LTO_CLANG, TEXT_MAIN becomes
".text .text.[0-9a-zA-Z_]*" which wrongly matches also the thunk
sections. The output layout is then different than expected. For
instance, the currently defined range [__indirect_thunk_start,
__indirect_thunk_end] becomes empty.

Prevent the problem by using ".." as the first separator, for example,
".text..__x86.indirect_thunk". This pattern is utilized by other
explicit section names which start with one of the standard prefixes,
such as ".text" or ".data", and that need to be individually selected in
the linker script.

Fixes: dc5723b02e52 ("kbuild: add support for Clang LTO")
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20230711091952.27944-2-petr.pavlu@suse.com
---
 arch/x86/kernel/vmlinux.lds.S | 2 +-
 arch/x86/lib/retpoline.S      | 4 ++--
 tools/objtool/check.c         | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 03c885d..a4cd04c 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -134,7 +134,7 @@ SECTIONS
 		SOFTIRQENTRY_TEXT
 #ifdef CONFIG_RETPOLINE
 		__indirect_thunk_start = .;
-		*(.text.__x86.*)
+		*(.text..__x86.*)
 		__indirect_thunk_end = .;
 #endif
 		STATIC_CALL_TEXT
diff --git a/arch/x86/lib/retpoline.S b/arch/x86/lib/retpoline.S
index 3fd066d..3bea963 100644
--- a/arch/x86/lib/retpoline.S
+++ b/arch/x86/lib/retpoline.S
@@ -12,7 +12,7 @@
 #include <asm/percpu.h>
 #include <asm/frame.h>
 
-	.section .text.__x86.indirect_thunk
+	.section .text..__x86.indirect_thunk
 
 
 .macro POLINE reg
@@ -131,7 +131,7 @@ SYM_CODE_END(__x86_indirect_jump_thunk_array)
  */
 #ifdef CONFIG_RETHUNK
 
-	.section .text.__x86.return_thunk
+	.section .text..__x86.return_thunk
 
 /*
  * Safety details here pertain to the AMD Zen{1,2} microarchitecture:
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 8936a05..e096eb3 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -389,7 +389,7 @@ static int decode_instructions(struct objtool_file *file)
 		if (!strcmp(sec->name, ".noinstr.text") ||
 		    !strcmp(sec->name, ".entry.text") ||
 		    !strcmp(sec->name, ".cpuidle.text") ||
-		    !strncmp(sec->name, ".text.__x86.", 12))
+		    !strncmp(sec->name, ".text..__x86.", 12))
 			sec->noinstr = true;
 
 		/*

Re: [tip: x86/core] x86/retpoline,kprobes: Fix position of thunk sections with CONFIG_LTO_CLANG

[Josh Poimboeuf]

On Wed, Aug 02, 2023 at 02:33:16PM -0000, tip-bot2 for Petr Pavlu wrote:
> The following commit has been merged into the x86/core branch of tip:
> 
> Commit-ID:     973ab2d61f33dc85212c486e624af348c4eeb5c9
> Gitweb:        https://git.kernel.org/tip/973ab2d61f33dc85212c486e624af348c4eeb5c9
> Author:        Petr Pavlu <petr.pavlu@suse.com>
> AuthorDate:    Tue, 11 Jul 2023 11:19:51 +02:00
> Committer:     Peter Zijlstra <peterz@infradead.org>
> CommitterDate: Wed, 02 Aug 2023 16:27:07 +02:00
> 
> x86/retpoline,kprobes: Fix position of thunk sections with CONFIG_LTO_CLANG
> 

[...]

> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -389,7 +389,7 @@ static int decode_instructions(struct objtool_file *file)
>  		if (!strcmp(sec->name, ".noinstr.text") ||
>  		    !strcmp(sec->name, ".entry.text") ||
>  		    !strcmp(sec->name, ".cpuidle.text") ||
> -		    !strncmp(sec->name, ".text.__x86.", 12))
> +		    !strncmp(sec->name, ".text..__x86.", 12))

Andy Cooper reported this should be 13.

-- 
Josh

[PATCH] x86/retpoline,kprobes: Fix "Fix position of thunk sections with CONFIG_LTO_CLANG"

[Andrew Cooper]

Lets hope there are no .text..__x86womble sections around.

Fixes: 973ab2d61f33 ("x86/retpoline,kprobes: Fix position of thunk sections with CONFIG_LTO_CLANG")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Petr Pavlu <petr.pavlu@suse.com>
CC: Peter Zijlstra (Intel) <peterz@infradead.org>
CC: Josh Poimboeuf <jpoimboe@kernel.org>
CC: linux-kernel@vger.kernel.org

Alternatively,

int strstarts(const char *s1, const char *s2)
{
        return strncmp(s1, s2, strlen(s2));
}
---
 tools/objtool/check.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index e096eb325acd..e2ee10ce7703 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -389,7 +389,7 @@ static int decode_instructions(struct objtool_file *file)
 		if (!strcmp(sec->name, ".noinstr.text") ||
 		    !strcmp(sec->name, ".entry.text") ||
 		    !strcmp(sec->name, ".cpuidle.text") ||
-		    !strncmp(sec->name, ".text..__x86.", 12))
+		    !strncmp(sec->name, ".text..__x86.", 13))
 			sec->noinstr = true;
 
 		/*

base-commit: 029239c5b0e6484e4443be90e5664fd0bf0f066b
-- 
2.30.2

Re: [PATCH] x86/retpoline,kprobes: Fix "Fix position of thunk sections with CONFIG_LTO_CLANG"

[Peter Zijlstra]

On Fri, Aug 04, 2023 at 12:03:23AM +0100, Andrew Cooper wrote:
> Lets hope there are no .text..__x86womble sections around.
> 
> Fixes: 973ab2d61f33 ("x86/retpoline,kprobes: Fix position of thunk sections with CONFIG_LTO_CLANG")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Petr Pavlu <petr.pavlu@suse.com>
> CC: Peter Zijlstra (Intel) <peterz@infradead.org>
> CC: Josh Poimboeuf <jpoimboe@kernel.org>
> CC: linux-kernel@vger.kernel.org
> 
> Alternatively,
> 
> int strstarts(const char *s1, const char *s2)
> {
>         return strncmp(s1, s2, strlen(s2));
> }

Durr, I hate C ;-/ And yes, we have a ton of it, lemme try that
strstarts thing.

Re: [PATCH] x86/retpoline,kprobes: Fix "Fix position of thunk sections with CONFIG_LTO_CLANG"

[Peter Zijlstra]

On Fri, Aug 04, 2023 at 10:28:53AM +0200, Peter Zijlstra wrote:
> On Fri, Aug 04, 2023 at 12:03:23AM +0100, Andrew Cooper wrote:
> > Lets hope there are no .text..__x86womble sections around.
> > 
> > Fixes: 973ab2d61f33 ("x86/retpoline,kprobes: Fix position of thunk sections with CONFIG_LTO_CLANG")
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > ---
> > CC: Petr Pavlu <petr.pavlu@suse.com>
> > CC: Peter Zijlstra (Intel) <peterz@infradead.org>
> > CC: Josh Poimboeuf <jpoimboe@kernel.org>
> > CC: linux-kernel@vger.kernel.org
> > 
> > Alternatively,
> > 
> > int strstarts(const char *s1, const char *s2)
> > {
> >         return strncmp(s1, s2, strlen(s2));
> > }
> 
> Durr, I hate C ;-/ And yes, we have a ton of it, lemme try that
> strstarts thing.

So there, that builds and compiles a kernel, must be good :-)

Now, let me go make some wake-up juice ...

---
 tools/objtool/check.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index e096eb325acd..a2b624b580ff 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -20,6 +20,7 @@
 #include <linux/hashtable.h>
 #include <linux/kernel.h>
 #include <linux/static_call_types.h>
+#include <linux/string.h>
 
 struct alternative {
 	struct alternative *next;
@@ -383,13 +384,13 @@ static int decode_instructions(struct objtool_file *file)
 
 		if (strcmp(sec->name, ".altinstr_replacement") &&
 		    strcmp(sec->name, ".altinstr_aux") &&
-		    strncmp(sec->name, ".discard.", 9))
+		    !strstarts(sec->name, ".discard."))
 			sec->text = true;
 
 		if (!strcmp(sec->name, ".noinstr.text") ||
 		    !strcmp(sec->name, ".entry.text") ||
 		    !strcmp(sec->name, ".cpuidle.text") ||
-		    !strncmp(sec->name, ".text..__x86.", 12))
+		    strstarts(sec->name, ".text..__x86."))
 			sec->noinstr = true;
 
 		/*
@@ -709,8 +710,7 @@ static int create_static_call_sections(struct objtool_file *file)
 			perror("strdup");
 			return -1;
 		}
-		if (strncmp(key_name, STATIC_CALL_TRAMP_PREFIX_STR,
-			    STATIC_CALL_TRAMP_PREFIX_LEN)) {
+		if (!strstarts(key_name, STATIC_CALL_TRAMP_PREFIX_STR)) {
 			WARN("static_call: trampoline name malformed: %s", key_name);
 			free(key_name);
 			return -1;
@@ -900,7 +900,7 @@ static int create_cfi_sections(struct objtool_file *file)
 		if (sym->type != STT_FUNC)
 			continue;
 
-		if (strncmp(sym->name, "__cfi_", 6))
+		if (!strstarts(sym->name, "__cfi_"))
 			continue;
 
 		idx++;
@@ -916,7 +916,7 @@ static int create_cfi_sections(struct objtool_file *file)
 		if (sym->type != STT_FUNC)
 			continue;
 
-		if (strncmp(sym->name, "__cfi_", 6))
+		if (!strstarts(sym->name, "__cfi_"))
 			continue;
 
 		if (!elf_init_reloc_text_sym(file->elf, sec,
@@ -2468,7 +2468,7 @@ static bool is_profiling_func(const char *name)
 	/*
 	 * Many compilers cannot disable KCOV with a function attribute.
 	 */
-	if (!strncmp(name, "__sanitizer_cov_", 16))
+	if (strstarts(name, "__sanitizer_cov_"))
 		return true;
 
 	/*
@@ -2477,7 +2477,7 @@ static bool is_profiling_func(const char *name)
 	 * the __no_sanitize_thread attribute, remove them. Once the kernel's
 	 * minimum Clang version is 14.0, this can be removed.
 	 */
-	if (!strncmp(name, "__tsan_func_", 12) ||
+	if (strstarts(name, "__tsan_func_") ||
 	    !strcmp(name, "__tsan_atomic_signal_fence"))
 		return true;
 
@@ -2492,8 +2492,7 @@ static int classify_symbols(struct objtool_file *file)
 		if (func->bind != STB_GLOBAL)
 			continue;
 
-		if (!strncmp(func->name, STATIC_CALL_TRAMP_PREFIX_STR,
-			     strlen(STATIC_CALL_TRAMP_PREFIX_STR)))
+		if (strstarts(func->name, STATIC_CALL_TRAMP_PREFIX_STR))
 			func->static_call_tramp = true;
 
 		if (arch_is_retpoline(func))
@@ -2528,7 +2527,7 @@ static void mark_rodata(struct objtool_file *file)
 	 * .rodata.str1.* sections are ignored; they don't contain jump tables.
 	 */
 	for_each_sec(file, sec) {
-		if (!strncmp(sec->name, ".rodata", 7) &&
+		if (strstarts(sec->name, ".rodata") &&
 		    !strstr(sec->name, ".str1.")) {
 			sec->rodata = true;
 			found = true;
@@ -3400,7 +3399,7 @@ static inline bool noinstr_call_dest(struct objtool_file *file,
 	 * something 'BAD' happened. At the risk of taking the machine down,
 	 * let them proceed to get the message out.
 	 */
-	if (!strncmp(func->name, "__ubsan_handle_", 15))
+	if (strstarts(func->name, "__ubsan_handle_"))
 		return true;
 
 	return false;
@@ -3531,8 +3530,8 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 
 		if (func && insn_func(insn) && func != insn_func(insn)->pfunc) {
 			/* Ignore KCFI type preambles, which always fall through */
-			if (!strncmp(func->name, "__cfi_", 6) ||
-			    !strncmp(func->name, "__pfx_", 6))
+			if (strstarts(func->name, "__cfi_") ||
+			    strstarts(func->name, "__pfx_"))
 				return 0;
 
 			WARN("%s() falls through to next function %s()",
@@ -4401,9 +4400,9 @@ static int validate_ibt(struct objtool_file *file)
 		 * These sections can reference text addresses, but not with
 		 * the intent to indirect branch to them.
 		 */
-		if ((!strncmp(sec->name, ".discard", 8) &&
+		if ((strstarts(sec->name, ".discard") &&
 		     strcmp(sec->name, ".discard.ibt_endbr_noseal"))	||
-		    !strncmp(sec->name, ".debug", 6)			||
+		    strstarts(sec->name, ".debug")			||
 		    !strcmp(sec->name, ".altinstructions")		||
 		    !strcmp(sec->name, ".ibt_endbr_seal")		||
 		    !strcmp(sec->name, ".orc_unwind_ip")		||

[tip: x86/urgent] x86/retpoline,kprobes: Fix position of thunk sections with CONFIG_LTO_CLANG

[tip-bot2 for Petr Pavlu]

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

Commit-ID:     79cd2a11224eab86d6673fe8a11d2046ae9d2757
Gitweb:        https://git.kernel.org/tip/79cd2a11224eab86d6673fe8a11d2046ae9d2757
Author:        Petr Pavlu <petr.pavlu@suse.com>
AuthorDate:    Tue, 11 Jul 2023 11:19:51 +02:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Mon, 14 Aug 2023 11:44:19 +02:00

x86/retpoline,kprobes: Fix position of thunk sections with CONFIG_LTO_CLANG

The linker script arch/x86/kernel/vmlinux.lds.S matches the thunk
sections ".text.__x86.*" from arch/x86/lib/retpoline.S as follows:

  .text {
    [...]
    TEXT_TEXT
    [...]
    __indirect_thunk_start = .;
    *(.text.__x86.*)
    __indirect_thunk_end = .;
    [...]
  }

Macro TEXT_TEXT references TEXT_MAIN which normally expands to only
".text". However, with CONFIG_LTO_CLANG, TEXT_MAIN becomes
".text .text.[0-9a-zA-Z_]*" which wrongly matches also the thunk
sections. The output layout is then different than expected. For
instance, the currently defined range [__indirect_thunk_start,
__indirect_thunk_end] becomes empty.

Prevent the problem by using ".." as the first separator, for example,
".text..__x86.indirect_thunk". This pattern is utilized by other
explicit section names which start with one of the standard prefixes,
such as ".text" or ".data", and that need to be individually selected in
the linker script.

  [ nathan: Fix conflicts with SRSO and fold in fix issue brought up by
    Andrew Cooper in post-review:
    https://lore.kernel.org/20230803230323.1478869-1-andrew.cooper3@citrix.com ]

Fixes: dc5723b02e52 ("kbuild: add support for Clang LTO")
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Link: https://lore.kernel.org/r/20230711091952.27944-2-petr.pavlu@suse.com
---
 arch/x86/kernel/vmlinux.lds.S | 8 ++++----
 arch/x86/lib/retpoline.S      | 8 ++++----
 tools/objtool/check.c         | 2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index ef06211..dfb8783 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -134,15 +134,15 @@ SECTIONS
 		SOFTIRQENTRY_TEXT
 #ifdef CONFIG_RETPOLINE
 		__indirect_thunk_start = .;
-		*(.text.__x86.indirect_thunk)
-		*(.text.__x86.return_thunk)
+		*(.text..__x86.indirect_thunk)
+		*(.text..__x86.return_thunk)
 		__indirect_thunk_end = .;
 #endif
 		STATIC_CALL_TEXT
 
 		ALIGN_ENTRY_TEXT_BEGIN
 #ifdef CONFIG_CPU_SRSO
-		*(.text.__x86.rethunk_untrain)
+		*(.text..__x86.rethunk_untrain)
 #endif
 
 		ENTRY_TEXT
@@ -153,7 +153,7 @@ SECTIONS
 		 * definition.
 		 */
 		. = srso_untrain_ret_alias | (1 << 2) | (1 << 8) | (1 << 14) | (1 << 20);
-		*(.text.__x86.rethunk_safe)
+		*(.text..__x86.rethunk_safe)
 #endif
 		ALIGN_ENTRY_TEXT_END
 		*(.gnu.warning)
diff --git a/arch/x86/lib/retpoline.S b/arch/x86/lib/retpoline.S
index 132cedb..8db74d8 100644
--- a/arch/x86/lib/retpoline.S
+++ b/arch/x86/lib/retpoline.S
@@ -13,7 +13,7 @@
 #include <asm/frame.h>
 #include <asm/nops.h>
 
-	.section .text.__x86.indirect_thunk
+	.section .text..__x86.indirect_thunk
 
 
 .macro POLINE reg
@@ -148,7 +148,7 @@ SYM_CODE_END(__x86_indirect_jump_thunk_array)
  * As a result, srso_safe_ret_alias() becomes a safe return.
  */
 #ifdef CONFIG_CPU_SRSO
-	.section .text.__x86.rethunk_untrain
+	.section .text..__x86.rethunk_untrain
 
 SYM_START(srso_untrain_ret_alias, SYM_L_GLOBAL, SYM_A_NONE)
 	ANNOTATE_NOENDBR
@@ -158,7 +158,7 @@ SYM_START(srso_untrain_ret_alias, SYM_L_GLOBAL, SYM_A_NONE)
 SYM_FUNC_END(srso_untrain_ret_alias)
 __EXPORT_THUNK(srso_untrain_ret_alias)
 
-	.section .text.__x86.rethunk_safe
+	.section .text..__x86.rethunk_safe
 #endif
 
 /* Needs a definition for the __x86_return_thunk alternative below. */
@@ -172,7 +172,7 @@ SYM_START(srso_safe_ret_alias, SYM_L_GLOBAL, SYM_A_NONE)
 	int3
 SYM_FUNC_END(srso_safe_ret_alias)
 
-	.section .text.__x86.return_thunk
+	.section .text..__x86.return_thunk
 
 /*
  * Safety details here pertain to the AMD Zen{1,2} microarchitecture:
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 8936a05..e2ee10c 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -389,7 +389,7 @@ static int decode_instructions(struct objtool_file *file)
 		if (!strcmp(sec->name, ".noinstr.text") ||
 		    !strcmp(sec->name, ".entry.text") ||
 		    !strcmp(sec->name, ".cpuidle.text") ||
-		    !strncmp(sec->name, ".text.__x86.", 12))
+		    !strncmp(sec->name, ".text..__x86.", 13))
 			sec->noinstr = true;
 
 		/*

[tip: x86/urgent] x86/retpoline,kprobes: Skip optprobe check for indirect jumps with retpolines and IBT

[tip-bot2 for Petr Pavlu]

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

Commit-ID:     833fd800bf56b74d39d71d3f5936dffb3e0409c6
Gitweb:        https://git.kernel.org/tip/833fd800bf56b74d39d71d3f5936dffb3e0409c6
Author:        Petr Pavlu <petr.pavlu@suse.com>
AuthorDate:    Tue, 11 Jul 2023 11:19:52 +02:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Mon, 14 Aug 2023 11:46:51 +02:00

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: 0b53c374b9ef ("x86/retpoline: Use -mfunction-return")
Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Link: https://lore.kernel.org/r/20230711091952.27944-3-petr.pavlu@suse.com
---
 arch/x86/include/asm/nospec-branch.h |  3 +--
 arch/x86/kernel/kprobes/opt.c        | 40 ++++++++++-----------------
 arch/x86/kernel/vmlinux.lds.S        |  2 +-
 tools/perf/util/thread-stack.c       |  4 +---
 4 files changed, 17 insertions(+), 32 deletions(-)

diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 3faf044..e50db53 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -478,9 +478,6 @@ enum ssb_mitigation {
 	SPEC_STORE_BYPASS_SECCOMP,
 };
 
-extern char __indirect_thunk_start[];
-extern char __indirect_thunk_end[];
-
 static __always_inline
 void alternative_msr_write(unsigned int msr, u64 val, unsigned int feature)
 {
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 57b0037..517821b 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -226,7 +226,7 @@ static int copy_optimized_instructions(u8 *dest, u8 *src, u8 *real)
 }
 
 /* Check whether insn is indirect jump */
-static int __insn_is_indirect_jump(struct insn *insn)
+static int insn_is_indirect_jump(struct insn *insn)
 {
 	return ((insn->opcode.bytes[0] == 0xff &&
 		(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)
 	return (start <= target && target <= start + len);
 }
 
-static int insn_is_indirect_jump(struct insn *insn)
-{
-	int ret = __insn_is_indirect_jump(insn);
-
-#ifdef CONFIG_RETPOLINE
-	/*
-	 * Jump to x86_indirect_thunk_* is treated as an indirect jump.
-	 * Note that even with CONFIG_RETPOLINE=y, the kernel compiled with
-	 * older gcc may use indirect jump. So we add this check instead of
-	 * replace indirect-jump check.
-	 */
-	if (!ret)
-		ret = insn_jump_into_range(insn,
-				(unsigned long)__indirect_thunk_start,
-				(unsigned long)__indirect_thunk_end -
-				(unsigned long)__indirect_thunk_start);
-#endif
-	return ret;
-}
-
 /* Decode whole function to ensure any instructions don't jump into target */
 static int can_optimize(unsigned long paddr)
 {
@@ -334,9 +314,21 @@ static int can_optimize(unsigned long paddr)
 		/* Recover address */
 		insn.kaddr = (void *)addr;
 		insn.next_byte = (void *)(addr + insn.length);
-		/* Check any instructions don't jump into target */
-		if (insn_is_indirect_jump(&insn) ||
-		    insn_jump_into_range(&insn, paddr + INT3_INSN_SIZE,
+		/*
+		 * Check any instructions don't jump into target, indirectly or
+		 * directly.
+		 *
+		 * The indirect case is present to handle a code with jump
+		 * tables. When the kernel uses retpolines, the check should in
+		 * theory additionally look for jumps to indirect thunks.
+		 * However, the kernel built with retpolines or IBT has jump
+		 * tables disabled so the check can be skipped altogether.
+		 */
+		if (!IS_ENABLED(CONFIG_RETPOLINE) &&
+		    !IS_ENABLED(CONFIG_X86_KERNEL_IBT) &&
+		    insn_is_indirect_jump(&insn))
+			return 0;
+		if (insn_jump_into_range(&insn, paddr + INT3_INSN_SIZE,
 					 DISP32_SIZE))
 			return 0;
 		addr += insn.length;
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index dfb8783..8e2a306 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -133,10 +133,8 @@ SECTIONS
 		KPROBES_TEXT
 		SOFTIRQENTRY_TEXT
 #ifdef CONFIG_RETPOLINE
-		__indirect_thunk_start = .;
 		*(.text..__x86.indirect_thunk)
 		*(.text..__x86.return_thunk)
-		__indirect_thunk_end = .;
 #endif
 		STATIC_CALL_TEXT
 
diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
index 374d142..c6a0a27 100644
--- a/tools/perf/util/thread-stack.c
+++ b/tools/perf/util/thread-stack.c
@@ -1038,9 +1038,7 @@ static int thread_stack__trace_end(struct thread_stack *ts,
 
 static bool is_x86_retpoline(const char *name)
 {
-	const char *p = strstr(name, "__x86_indirect_thunk_");
-
-	return p == name || !strcmp(name, "__indirect_thunk_start");
+	return strstr(name, "__x86_indirect_thunk_") == name;
 }
 
 /*

Re: [PATCH] x86/retpoline,kprobes: Fix "Fix position of thunk sections with CONFIG_LTO_CLANG"

[Josh Poimboeuf]

On Fri, Aug 04, 2023 at 10:43:28AM +0200, Peter Zijlstra wrote:
> On Fri, Aug 04, 2023 at 10:28:53AM +0200, Peter Zijlstra wrote:
> > On Fri, Aug 04, 2023 at 12:03:23AM +0100, Andrew Cooper wrote:
> > > Alternatively,
> > > 
> > > int strstarts(const char *s1, const char *s2)
> > > {
> > >         return strncmp(s1, s2, strlen(s2));
> > > }
> > 
> > Durr, I hate C ;-/ And yes, we have a ton of it, lemme try that
> > strstarts thing.
> 
> So there, that builds and compiles a kernel, must be good :-)

Would be nice to see this strstarts() change go in ;-)

Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>

-- 
Josh