[PATCH 0/5] objtool fixes

[PATCH 0/5] objtool fixes

[Josh Poimboeuf]

Some objtool fixes related to CONFIG_UBSAN_TRAP, Clang assembler, and
more...

Josh Poimboeuf (5):
  objtool: Fix CONFIG_UBSAN_TRAP unreachable warnings
  objtool: Support Clang non-section symbols in ORC dump
  objtool: Support Clang non-section symbols in ORC generation
  objtool: Fix switch table detection in .text.unlikely
  objtool: Make BP scratch register warning more robust

 tools/objtool/check.c    | 26 ++++++++++++++++--------
 tools/objtool/orc_dump.c | 44 ++++++++++++++++++++++++----------------
 tools/objtool/orc_gen.c  | 33 +++++++++++++++++++++++-------
 3 files changed, 71 insertions(+), 32 deletions(-)

-- 
2.21.1

[PATCH 1/5] objtool: Fix CONFIG_UBSAN_TRAP unreachable warnings

[Josh Poimboeuf]

CONFIG_UBSAN_TRAP causes GCC to emit a UD2 whenever it encounters an
unreachable code path.  This includes __builtin_unreachable().  Because
the BUG() macro uses __builtin_unreachable() after it emits its own UD2,
this results in a double UD2.  In this case objtool rightfully detects
that the second UD2 is unreachable:

  init/main.o: warning: objtool: repair_env_string()+0x1c8: unreachable instruction

We weren't able to figure out a way to get rid of the double UD2s, so
just silence the warning.

Cc: Kees Cook <keescook@chromium.org>
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 tools/objtool/check.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index e3bb76358148..aaec5e1277ea 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2382,14 +2382,27 @@ static bool ignore_unreachable_insn(struct instruction *insn)
 	    !strcmp(insn->sec->name, ".altinstr_aux"))
 		return true;
 
+	if (!insn->func)
+		return false;
+
+	/*
+	 * CONFIG_UBSAN_TRAP inserts a UD2 when it sees
+	 * __builtin_unreachable().  The BUG() macro has an unreachable() after
+	 * the UD2, which causes GCC's undefined trap logic to emit another UD2
+	 * (or occasionally a JMP to UD2).
+	 */
+	if (list_prev_entry(insn, list)->dead_end &&
+	    (insn->type == INSN_BUG ||
+	     (insn->type == INSN_JUMP_UNCONDITIONAL &&
+	      insn->jump_dest && insn->jump_dest->type == INSN_BUG)))
+		return true;
+
 	/*
 	 * Check if this (or a subsequent) instruction is related to
 	 * CONFIG_UBSAN or CONFIG_KASAN.
 	 *
 	 * End the search at 5 instructions to avoid going into the weeds.
 	 */
-	if (!insn->func)
-		return false;
 	for (i = 0; i < 5; i++) {
 
 		if (is_kasan_insn(insn) || is_ubsan_insn(insn))
-- 
2.21.1

Re: [PATCH 1/5] objtool: Fix CONFIG_UBSAN_TRAP unreachable warnings

[Kees Cook]

On Wed, Apr 01, 2020 at 01:23:25PM -0500, Josh Poimboeuf wrote:
> CONFIG_UBSAN_TRAP causes GCC to emit a UD2 whenever it encounters an
> unreachable code path.  This includes __builtin_unreachable().  Because
> the BUG() macro uses __builtin_unreachable() after it emits its own UD2,
> this results in a double UD2.  In this case objtool rightfully detects
> that the second UD2 is unreachable:
> 
>   init/main.o: warning: objtool: repair_env_string()+0x1c8: unreachable instruction
> 
> We weren't able to figure out a way to get rid of the double UD2s, so
> just silence the warning.
> 
> Cc: Kees Cook <keescook@chromium.org>
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>

Thanks for finding a way to work around this!

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  tools/objtool/check.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index e3bb76358148..aaec5e1277ea 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -2382,14 +2382,27 @@ static bool ignore_unreachable_insn(struct instruction *insn)
>  	    !strcmp(insn->sec->name, ".altinstr_aux"))
>  		return true;
>  
> +	if (!insn->func)
> +		return false;
> +
> +	/*
> +	 * CONFIG_UBSAN_TRAP inserts a UD2 when it sees
> +	 * __builtin_unreachable().  The BUG() macro has an unreachable() after
> +	 * the UD2, which causes GCC's undefined trap logic to emit another UD2
> +	 * (or occasionally a JMP to UD2).
> +	 */
> +	if (list_prev_entry(insn, list)->dead_end &&
> +	    (insn->type == INSN_BUG ||
> +	     (insn->type == INSN_JUMP_UNCONDITIONAL &&
> +	      insn->jump_dest && insn->jump_dest->type == INSN_BUG)))
> +		return true;
> +
>  	/*
>  	 * Check if this (or a subsequent) instruction is related to
>  	 * CONFIG_UBSAN or CONFIG_KASAN.
>  	 *
>  	 * End the search at 5 instructions to avoid going into the weeds.
>  	 */
> -	if (!insn->func)
> -		return false;
>  	for (i = 0; i < 5; i++) {
>  
>  		if (is_kasan_insn(insn) || is_ubsan_insn(insn))
> -- 
> 2.21.1
> 

-- 
Kees Cook

[tip: x86/urgent] objtool: Fix CONFIG_UBSAN_TRAP unreachable warnings

[tip-bot2 for Josh Poimboeuf]

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

Commit-ID:     bd841d6154f5f41f8a32d3c1b0bc229e326e640a
Gitweb:        https://git.kernel.org/tip/bd841d6154f5f41f8a32d3c1b0bc229e326e640a
Author:        Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate:    Wed, 01 Apr 2020 13:23:25 -05:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Tue, 14 Apr 2020 11:55:09 +02:00

objtool: Fix CONFIG_UBSAN_TRAP unreachable warnings

CONFIG_UBSAN_TRAP causes GCC to emit a UD2 whenever it encounters an
unreachable code path.  This includes __builtin_unreachable().  Because
the BUG() macro uses __builtin_unreachable() after it emits its own UD2,
this results in a double UD2.  In this case objtool rightfully detects
that the second UD2 is unreachable:

  init/main.o: warning: objtool: repair_env_string()+0x1c8: unreachable instruction

We weren't able to figure out a way to get rid of the double UD2s, so
just silence the warning.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Miroslav Benes <mbenes@suse.cz>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/6653ad73c6b59c049211bd7c11ed3809c20ee9f5.1585761021.git.jpoimboe@redhat.com
---
 tools/objtool/check.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 8dd01f9..4811325 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2364,14 +2364,27 @@ static bool ignore_unreachable_insn(struct instruction *insn)
 	    !strcmp(insn->sec->name, ".altinstr_aux"))
 		return true;
 
+	if (!insn->func)
+		return false;
+
+	/*
+	 * CONFIG_UBSAN_TRAP inserts a UD2 when it sees
+	 * __builtin_unreachable().  The BUG() macro has an unreachable() after
+	 * the UD2, which causes GCC's undefined trap logic to emit another UD2
+	 * (or occasionally a JMP to UD2).
+	 */
+	if (list_prev_entry(insn, list)->dead_end &&
+	    (insn->type == INSN_BUG ||
+	     (insn->type == INSN_JUMP_UNCONDITIONAL &&
+	      insn->jump_dest && insn->jump_dest->type == INSN_BUG)))
+		return true;
+
 	/*
 	 * Check if this (or a subsequent) instruction is related to
 	 * CONFIG_UBSAN or CONFIG_KASAN.
 	 *
 	 * End the search at 5 instructions to avoid going into the weeds.
 	 */
-	if (!insn->func)
-		return false;
 	for (i = 0; i < 5; i++) {
 
 		if (is_kasan_insn(insn) || is_ubsan_insn(insn))

[PATCH 2/5] objtool: Support Clang non-section symbols in ORC dump

[Josh Poimboeuf]

Historically, the relocation symbols for ORC entries have only been
section symbols:

  .text+0: sp:sp+8 bp:(und) type:call end:0

However, the Clang assembler is aggressive about stripping section
symbols.  In that case we will need to use function symbols:

  freezing_slow_path+0: sp:sp+8 bp:(und) type:call end:0

In preparation for the generation of such entries in "objtool orc
generate", add support for reading them in "objtool orc dump".

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 tools/objtool/orc_dump.c | 44 ++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/tools/objtool/orc_dump.c b/tools/objtool/orc_dump.c
index 13ccf775a83a..ba4cbb1cdd63 100644
--- a/tools/objtool/orc_dump.c
+++ b/tools/objtool/orc_dump.c
@@ -66,7 +66,7 @@ int orc_dump(const char *_objname)
 	char *name;
 	size_t nr_sections;
 	Elf64_Addr orc_ip_addr = 0;
-	size_t shstrtab_idx;
+	size_t shstrtab_idx, strtab_idx = 0;
 	Elf *elf;
 	Elf_Scn *scn;
 	GElf_Shdr sh;
@@ -127,6 +127,8 @@ int orc_dump(const char *_objname)
 
 		if (!strcmp(name, ".symtab")) {
 			symtab = data;
+		} else if (!strcmp(name, ".strtab")) {
+			strtab_idx = i;
 		} else if (!strcmp(name, ".orc_unwind")) {
 			orc = data->d_buf;
 			orc_size = sh.sh_size;
@@ -138,7 +140,7 @@ int orc_dump(const char *_objname)
 		}
 	}
 
-	if (!symtab || !orc || !orc_ip)
+	if (!symtab || !strtab_idx || !orc || !orc_ip)
 		return 0;
 
 	if (orc_size % sizeof(*orc) != 0) {
@@ -159,21 +161,29 @@ int orc_dump(const char *_objname)
 				return -1;
 			}
 
-			scn = elf_getscn(elf, sym.st_shndx);
-			if (!scn) {
-				WARN_ELF("elf_getscn");
-				return -1;
-			}
-
-			if (!gelf_getshdr(scn, &sh)) {
-				WARN_ELF("gelf_getshdr");
-				return -1;
-			}
-
-			name = elf_strptr(elf, shstrtab_idx, sh.sh_name);
-			if (!name || !*name) {
-				WARN_ELF("elf_strptr");
-				return -1;
+			if (GELF_ST_TYPE(sym.st_info) == STT_SECTION) {
+				scn = elf_getscn(elf, sym.st_shndx);
+				if (!scn) {
+					WARN_ELF("elf_getscn");
+					return -1;
+				}
+
+				if (!gelf_getshdr(scn, &sh)) {
+					WARN_ELF("gelf_getshdr");
+					return -1;
+				}
+
+				name = elf_strptr(elf, shstrtab_idx, sh.sh_name);
+				if (!name) {
+					WARN_ELF("elf_strptr");
+					return -1;
+				}
+			} else {
+				name = elf_strptr(elf, strtab_idx, sym.st_name);
+				if (!name) {
+					WARN_ELF("elf_strptr");
+					return -1;
+				}
 			}
 
 			printf("%s+%llx:", name, (unsigned long long)rela.r_addend);
-- 
2.21.1

[tip: x86/urgent] objtool: Support Clang non-section symbols in ORC dump

[tip-bot2 for Josh Poimboeuf]

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

Commit-ID:     8782e7cab51b6bf01a5a86471dd82228af1ac185
Gitweb:        https://git.kernel.org/tip/8782e7cab51b6bf01a5a86471dd82228af1ac185
Author:        Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate:    Wed, 01 Apr 2020 13:23:26 -05:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Tue, 14 Apr 2020 11:59:52 +02:00

objtool: Support Clang non-section symbols in ORC dump

Historically, the relocation symbols for ORC entries have only been
section symbols:

  .text+0: sp:sp+8 bp:(und) type:call end:0

However, the Clang assembler is aggressive about stripping section
symbols.  In that case we will need to use function symbols:

  freezing_slow_path+0: sp:sp+8 bp:(und) type:call end:0

In preparation for the generation of such entries in "objtool orc
generate", add support for reading them in "objtool orc dump".

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Miroslav Benes <mbenes@suse.cz>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/b811b5eb1a42602c3b523576dc5efab9ad1c174d.1585761021.git.jpoimboe@redhat.com
---
 tools/objtool/orc_dump.c | 44 +++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/tools/objtool/orc_dump.c b/tools/objtool/orc_dump.c
index 13ccf77..ba4cbb1 100644
--- a/tools/objtool/orc_dump.c
+++ b/tools/objtool/orc_dump.c
@@ -66,7 +66,7 @@ int orc_dump(const char *_objname)
 	char *name;
 	size_t nr_sections;
 	Elf64_Addr orc_ip_addr = 0;
-	size_t shstrtab_idx;
+	size_t shstrtab_idx, strtab_idx = 0;
 	Elf *elf;
 	Elf_Scn *scn;
 	GElf_Shdr sh;
@@ -127,6 +127,8 @@ int orc_dump(const char *_objname)
 
 		if (!strcmp(name, ".symtab")) {
 			symtab = data;
+		} else if (!strcmp(name, ".strtab")) {
+			strtab_idx = i;
 		} else if (!strcmp(name, ".orc_unwind")) {
 			orc = data->d_buf;
 			orc_size = sh.sh_size;
@@ -138,7 +140,7 @@ int orc_dump(const char *_objname)
 		}
 	}
 
-	if (!symtab || !orc || !orc_ip)
+	if (!symtab || !strtab_idx || !orc || !orc_ip)
 		return 0;
 
 	if (orc_size % sizeof(*orc) != 0) {
@@ -159,21 +161,29 @@ int orc_dump(const char *_objname)
 				return -1;
 			}
 
-			scn = elf_getscn(elf, sym.st_shndx);
-			if (!scn) {
-				WARN_ELF("elf_getscn");
-				return -1;
-			}
-
-			if (!gelf_getshdr(scn, &sh)) {
-				WARN_ELF("gelf_getshdr");
-				return -1;
-			}
-
-			name = elf_strptr(elf, shstrtab_idx, sh.sh_name);
-			if (!name || !*name) {
-				WARN_ELF("elf_strptr");
-				return -1;
+			if (GELF_ST_TYPE(sym.st_info) == STT_SECTION) {
+				scn = elf_getscn(elf, sym.st_shndx);
+				if (!scn) {
+					WARN_ELF("elf_getscn");
+					return -1;
+				}
+
+				if (!gelf_getshdr(scn, &sh)) {
+					WARN_ELF("gelf_getshdr");
+					return -1;
+				}
+
+				name = elf_strptr(elf, shstrtab_idx, sh.sh_name);
+				if (!name) {
+					WARN_ELF("elf_strptr");
+					return -1;
+				}
+			} else {
+				name = elf_strptr(elf, strtab_idx, sym.st_name);
+				if (!name) {
+					WARN_ELF("elf_strptr");
+					return -1;
+				}
 			}
 
 			printf("%s+%llx:", name, (unsigned long long)rela.r_addend);

[PATCH 3/5] objtool: Support Clang non-section symbols in ORC generation

[Josh Poimboeuf]

When compiling the kernel with AS=clang, objtool produces a lot of
warnings:

  warning: objtool: missing symbol for section .text
  warning: objtool: missing symbol for section .init.text
  warning: objtool: missing symbol for section .ref.text

It then fails to generate the ORC table.

The problem is that objtool assumes text section symbols always exist.
But the Clang assembler is aggressive about removing them.

When generating relocations for the ORC table, objtool always tries to
reference instructions by their section symbol offset.  If the section
symbol doesn't exist, it bails.

Do a fallback: when a section symbol isn't available, reference a
function symbol instead.

Link: https://github.com/ClangBuiltLinux/linux/issues/669
Cc: Nick Desaulniers <ndesaulniers@google.com>
Reported-by: Dmitry Golovin <dima@golovin.in>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 tools/objtool/orc_gen.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c
index 41e4a2754da4..4c0dabd28000 100644
--- a/tools/objtool/orc_gen.c
+++ b/tools/objtool/orc_gen.c
@@ -88,11 +88,6 @@ static int create_orc_entry(struct elf *elf, struct section *u_sec, struct secti
 	struct orc_entry *orc;
 	struct rela *rela;
 
-	if (!insn_sec->sym) {
-		WARN("missing symbol for section %s", insn_sec->name);
-		return -1;
-	}
-
 	/* populate ORC data */
 	orc = (struct orc_entry *)u_sec->data->d_buf + idx;
 	memcpy(orc, o, sizeof(*orc));
@@ -105,8 +100,32 @@ static int create_orc_entry(struct elf *elf, struct section *u_sec, struct secti
 	}
 	memset(rela, 0, sizeof(*rela));
 
-	rela->sym = insn_sec->sym;
-	rela->addend = insn_off;
+	if (insn_sec->sym) {
+		rela->sym = insn_sec->sym;
+		rela->addend = insn_off;
+	} else {
+		/*
+		 * The Clang assembler doesn't produce section symbols, so we
+		 * have to reference the function symbol instead:
+		 */
+		rela->sym = find_symbol_containing(insn_sec, insn_off);
+		if (!rela->sym) {
+			/*
+			 * Hack alert.  This happens when we need to reference
+			 * the NOP pad insn immediately after the function.
+			 */
+			rela->sym = find_symbol_containing(insn_sec,
+							   insn_off - 1);
+		}
+		if (!rela->sym) {
+			WARN("missing symbol for insn at offset 0x%lx\n",
+			     insn_off);
+			return -1;
+		}
+
+		rela->addend = insn_off - rela->sym->offset;
+	}
+
 	rela->type = R_X86_64_PC32;
 	rela->offset = idx * sizeof(int);
 	rela->sec = ip_relasec;
-- 
2.21.1

Re: [PATCH 3/5] objtool: Support Clang non-section symbols in ORC generation

[Peter Zijlstra]

On Wed, Apr 01, 2020 at 01:23:27PM -0500, Josh Poimboeuf wrote:

> @@ -105,8 +100,32 @@ static int create_orc_entry(struct elf *elf, struct section *u_sec, struct secti
>  	}
>  	memset(rela, 0, sizeof(*rela));
>  
> -	rela->sym = insn_sec->sym;
> -	rela->addend = insn_off;
> +	if (insn_sec->sym) {
> +		rela->sym = insn_sec->sym;
> +		rela->addend = insn_off;
> +	} else {
> +		/*
> +		 * The Clang assembler doesn't produce section symbols, so we
> +		 * have to reference the function symbol instead:
> +		 */
> +		rela->sym = find_symbol_containing(insn_sec, insn_off);

It's a good thing I made that a lot faster I suppose ;-)

> +		if (!rela->sym) {
> +			/*
> +			 * Hack alert.  This happens when we need to reference
> +			 * the NOP pad insn immediately after the function.
> +			 */
> +			rela->sym = find_symbol_containing(insn_sec,
> +							   insn_off - 1);

Urgh, when does that happen? 

> +		}
> +		if (!rela->sym) {
> +			WARN("missing symbol for insn at offset 0x%lx\n",
> +			     insn_off);
> +			return -1;
> +		}
> +
> +		rela->addend = insn_off - rela->sym->offset;
> +	}
> +
>  	rela->type = R_X86_64_PC32;
>  	rela->offset = idx * sizeof(int);
>  	rela->sec = ip_relasec;
> -- 
> 2.21.1
> 

Re: [PATCH 3/5] objtool: Support Clang non-section symbols in ORC generation

[Josh Poimboeuf]

On Wed, Apr 01, 2020 at 08:49:53PM +0200, Peter Zijlstra wrote:
> On Wed, Apr 01, 2020 at 01:23:27PM -0500, Josh Poimboeuf wrote:
> 
> > @@ -105,8 +100,32 @@ static int create_orc_entry(struct elf *elf, struct section *u_sec, struct secti
> >  	}
> >  	memset(rela, 0, sizeof(*rela));
> >  
> > -	rela->sym = insn_sec->sym;
> > -	rela->addend = insn_off;
> > +	if (insn_sec->sym) {
> > +		rela->sym = insn_sec->sym;
> > +		rela->addend = insn_off;
> > +	} else {
> > +		/*
> > +		 * The Clang assembler doesn't produce section symbols, so we
> > +		 * have to reference the function symbol instead:
> > +		 */
> > +		rela->sym = find_symbol_containing(insn_sec, insn_off);
> 
> It's a good thing I made that a lot faster I suppose ;-)

:-)

> > +		if (!rela->sym) {
> > +			/*
> > +			 * Hack alert.  This happens when we need to reference
> > +			 * the NOP pad insn immediately after the function.
> > +			 */
> > +			rela->sym = find_symbol_containing(insn_sec,
> > +							   insn_off - 1);
> 
> Urgh, when does that happen? 

It happens naturally in the padding between functions, since objtool
doesn't traverse those instructions.  So they have undefined entries
like

 .text+68: sp:(und) bp:(und) type:call end:0

I suppose those aren't technically necessary.

-- 
Josh

Re: [PATCH 3/5] objtool: Support Clang non-section symbols in ORC generation

[Josh Poimboeuf]

On Wed, Apr 01, 2020 at 02:05:51PM -0500, Josh Poimboeuf wrote:
> On Wed, Apr 01, 2020 at 08:49:53PM +0200, Peter Zijlstra wrote:
> > On Wed, Apr 01, 2020 at 01:23:27PM -0500, Josh Poimboeuf wrote:
> > 
> > > @@ -105,8 +100,32 @@ static int create_orc_entry(struct elf *elf, struct section *u_sec, struct secti
> > >  	}
> > >  	memset(rela, 0, sizeof(*rela));
> > >  
> > > -	rela->sym = insn_sec->sym;
> > > -	rela->addend = insn_off;
> > > +	if (insn_sec->sym) {
> > > +		rela->sym = insn_sec->sym;
> > > +		rela->addend = insn_off;
> > > +	} else {
> > > +		/*
> > > +		 * The Clang assembler doesn't produce section symbols, so we
> > > +		 * have to reference the function symbol instead:
> > > +		 */
> > > +		rela->sym = find_symbol_containing(insn_sec, insn_off);
> > 
> > It's a good thing I made that a lot faster I suppose ;-)
> 
> :-)
> 
> > > +		if (!rela->sym) {
> > > +			/*
> > > +			 * Hack alert.  This happens when we need to reference
> > > +			 * the NOP pad insn immediately after the function.
> > > +			 */
> > > +			rela->sym = find_symbol_containing(insn_sec,
> > > +							   insn_off - 1);
> > 
> > Urgh, when does that happen? 
> 
> It happens naturally in the padding between functions, since objtool
> doesn't traverse those instructions.  So they have undefined entries
> like
> 
>  .text+68: sp:(und) bp:(und) type:call end:0
> 
> I suppose those aren't technically necessary.

In fact, we could probably get substantial savings in the ORC table if
we skipped those, i.e.

  .text+0: sp:sp+8 bp:(und) type:call end:0
  .text+8: sp:(und) bp:(und) type:call end:0
  .text+10: sp:sp+8 bp:(und) type:call end:0
  .text+17: sp:sp+16 bp:(und) type:call end:0
  .text+18: sp:sp+24 bp:prevsp-24 type:call end:0
  .text+1c: sp:sp+32 bp:prevsp-24 type:call end:0
  .text+5a: sp:sp+24 bp:prevsp-24 type:call end:0
  .text+61: sp:sp+16 bp:(und) type:call end:0
  .text+63: sp:sp+8 bp:(und) type:call end:0
  .text+68: sp:(und) bp:(und) type:call end:0
  .text+70: sp:sp+8 bp:(und) type:call end:0
  .text+8c: sp:(und) bp:(und) type:call end:0
  .text+90: sp:sp+8 bp:(und) type:call end:0
  .text+cd: sp:(und) bp:(und) type:call end:0
  .text+d0: sp:sp+8 bp:(und) type:call end:0

would be compressed to

  .text+0: sp:sp+8 bp:(und) type:call end:0
  .text+17: sp:sp+16 bp:(und) type:call end:0
  .text+18: sp:sp+24 bp:prevsp-24 type:call end:0
  .text+1c: sp:sp+32 bp:prevsp-24 type:call end:0
  .text+5a: sp:sp+24 bp:prevsp-24 type:call end:0
  .text+61: sp:sp+16 bp:(und) type:call end:0
  .text+63: sp:sp+8 bp:(und) type:call end:0

but I can do that in a separate patch, and if it works I can remove this
hack.

-- 
Josh

Re: [PATCH 3/5] objtool: Support Clang non-section symbols in ORC generation

[Miroslav Benes]

On Wed, 1 Apr 2020, Josh Poimboeuf wrote:

> When compiling the kernel with AS=clang, objtool produces a lot of
> warnings:
> 
>   warning: objtool: missing symbol for section .text
>   warning: objtool: missing symbol for section .init.text
>   warning: objtool: missing symbol for section .ref.text
> 
> It then fails to generate the ORC table.
> 
> The problem is that objtool assumes text section symbols always exist.
> But the Clang assembler is aggressive about removing them.
> 
> When generating relocations for the ORC table, objtool always tries to
> reference instructions by their section symbol offset.  If the section
> symbol doesn't exist, it bails.
> 
> Do a fallback: when a section symbol isn't available, reference a
> function symbol instead.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/669
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Reported-by: Dmitry Golovin <dima@golovin.in>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
>  tools/objtool/orc_gen.c | 33 ++++++++++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c
> index 41e4a2754da4..4c0dabd28000 100644
> --- a/tools/objtool/orc_gen.c
> +++ b/tools/objtool/orc_gen.c
> @@ -88,11 +88,6 @@ static int create_orc_entry(struct elf *elf, struct section *u_sec, struct secti
>  	struct orc_entry *orc;
>  	struct rela *rela;
>  
> -	if (!insn_sec->sym) {
> -		WARN("missing symbol for section %s", insn_sec->name);
> -		return -1;
> -	}
> -
>  	/* populate ORC data */
>  	orc = (struct orc_entry *)u_sec->data->d_buf + idx;
>  	memcpy(orc, o, sizeof(*orc));
> @@ -105,8 +100,32 @@ static int create_orc_entry(struct elf *elf, struct section *u_sec, struct secti
>  	}
>  	memset(rela, 0, sizeof(*rela));
>  
> -	rela->sym = insn_sec->sym;
> -	rela->addend = insn_off;
> +	if (insn_sec->sym) {
> +		rela->sym = insn_sec->sym;
> +		rela->addend = insn_off;
> +	} else {
> +		/*
> +		 * The Clang assembler doesn't produce section symbols, so we
> +		 * have to reference the function symbol instead:
> +		 */
> +		rela->sym = find_symbol_containing(insn_sec, insn_off);
> +		if (!rela->sym) {
> +			/*
> +			 * Hack alert.  This happens when we need to reference
> +			 * the NOP pad insn immediately after the function.
> +			 */
> +			rela->sym = find_symbol_containing(insn_sec,
> +							   insn_off - 1);
> +		}

I suppose there is always just one NOP pad insn, right? Anyway, it would 
be better to get rid of it as you proposed.

> +		if (!rela->sym) {
> +			WARN("missing symbol for insn at offset 0x%lx\n",
> +			     insn_off);
> +			return -1;
> +		}
> +
> +		rela->addend = insn_off - rela->sym->offset;
> +	}
> +
>  	rela->type = R_X86_64_PC32;
>  	rela->offset = idx * sizeof(int);
>  	rela->sec = ip_relasec;

Miroslav

Re: [PATCH 3/5] objtool: Support Clang non-section symbols in ORC generation

[Josh Poimboeuf]

On Fri, Apr 03, 2020 at 10:58:20AM +0200, Miroslav Benes wrote:
> > +	if (insn_sec->sym) {
> > +		rela->sym = insn_sec->sym;
> > +		rela->addend = insn_off;
> > +	} else {
> > +		/*
> > +		 * The Clang assembler doesn't produce section symbols, so we
> > +		 * have to reference the function symbol instead:
> > +		 */
> > +		rela->sym = find_symbol_containing(insn_sec, insn_off);
> > +		if (!rela->sym) {
> > +			/*
> > +			 * Hack alert.  This happens when we need to reference
> > +			 * the NOP pad insn immediately after the function.
> > +			 */
> > +			rela->sym = find_symbol_containing(insn_sec,
> > +							   insn_off - 1);
> > +		}
> 
> I suppose there is always just one NOP pad insn, right? Anyway, it would 
> be better to get rid of it as you proposed.

There can actually be multiple NOPs because functions are aligned on a
16-byte boundary.  But the undefined ORC entry is always at the first
NOP because objtool merges duplicate entries.

-- 
Josh

[tip: x86/urgent] objtool: Support Clang non-section symbols in ORC generation

[tip-bot2 for Josh Poimboeuf]

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

Commit-ID:     e81e0724432542af8d8c702c31e9d82f57b1ff31
Gitweb:        https://git.kernel.org/tip/e81e0724432542af8d8c702c31e9d82f57b1ff31
Author:        Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate:    Wed, 01 Apr 2020 13:23:27 -05:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Tue, 14 Apr 2020 12:03:42 +02:00

objtool: Support Clang non-section symbols in ORC generation

When compiling the kernel with AS=clang, objtool produces a lot of
warnings:

  warning: objtool: missing symbol for section .text
  warning: objtool: missing symbol for section .init.text
  warning: objtool: missing symbol for section .ref.text

It then fails to generate the ORC table.

The problem is that objtool assumes text section symbols always exist.
But the Clang assembler is aggressive about removing them.

When generating relocations for the ORC table, objtool always tries to
reference instructions by their section symbol offset.  If the section
symbol doesn't exist, it bails.

Do a fallback: when a section symbol isn't available, reference a
function symbol instead.

Reported-by: Dmitry Golovin <dima@golovin.in>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Reviewed-by: Miroslav Benes <mbenes@suse.cz>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://github.com/ClangBuiltLinux/linux/issues/669
Link: https://lkml.kernel.org/r/9a9cae7fcf628843aabe5a086b1a3c5bf50f42e8.1585761021.git.jpoimboe@redhat.com
---
 tools/objtool/orc_gen.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c
index 41e4a27..4c0dabd 100644
--- a/tools/objtool/orc_gen.c
+++ b/tools/objtool/orc_gen.c
@@ -88,11 +88,6 @@ static int create_orc_entry(struct elf *elf, struct section *u_sec, struct secti
 	struct orc_entry *orc;
 	struct rela *rela;
 
-	if (!insn_sec->sym) {
-		WARN("missing symbol for section %s", insn_sec->name);
-		return -1;
-	}
-
 	/* populate ORC data */
 	orc = (struct orc_entry *)u_sec->data->d_buf + idx;
 	memcpy(orc, o, sizeof(*orc));
@@ -105,8 +100,32 @@ static int create_orc_entry(struct elf *elf, struct section *u_sec, struct secti
 	}
 	memset(rela, 0, sizeof(*rela));
 
-	rela->sym = insn_sec->sym;
-	rela->addend = insn_off;
+	if (insn_sec->sym) {
+		rela->sym = insn_sec->sym;
+		rela->addend = insn_off;
+	} else {
+		/*
+		 * The Clang assembler doesn't produce section symbols, so we
+		 * have to reference the function symbol instead:
+		 */
+		rela->sym = find_symbol_containing(insn_sec, insn_off);
+		if (!rela->sym) {
+			/*
+			 * Hack alert.  This happens when we need to reference
+			 * the NOP pad insn immediately after the function.
+			 */
+			rela->sym = find_symbol_containing(insn_sec,
+							   insn_off - 1);
+		}
+		if (!rela->sym) {
+			WARN("missing symbol for insn at offset 0x%lx\n",
+			     insn_off);
+			return -1;
+		}
+
+		rela->addend = insn_off - rela->sym->offset;
+	}
+
 	rela->type = R_X86_64_PC32;
 	rela->offset = idx * sizeof(int);
 	rela->sec = ip_relasec;

[PATCH 4/5] objtool: Fix switch table detection in .text.unlikely

[Josh Poimboeuf]

If a switch jump table's indirect branch is in a ".cold" subfunction in
.text.unlikely, objtool doesn't detect it, and instead prints a false
warning:

  drivers/media/v4l2-core/v4l2-ioctl.o: warning: objtool: v4l_print_format.cold()+0xd6: sibling call from callable instruction with modified stack frame
  drivers/hwmon/max6650.o: warning: objtool: max6650_probe.cold()+0xa5: sibling call from callable instruction with modified stack frame
  drivers/media/dvb-frontends/drxk_hard.o: warning: objtool: init_drxk.cold()+0x16f: sibling call from callable instruction with modified stack frame

Fix it by comparing the function, instead of the section and offset.

Fixes: 13810435b9a7 ("objtool: Support GCC 8's cold subfunctions")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 tools/objtool/check.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index aaec5e1277ea..c681a26c25ac 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1068,10 +1068,7 @@ static struct rela *find_jump_table(struct objtool_file *file,
 	 * it.
 	 */
 	for (;
-	     &insn->list != &file->insn_list &&
-	     insn->sec == func->sec &&
-	     insn->offset >= func->offset;
-
+	     &insn->list != &file->insn_list && insn->func && insn->func->pfunc == func;
 	     insn = insn->first_jump_src ?: list_prev_entry(insn, list)) {
 
 		if (insn != orig_insn && insn->type == INSN_JUMP_DYNAMIC)
-- 
2.21.1

[tip: x86/urgent] objtool: Fix switch table detection in .text.unlikely

[tip-bot2 for Josh Poimboeuf]

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

Commit-ID:     b401efc120a399dfda1f4d2858a4de365c9b08ef
Gitweb:        https://git.kernel.org/tip/b401efc120a399dfda1f4d2858a4de365c9b08ef
Author:        Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate:    Wed, 01 Apr 2020 13:23:28 -05:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Tue, 14 Apr 2020 12:06:21 +02:00

objtool: Fix switch table detection in .text.unlikely

If a switch jump table's indirect branch is in a ".cold" subfunction in
.text.unlikely, objtool doesn't detect it, and instead prints a false
warning:

  drivers/media/v4l2-core/v4l2-ioctl.o: warning: objtool: v4l_print_format.cold()+0xd6: sibling call from callable instruction with modified stack frame
  drivers/hwmon/max6650.o: warning: objtool: max6650_probe.cold()+0xa5: sibling call from callable instruction with modified stack frame
  drivers/media/dvb-frontends/drxk_hard.o: warning: objtool: init_drxk.cold()+0x16f: sibling call from callable instruction with modified stack frame

Fix it by comparing the function, instead of the section and offset.

Fixes: 13810435b9a7 ("objtool: Support GCC 8's cold subfunctions")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Miroslav Benes <mbenes@suse.cz>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/157c35d42ca9b6354bbb1604fe9ad7d1153ccb21.1585761021.git.jpoimboe@redhat.com
---
 tools/objtool/check.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 4811325..cb2d299 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1050,10 +1050,7 @@ static struct rela *find_jump_table(struct objtool_file *file,
 	 * it.
 	 */
 	for (;
-	     &insn->list != &file->insn_list &&
-	     insn->sec == func->sec &&
-	     insn->offset >= func->offset;
-
+	     &insn->list != &file->insn_list && insn->func && insn->func->pfunc == func;
 	     insn = insn->first_jump_src ?: list_prev_entry(insn, list)) {
 
 		if (insn != orig_insn && insn->type == INSN_JUMP_DYNAMIC)

[PATCH 5/5] objtool: Make BP scratch register warning more robust

[Josh Poimboeuf]

If func is NULL, a seg fault can result.

This is a theoretical issue which was found by Coverity.

Fixes: c705cecc8431 ("objtool: Track original function across branches")
Addresses-Coverity-ID: 1492002 ("Dereference after null check")
Reported-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 tools/objtool/check.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index c681a26c25ac..93fa7be67e9f 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2023,8 +2023,8 @@ static int validate_return(struct symbol *func, struct instruction *insn, struct
 	}
 
 	if (state->bp_scratch) {
-		WARN("%s uses BP as a scratch register",
-		     func->name);
+		WARN_FUNC("BP used as a scratch register",
+			  insn->sec, insn->offset);
 		return 1;
 	}
 
-- 
2.21.1

[tip: x86/urgent] objtool: Make BP scratch register warning more robust

[tip-bot2 for Josh Poimboeuf]

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

Commit-ID:     b296695298d8632d8b703ac25fe70be34a07c0d9
Gitweb:        https://git.kernel.org/tip/b296695298d8632d8b703ac25fe70be34a07c0d9
Author:        Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate:    Wed, 01 Apr 2020 13:23:29 -05:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Tue, 14 Apr 2020 12:24:22 +02:00

objtool: Make BP scratch register warning more robust

If func is NULL, a seg fault can result.

This is a theoretical issue which was found by Coverity, ID: 1492002
("Dereference after null check").

Fixes: c705cecc8431 ("objtool: Track original function across branches")
Reported-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Link: https://lkml.kernel.org/r/afc628693a37acd287e843bcc5c0430263d93c74.1585761021.git.jpoimboe@redhat.com
---
 tools/objtool/check.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index cb2d299..4b170fd 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2005,8 +2005,8 @@ static int validate_return(struct symbol *func, struct instruction *insn, struct
 	}
 
 	if (state->bp_scratch) {
-		WARN("%s uses BP as a scratch register",
-		     func->name);
+		WARN_FUNC("BP used as a scratch register",
+			  insn->sec, insn->offset);
 		return 1;
 	}
 

Re: [PATCH 0/5] objtool fixes

[Peter Zijlstra]

On Wed, Apr 01, 2020 at 01:23:24PM -0500, Josh Poimboeuf wrote:
> Some objtool fixes related to CONFIG_UBSAN_TRAP, Clang assembler, and
> more...
> 
> Josh Poimboeuf (5):
>   objtool: Fix CONFIG_UBSAN_TRAP unreachable warnings
>   objtool: Support Clang non-section symbols in ORC dump
>   objtool: Support Clang non-section symbols in ORC generation
>   objtool: Fix switch table detection in .text.unlikely
>   objtool: Make BP scratch register warning more robust
> 
>  tools/objtool/check.c    | 26 ++++++++++++++++--------
>  tools/objtool/orc_dump.c | 44 ++++++++++++++++++++++++----------------
>  tools/objtool/orc_gen.c  | 33 +++++++++++++++++++++++-------
>  3 files changed, 71 insertions(+), 32 deletions(-)

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Re: [PATCH 0/5] objtool fixes

[Miroslav Benes]

On Wed, 1 Apr 2020, Josh Poimboeuf wrote:

> Some objtool fixes related to CONFIG_UBSAN_TRAP, Clang assembler, and
> more...
> 
> Josh Poimboeuf (5):
>   objtool: Fix CONFIG_UBSAN_TRAP unreachable warnings
>   objtool: Support Clang non-section symbols in ORC dump
>   objtool: Support Clang non-section symbols in ORC generation
>   objtool: Fix switch table detection in .text.unlikely
>   objtool: Make BP scratch register warning more robust
> 
>  tools/objtool/check.c    | 26 ++++++++++++++++--------
>  tools/objtool/orc_dump.c | 44 ++++++++++++++++++++++++----------------
>  tools/objtool/orc_gen.c  | 33 +++++++++++++++++++++++-------
>  3 files changed, 71 insertions(+), 32 deletions(-)

Reviewed-by: Miroslav Benes <mbenes@suse.cz>

M

Re: [PATCH 0/5] objtool fixes

[Josh Poimboeuf]

Hi x86 maintainers,

Ping?

On Wed, Apr 01, 2020 at 01:23:24PM -0500, Josh Poimboeuf wrote:
> Some objtool fixes related to CONFIG_UBSAN_TRAP, Clang assembler, and
> more...
> 
> Josh Poimboeuf (5):
>   objtool: Fix CONFIG_UBSAN_TRAP unreachable warnings
>   objtool: Support Clang non-section symbols in ORC dump
>   objtool: Support Clang non-section symbols in ORC generation
>   objtool: Fix switch table detection in .text.unlikely
>   objtool: Make BP scratch register warning more robust
> 
>  tools/objtool/check.c    | 26 ++++++++++++++++--------
>  tools/objtool/orc_dump.c | 44 ++++++++++++++++++++++++----------------
>  tools/objtool/orc_gen.c  | 33 +++++++++++++++++++++++-------
>  3 files changed, 71 insertions(+), 32 deletions(-)

-- 
Josh