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
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 <[email protected]>
---
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
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 <[email protected]>
Reported-by: Dmitry Golovin <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
---
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
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 <[email protected]>
Reported-by: Randy Dunlap <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
---
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
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
>
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
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
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 <[email protected]>
> Reported-by: Randy Dunlap <[email protected]>
> Signed-off-by: Josh Poimboeuf <[email protected]>
Thanks for finding a way to work around this!
Reviewed-by: Kees Cook <[email protected]>
-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
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) <[email protected]>
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 <[email protected]>
> Reported-by: Dmitry Golovin <[email protected]>
> Tested-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Josh Poimboeuf <[email protected]>
> ---
> 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
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 <[email protected]>
M
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
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
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 <[email protected]>
AuthorDate: Wed, 01 Apr 2020 13:23:26 -05:00
Committer: Borislav Petkov <[email protected]>
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 <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Reviewed-by: Miroslav Benes <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
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);
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 <[email protected]>
AuthorDate: Wed, 01 Apr 2020 13:23:25 -05:00
Committer: Borislav Petkov <[email protected]>
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 <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Reviewed-by: Miroslav Benes <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
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))
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 <[email protected]>
AuthorDate: Wed, 01 Apr 2020 13:23:27 -05:00
Committer: Borislav Petkov <[email protected]>
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 <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>
Reviewed-by: Miroslav Benes <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
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;
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 <[email protected]>
AuthorDate: Wed, 01 Apr 2020 13:23:28 -05:00
Committer: Borislav Petkov <[email protected]>
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 <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Reviewed-by: Miroslav Benes <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
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)
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 <[email protected]>
AuthorDate: Wed, 01 Apr 2020 13:23:29 -05:00
Committer: Borislav Petkov <[email protected]>
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 <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
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;
}