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

  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(-)
  

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.
  

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.
  

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")		||
  

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>