From 963d5669171adcd59b45cad58fab81bbd599c3c8 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Mon, 20 Apr 2020 10:33:32 +0200 Subject: [PATCH 01/54] objtool: Fix 32bit cross builds Apparently there's people doing 64bit builds on 32bit machines. Fixes: 74b873e49d92 ("objtool: Optimize find_rela_by_dest_range()") Reported-by: youling257@gmail.com Signed-off-by: Peter Zijlstra (Intel) Signed-off-by: Ingo Molnar --- tools/objtool/elf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h index ebbb10c61e24..0b79c2353a21 100644 --- a/tools/objtool/elf.h +++ b/tools/objtool/elf.h @@ -99,7 +99,7 @@ static inline u32 sec_offset_hash(struct section *sec, unsigned long offset) offset &= OFFSET_STRIDE_MASK; ol = offset; - oh = offset >> 32; + oh = (offset >> 16) >> 16; __jhash_mix(ol, oh, idx); From 5377cae94ae31b089d4a69e7706672501c974f4d Mon Sep 17 00:00:00 2001 From: Julien Thierry Date: Fri, 3 Apr 2020 14:17:30 +0100 Subject: [PATCH 02/54] objtool: Fix off-by-one in symbol_by_offset() Sometimes, WARN_FUNC() and other users of symbol_by_offset() will associate the first instruction of a symbol with the symbol preceding it. This is because symbol->offset + symbol->len is already outside of the symbol's range. Fixes: 2a362ecc3ec9 ("objtool: Optimize find_symbol_*() and read_symbols()") Signed-off-by: Julien Thierry Reviewed-by: Miroslav Benes Signed-off-by: Josh Poimboeuf Signed-off-by: Ingo Molnar --- tools/objtool/elf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index 09ddc8f1def3..c4857fa3f1d1 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -105,7 +105,7 @@ static int symbol_by_offset(const void *key, const struct rb_node *node) if (*o < s->offset) return -1; - if (*o > s->offset + s->len) + if (*o >= s->offset + s->len) return 1; return 0; From aa5847270a0eba62e5ab8445163c0ac974844598 Mon Sep 17 00:00:00 2001 From: Julien Thierry Date: Fri, 27 Mar 2020 15:28:38 +0000 Subject: [PATCH 03/54] objtool: Always do header sync check Currently, the check of tools files against kernel equivalent is only done after every object file has been built. This means one might fix build issues against outdated headers without seeing a warning about this. Check headers before any object is built. Also, make it part of a FORCE'd recipe so every attempt to build objtool will report the outdated headers (if any). Signed-off-by: Julien Thierry Acked-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Signed-off-by: Josh Poimboeuf Signed-off-by: Ingo Molnar --- tools/objtool/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile index f591c4d1b6fe..d1ab0ccb70c7 100644 --- a/tools/objtool/Makefile +++ b/tools/objtool/Makefile @@ -49,10 +49,10 @@ export srctree OUTPUT CFLAGS SRCARCH AWK include $(srctree)/tools/build/Makefile.include $(OBJTOOL_IN): fixdep FORCE + @$(CONFIG_SHELL) ./sync-check.sh @$(MAKE) $(build)=objtool $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN) - @$(CONFIG_SHELL) ./sync-check.sh $(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@ From a70266b5b2e1c4262566a52f2ef16bdcde90f99b Mon Sep 17 00:00:00 2001 From: Julien Thierry Date: Fri, 27 Mar 2020 15:28:39 +0000 Subject: [PATCH 04/54] objtool: Remove redundant checks on operand type POP operations are already in the code path where the destination operand is OP_DEST_REG. There is no need to check the operand type again. Signed-off-by: Julien Thierry Acked-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Signed-off-by: Josh Poimboeuf Signed-off-by: Ingo Molnar --- tools/objtool/check.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 4b170fd08a28..c18eca151b6d 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1715,15 +1715,13 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state) case OP_SRC_POP: case OP_SRC_POPF: - if (!state->drap && op->dest.type == OP_DEST_REG && - op->dest.reg == cfa->base) { + if (!state->drap && op->dest.reg == cfa->base) { /* pop %rbp */ cfa->base = CFI_SP; } if (state->drap && cfa->base == CFI_BP_INDIRECT && - op->dest.type == OP_DEST_REG && op->dest.reg == state->drap_reg && state->drap_offset == -state->stack_size) { From 0699e551af268c9841a205a3e90dc1615fb63d84 Mon Sep 17 00:00:00 2001 From: Julien Thierry Date: Fri, 27 Mar 2020 15:28:40 +0000 Subject: [PATCH 05/54] objtool: Clean instruction state before each function validation When a function fails its validation, it might leave a stale state that will be used for the validation of other functions. That would cause false warnings on potentially valid functions. Reset the instruction state before the validation of each individual function. Signed-off-by: Julien Thierry Acked-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Signed-off-by: Josh Poimboeuf Signed-off-by: Ingo Molnar --- tools/objtool/check.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index c18eca151b6d..5b67d6150a0b 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2411,13 +2411,6 @@ static int validate_section(struct objtool_file *file, struct section *sec) struct insn_state state; int ret, warnings = 0; - clear_insn_state(&state); - - state.cfa = initial_func_cfi.cfa; - memcpy(&state.regs, &initial_func_cfi.regs, - CFI_NUM_REGS * sizeof(struct cfi_reg)); - state.stack_size = initial_func_cfi.cfa.offset; - list_for_each_entry(func, &sec->symbol_list, list) { if (func->type != STT_FUNC) continue; @@ -2435,6 +2428,12 @@ static int validate_section(struct objtool_file *file, struct section *sec) if (!insn || insn->ignore || insn->visited) continue; + clear_insn_state(&state); + state.cfa = initial_func_cfi.cfa; + memcpy(&state.regs, &initial_func_cfi.regs, + CFI_NUM_REGS * sizeof(struct cfi_reg)); + state.stack_size = initial_func_cfi.cfa.offset; + state.uaccess = func->uaccess_safe; ret = validate_branch(file, func, insn, state); From 7170cf47d16f1ba29eca07fd818870b7af0a93a5 Mon Sep 17 00:00:00 2001 From: Julien Thierry Date: Fri, 27 Mar 2020 15:28:41 +0000 Subject: [PATCH 06/54] objtool: Ignore empty alternatives The .alternatives section can contain entries with no original instructions. Objtool will currently crash when handling such an entry. Just skip that entry, but still give a warning to discourage useless entries. Signed-off-by: Julien Thierry Acked-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Signed-off-by: Josh Poimboeuf Signed-off-by: Ingo Molnar --- tools/objtool/check.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 5b67d6150a0b..efb964013552 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -905,6 +905,12 @@ static int add_special_section_alts(struct objtool_file *file) } if (special_alt->group) { + if (!special_alt->orig_len) { + WARN_FUNC("empty alternative entry", + orig_insn->sec, orig_insn->offset); + continue; + } + ret = handle_group_alt(file, special_alt, orig_insn, &new_insn); if (ret) From aff5e16918c3706622b138ea82391d6c597c5660 Mon Sep 17 00:00:00 2001 From: Julien Thierry Date: Fri, 27 Mar 2020 15:28:43 +0000 Subject: [PATCH 07/54] objtool: Use arch specific values in restore_reg() The initial register state is set up by arch specific code. Use the value the arch code has set when restoring registers from the stack. Suggested-by: Raphael Gault Signed-off-by: Julien Thierry Acked-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Signed-off-by: Josh Poimboeuf Signed-off-by: Ingo Molnar --- 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 efb964013552..229d61eba176 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1486,8 +1486,8 @@ static void save_reg(struct insn_state *state, unsigned char reg, int base, static void restore_reg(struct insn_state *state, unsigned char reg) { - state->regs[reg].base = CFI_UNDEFINED; - state->regs[reg].offset = 0; + state->regs[reg].base = initial_func_cfi.regs[reg].base; + state->regs[reg].offset = initial_func_cfi.regs[reg].offset; } /* From bfb08f220312a4634532114eb3e7062f17dfb707 Mon Sep 17 00:00:00 2001 From: Raphael Gault Date: Fri, 27 Mar 2020 15:28:45 +0000 Subject: [PATCH 08/54] objtool: Add abstraction for destination offsets The jump and call destination relocation offsets are x86-specific. Abstract them by calling arch-specific implementations. [ jthierry: Remove superfluous comment; replace other addend offsets with arch_dest_rela_offset() ] Signed-off-by: Raphael Gault Signed-off-by: Julien Thierry Acked-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Signed-off-by: Josh Poimboeuf Signed-off-by: Ingo Molnar --- tools/objtool/arch.h | 6 ++++++ tools/objtool/arch/x86/decode.c | 11 +++++++++++ tools/objtool/check.c | 18 ++++++++++-------- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h index ced3765c4f44..a9a50a25ca66 100644 --- a/tools/objtool/arch.h +++ b/tools/objtool/arch.h @@ -66,6 +66,8 @@ struct stack_op { struct op_src src; }; +struct instruction; + void arch_initial_func_cfi_state(struct cfi_state *state); int arch_decode_instruction(struct elf *elf, struct section *sec, @@ -75,4 +77,8 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, bool arch_callee_saved_reg(unsigned char reg); +unsigned long arch_jump_destination(struct instruction *insn); + +unsigned long arch_dest_rela_offset(int addend); + #endif /* _ARCH_H */ diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index a62e032863a8..7ce8650cf085 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -11,6 +11,7 @@ #include "../../../arch/x86/lib/inat.c" #include "../../../arch/x86/lib/insn.c" +#include "../../check.h" #include "../../elf.h" #include "../../arch.h" #include "../../warn.h" @@ -66,6 +67,16 @@ bool arch_callee_saved_reg(unsigned char reg) } } +unsigned long arch_dest_rela_offset(int addend) +{ + return addend + 4; +} + +unsigned long arch_jump_destination(struct instruction *insn) +{ + return insn->offset + insn->len + insn->immediate; +} + int arch_decode_instruction(struct elf *elf, struct section *sec, unsigned long offset, unsigned int maxlen, unsigned int *len, enum insn_type *type, diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 229d61eba176..cffa5e357788 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -574,13 +574,14 @@ static int add_jump_destinations(struct objtool_file *file) insn->offset, insn->len); if (!rela) { dest_sec = insn->sec; - dest_off = insn->offset + insn->len + insn->immediate; + dest_off = arch_jump_destination(insn); } else if (rela->sym->type == STT_SECTION) { dest_sec = rela->sym->sec; - dest_off = rela->addend + 4; + dest_off = arch_dest_rela_offset(rela->addend); } else if (rela->sym->sec->idx) { dest_sec = rela->sym->sec; - dest_off = rela->sym->sym.st_value + rela->addend + 4; + dest_off = rela->sym->sym.st_value + + arch_dest_rela_offset(rela->addend); } else if (strstr(rela->sym->name, "_indirect_thunk_")) { /* * Retpoline jumps are really dynamic jumps in @@ -670,7 +671,7 @@ static int add_call_destinations(struct objtool_file *file) rela = find_rela_by_dest_range(file->elf, insn->sec, insn->offset, insn->len); if (!rela) { - dest_off = insn->offset + insn->len + insn->immediate; + dest_off = arch_jump_destination(insn); insn->call_dest = find_func_by_offset(insn->sec, dest_off); if (!insn->call_dest) insn->call_dest = find_symbol_by_offset(insn->sec, dest_off); @@ -693,13 +694,14 @@ static int add_call_destinations(struct objtool_file *file) } } else if (rela->sym->type == STT_SECTION) { + dest_off = arch_dest_rela_offset(rela->addend); insn->call_dest = find_func_by_offset(rela->sym->sec, - rela->addend+4); + dest_off); if (!insn->call_dest) { - WARN_FUNC("can't find call dest symbol at %s+0x%x", + WARN_FUNC("can't find call dest symbol at %s+0x%lx", insn->sec, insn->offset, rela->sym->sec->name, - rela->addend + 4); + dest_off); return -1; } } else @@ -810,7 +812,7 @@ static int handle_group_alt(struct objtool_file *file, if (!insn->immediate) continue; - dest_off = insn->offset + insn->len + insn->immediate; + dest_off = arch_jump_destination(insn); if (dest_off == special_alt->new_off + special_alt->new_len) { if (!fake_jump) { WARN("%s: alternative jump to end of section", From 6f8ca67683962d408c7a337664212669bd3e8355 Mon Sep 17 00:00:00 2001 From: Julien Thierry Date: Fri, 27 Mar 2020 15:28:46 +0000 Subject: [PATCH 09/54] objtool: Split out arch-specific CFI definitions Some CFI definitions used by generic objtool code have no reason to vary from one architecture to another. Keep those definitions in generic code and move the arch-specific ones to a new arch-specific header. Signed-off-by: Julien Thierry Acked-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Signed-off-by: Josh Poimboeuf Signed-off-by: Ingo Molnar --- tools/objtool/Makefile | 3 ++- tools/objtool/arch/x86/include/cfi_regs.h | 25 +++++++++++++++++++++++ tools/objtool/cfi.h | 21 ++----------------- 3 files changed, 29 insertions(+), 20 deletions(-) create mode 100644 tools/objtool/arch/x86/include/cfi_regs.h diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile index d1ab0ccb70c7..6b91388aecbb 100644 --- a/tools/objtool/Makefile +++ b/tools/objtool/Makefile @@ -35,7 +35,8 @@ all: $(OBJTOOL) INCLUDES := -I$(srctree)/tools/include \ -I$(srctree)/tools/arch/$(HOSTARCH)/include/uapi \ - -I$(srctree)/tools/arch/$(SRCARCH)/include + -I$(srctree)/tools/arch/$(SRCARCH)/include \ + -I$(srctree)/tools/objtool/arch/$(SRCARCH)/include WARNINGS := $(EXTRA_WARNINGS) -Wno-switch-default -Wno-switch-enum -Wno-packed CFLAGS := -Werror $(WARNINGS) $(KBUILD_HOSTCFLAGS) -g $(INCLUDES) $(LIBELF_FLAGS) LDFLAGS += $(LIBELF_LIBS) $(LIBSUBCMD) $(KBUILD_HOSTLDFLAGS) diff --git a/tools/objtool/arch/x86/include/cfi_regs.h b/tools/objtool/arch/x86/include/cfi_regs.h new file mode 100644 index 000000000000..79bc517efba8 --- /dev/null +++ b/tools/objtool/arch/x86/include/cfi_regs.h @@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#ifndef _OBJTOOL_CFI_REGS_H +#define _OBJTOOL_CFI_REGS_H + +#define CFI_AX 0 +#define CFI_DX 1 +#define CFI_CX 2 +#define CFI_BX 3 +#define CFI_SI 4 +#define CFI_DI 5 +#define CFI_BP 6 +#define CFI_SP 7 +#define CFI_R8 8 +#define CFI_R9 9 +#define CFI_R10 10 +#define CFI_R11 11 +#define CFI_R12 12 +#define CFI_R13 13 +#define CFI_R14 14 +#define CFI_R15 15 +#define CFI_RA 16 +#define CFI_NUM_REGS 17 + +#endif /* _OBJTOOL_CFI_REGS_H */ diff --git a/tools/objtool/cfi.h b/tools/objtool/cfi.h index 4427bf8ed686..1a3e7b807994 100644 --- a/tools/objtool/cfi.h +++ b/tools/objtool/cfi.h @@ -6,30 +6,13 @@ #ifndef _OBJTOOL_CFI_H #define _OBJTOOL_CFI_H +#include "cfi_regs.h" + #define CFI_UNDEFINED -1 #define CFI_CFA -2 #define CFI_SP_INDIRECT -3 #define CFI_BP_INDIRECT -4 -#define CFI_AX 0 -#define CFI_DX 1 -#define CFI_CX 2 -#define CFI_BX 3 -#define CFI_SI 4 -#define CFI_DI 5 -#define CFI_BP 6 -#define CFI_SP 7 -#define CFI_R8 8 -#define CFI_R9 9 -#define CFI_R10 10 -#define CFI_R11 11 -#define CFI_R12 12 -#define CFI_R13 13 -#define CFI_R14 14 -#define CFI_R15 15 -#define CFI_RA 16 -#define CFI_NUM_REGS 17 - struct cfi_reg { int base; int offset; From e378fa17d3fac5b118381923abd2636f45a98c6e Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Thu, 26 Mar 2020 11:37:06 -0700 Subject: [PATCH 10/54] objtool: Documentation: document UACCESS warnings Compiling with Clang and CONFIG_KASAN=y was exposing a few warnings: call to memset() with UACCESS enabled Document how to fix these for future travelers. Link: https://github.com/ClangBuiltLinux/linux/issues/876 Suggested-by: Kamalesh Babulal Suggested-by: Matt Helsley Suggested-by: Peter Zijlstra Suggested-by: Randy Dunlap Signed-off-by: Nick Desaulniers Signed-off-by: Josh Poimboeuf Signed-off-by: Ingo Molnar --- .../Documentation/stack-validation.txt | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tools/objtool/Documentation/stack-validation.txt b/tools/objtool/Documentation/stack-validation.txt index de094670050b..faa47c3aafae 100644 --- a/tools/objtool/Documentation/stack-validation.txt +++ b/tools/objtool/Documentation/stack-validation.txt @@ -289,6 +289,32 @@ they mean, and suggestions for how to fix them. might be corrupt due to a gcc bug. For more details, see: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646 +9. file.o: warning: objtool: funcA() call to funcB() with UACCESS enabled + + This means that an unexpected call to a non-whitelisted function exists + outside of arch-specific guards. + X86: SMAP (stac/clac): __uaccess_begin()/__uaccess_end() + ARM: PAN: uaccess_enable()/uaccess_disable() + + These functions should be called to denote a minimal critical section around + access to __user variables. See also: https://lwn.net/Articles/517475/ + + The intention of the warning is to prevent calls to funcB() from eventually + calling schedule(), potentially leaking the AC flags state, and not + restoring them correctly. + + It also helps verify that there are no unexpected calls to funcB() which may + access user space pages with protections against doing so disabled. + + To fix, either: + 1) remove explicit calls to funcB() from funcA(). + 2) add the correct guards before and after calls to low level functions like + __get_user_size()/__put_user_size(). + 3) add funcB to uaccess_safe_builtin whitelist in tools/objtool/check.c, if + funcB obviously does not call schedule(), and is marked notrace (since + function tracing inserts additional calls, which is not obvious from the + sources). + If the error doesn't seem to make sense, it could be a bug in objtool. Feel free to ask the objtool maintainer for help. From 1ee444700e960b017558038a9443474e808b0045 Mon Sep 17 00:00:00 2001 From: Muchun Song Date: Sun, 12 Apr 2020 22:44:05 +0800 Subject: [PATCH 11/54] objtool: Remove redundant .rodata section name comparison If the prefix of section name is not '.rodata', the following function call can never return 0. strcmp(sec->name, C_JUMP_TABLE_SECTION) So the name comparison is pointless, just remove it. Signed-off-by: Muchun Song Signed-off-by: Josh Poimboeuf Signed-off-by: Ingo Molnar --- 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 cffa5e357788..e06a891a4a3b 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1354,8 +1354,8 @@ 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) && !strstr(sec->name, ".str1.")) || - !strcmp(sec->name, C_JUMP_TABLE_SECTION)) { + if (!strncmp(sec->name, ".rodata", 7) && + !strstr(sec->name, ".str1.")) { sec->rodata = true; found = true; } From 65ea47dcf4f936987a5fbf839c97acea00f4f196 Mon Sep 17 00:00:00 2001 From: Julien Thierry Date: Fri, 27 Mar 2020 15:28:47 +0000 Subject: [PATCH 12/54] objtool: Support multiple stack_op per instruction Instruction sets can include more or less complex operations which might not fit the currently defined set of stack_ops. Combining more than one stack_op provides more flexibility to describe the behaviour of an instruction. This also reduces the need to define new stack_ops specific to a single instruction set. Allow instruction decoders to generate multiple stack_op per instruction. Signed-off-by: Julien Thierry Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200327152847.15294-11-jthierry@redhat.com Signed-off-by: Ingo Molnar --- tools/objtool/arch.h | 4 +- tools/objtool/arch/x86/decode.c | 13 +++++- tools/objtool/check.c | 74 ++++++++++++++++++++------------- tools/objtool/check.h | 2 +- 4 files changed, 62 insertions(+), 31 deletions(-) diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h index a9a50a25ca66..f9883c431949 100644 --- a/tools/objtool/arch.h +++ b/tools/objtool/arch.h @@ -64,6 +64,7 @@ struct op_src { struct stack_op { struct op_dest dest; struct op_src src; + struct list_head list; }; struct instruction; @@ -73,7 +74,8 @@ void arch_initial_func_cfi_state(struct cfi_state *state); int arch_decode_instruction(struct elf *elf, struct section *sec, unsigned long offset, unsigned int maxlen, unsigned int *len, enum insn_type *type, - unsigned long *immediate, struct stack_op *op); + unsigned long *immediate, + struct list_head *ops_list); bool arch_callee_saved_reg(unsigned char reg); diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index 7ce8650cf085..199b4084a13c 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -80,13 +80,15 @@ unsigned long arch_jump_destination(struct instruction *insn) int arch_decode_instruction(struct elf *elf, struct section *sec, unsigned long offset, unsigned int maxlen, unsigned int *len, enum insn_type *type, - unsigned long *immediate, struct stack_op *op) + unsigned long *immediate, + struct list_head *ops_list) { struct insn insn; int x86_64, sign; unsigned char op1, op2, rex = 0, rex_b = 0, rex_r = 0, rex_w = 0, rex_x = 0, modrm = 0, modrm_mod = 0, modrm_rm = 0, modrm_reg = 0, sib = 0; + struct stack_op *op; x86_64 = is_x86_64(elf); if (x86_64 == -1) @@ -127,6 +129,10 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, if (insn.sib.nbytes) sib = insn.sib.bytes[0]; + op = calloc(1, sizeof(*op)); + if (!op) + return -1; + switch (op1) { case 0x1: @@ -488,6 +494,11 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, *immediate = insn.immediate.nbytes ? insn.immediate.value : 0; + if (*type == INSN_STACK) + list_add_tail(&op->list, ops_list); + else + free(op); + return 0; } diff --git a/tools/objtool/check.c b/tools/objtool/check.c index e06a891a4a3b..9e854fd128d4 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -260,6 +260,7 @@ static int decode_instructions(struct objtool_file *file) } memset(insn, 0, sizeof(*insn)); INIT_LIST_HEAD(&insn->alts); + INIT_LIST_HEAD(&insn->stack_ops); clear_insn_state(&insn->state); insn->sec = sec; @@ -269,7 +270,7 @@ static int decode_instructions(struct objtool_file *file) sec->len - offset, &insn->len, &insn->type, &insn->immediate, - &insn->stack_op); + &insn->stack_ops); if (ret) goto err; @@ -754,6 +755,7 @@ static int handle_group_alt(struct objtool_file *file, } memset(fake_jump, 0, sizeof(*fake_jump)); INIT_LIST_HEAD(&fake_jump->alts); + INIT_LIST_HEAD(&fake_jump->stack_ops); clear_insn_state(&fake_jump->state); fake_jump->sec = special_alt->new_sec; @@ -1452,10 +1454,11 @@ static bool has_valid_stack_frame(struct insn_state *state) return false; } -static int update_insn_state_regs(struct instruction *insn, struct insn_state *state) +static int update_insn_state_regs(struct instruction *insn, + struct insn_state *state, + struct stack_op *op) { struct cfi_reg *cfa = &state->cfa; - struct stack_op *op = &insn->stack_op; if (cfa->base != CFI_SP) return 0; @@ -1545,9 +1548,9 @@ static void restore_reg(struct insn_state *state, unsigned char reg) * 41 5d pop %r13 * c3 retq */ -static int update_insn_state(struct instruction *insn, struct insn_state *state) +static int update_insn_state(struct instruction *insn, struct insn_state *state, + struct stack_op *op) { - struct stack_op *op = &insn->stack_op; struct cfi_reg *cfa = &state->cfa; struct cfi_reg *regs = state->regs; @@ -1561,7 +1564,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state) } if (state->type == ORC_TYPE_REGS || state->type == ORC_TYPE_REGS_IRET) - return update_insn_state_regs(insn, state); + return update_insn_state_regs(insn, state, op); switch (op->dest.type) { @@ -1898,6 +1901,42 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state) return 0; } +static int handle_insn_ops(struct instruction *insn, struct insn_state *state) +{ + struct stack_op *op; + + list_for_each_entry(op, &insn->stack_ops, list) { + int res; + + res = update_insn_state(insn, state, op); + if (res) + return res; + + if (op->dest.type == OP_DEST_PUSHF) { + if (!state->uaccess_stack) { + state->uaccess_stack = 1; + } else if (state->uaccess_stack >> 31) { + WARN_FUNC("PUSHF stack exhausted", + insn->sec, insn->offset); + return 1; + } + state->uaccess_stack <<= 1; + state->uaccess_stack |= state->uaccess; + } + + if (op->src.type == OP_SRC_POPF) { + if (state->uaccess_stack) { + state->uaccess = state->uaccess_stack & 1; + state->uaccess_stack >>= 1; + if (state->uaccess_stack == 1) + state->uaccess_stack = 0; + } + } + } + + return 0; +} + static bool insn_state_match(struct instruction *insn, struct insn_state *state) { struct insn_state *state1 = &insn->state, *state2 = state; @@ -2198,29 +2237,8 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, return 0; case INSN_STACK: - if (update_insn_state(insn, &state)) + if (handle_insn_ops(insn, &state)) return 1; - - if (insn->stack_op.dest.type == OP_DEST_PUSHF) { - if (!state.uaccess_stack) { - state.uaccess_stack = 1; - } else if (state.uaccess_stack >> 31) { - WARN_FUNC("PUSHF stack exhausted", sec, insn->offset); - return 1; - } - state.uaccess_stack <<= 1; - state.uaccess_stack |= state.uaccess; - } - - if (insn->stack_op.src.type == OP_SRC_POPF) { - if (state.uaccess_stack) { - state.uaccess = state.uaccess_stack & 1; - state.uaccess_stack >>= 1; - if (state.uaccess_stack == 1) - state.uaccess_stack = 0; - } - } - break; case INSN_STAC: diff --git a/tools/objtool/check.h b/tools/objtool/check.h index f0ce8ffe7135..2c55f7591443 100644 --- a/tools/objtool/check.h +++ b/tools/objtool/check.h @@ -42,7 +42,7 @@ struct instruction { struct rela *jump_table; struct list_head alts; struct symbol *func; - struct stack_op stack_op; + struct list_head stack_ops; struct insn_state state; struct orc_entry orc; }; From b746046238bb99b8f703c79f6d95357428fb6476 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Thu, 2 Apr 2020 10:15:51 +0200 Subject: [PATCH 13/54] objtool: Better handle IRET Teach objtool a little more about IRET so that we can avoid using the SAVE/RESTORE annotation. In particular, make the weird corner case in insn->restore go away. The purpose of that corner case is to deal with the fact that UNWIND_HINT_RESTORE lands on the instruction after IRET, but that instruction can end up being outside the basic block, consider: if (cond) sync_core() foo(); Then the hint will land on foo(), and we'll encounter the restore hint without ever having seen the save hint. By teaching objtool about the arch specific exception frame size, and assuming that any IRET in an STT_FUNC symbol is an exception frame sized POP, we can remove the use of save/restore hints for this code. Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115118.631224674@infradead.org Signed-off-by: Ingo Molnar --- arch/x86/include/asm/processor.h | 2 -- tools/objtool/arch.h | 1 + tools/objtool/arch/x86/decode.c | 14 ++++++++++++-- tools/objtool/check.c | 29 ++++++++++++++++------------- 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 3bcf27caf6c9..3eeaaeb75638 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -727,7 +727,6 @@ static inline void sync_core(void) unsigned int tmp; asm volatile ( - UNWIND_HINT_SAVE "mov %%ss, %0\n\t" "pushq %q0\n\t" "pushq %%rsp\n\t" @@ -737,7 +736,6 @@ static inline void sync_core(void) "pushq %q0\n\t" "pushq $1f\n\t" "iretq\n\t" - UNWIND_HINT_RESTORE "1:" : "=&r" (tmp), ASM_CALL_CONSTRAINT : : "cc", "memory"); #endif diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h index f9883c431949..55396dfe0d07 100644 --- a/tools/objtool/arch.h +++ b/tools/objtool/arch.h @@ -19,6 +19,7 @@ enum insn_type { INSN_CALL, INSN_CALL_DYNAMIC, INSN_RETURN, + INSN_EXCEPTION_RETURN, INSN_CONTEXT_SWITCH, INSN_STACK, INSN_BUG, diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index 199b4084a13c..32736383ead1 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -446,9 +446,19 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, *type = INSN_RETURN; break; + case 0xcf: /* iret */ + *type = INSN_EXCEPTION_RETURN; + + /* add $40, %rsp */ + op->src.type = OP_SRC_ADD; + op->src.reg = CFI_SP; + op->src.offset = 5*8; + op->dest.type = OP_DEST_REG; + op->dest.reg = CFI_SP; + break; + case 0xca: /* retf */ case 0xcb: /* retf */ - case 0xcf: /* iret */ *type = INSN_CONTEXT_SWITCH; break; @@ -494,7 +504,7 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, *immediate = insn.immediate.nbytes ? insn.immediate.value : 0; - if (*type == INSN_STACK) + if (*type == INSN_STACK || *type == INSN_EXCEPTION_RETURN) list_add_tail(&op->list, ops_list); else free(op); diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 9e854fd128d4..781b3a3c2ba6 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2065,15 +2065,14 @@ static int validate_return(struct symbol *func, struct instruction *insn, struct * tools/objtool/Documentation/stack-validation.txt. */ static int validate_branch(struct objtool_file *file, struct symbol *func, - struct instruction *first, struct insn_state state) + struct instruction *insn, struct insn_state state) { struct alternative *alt; - struct instruction *insn, *next_insn; + struct instruction *next_insn; struct section *sec; u8 visited; int ret; - insn = first; sec = insn->sec; if (insn->alt_group && list_empty(&insn->alts)) { @@ -2126,16 +2125,6 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, } if (!save_insn->visited) { - /* - * Oops, no state to copy yet. - * Hopefully we can reach this - * instruction from another branch - * after the save insn has been - * visited. - */ - if (insn == first) - return 0; - WARN_FUNC("objtool isn't smart enough to handle this CFI save/restore combo", sec, insn->offset); return 1; @@ -2228,6 +2217,20 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, break; + case INSN_EXCEPTION_RETURN: + if (handle_insn_ops(insn, &state)) + return 1; + + /* + * This handles x86's sync_core() case, where we use an + * IRET to self. All 'normal' IRET instructions are in + * STT_NOTYPE entry symbols. + */ + if (func) + break; + + return 0; + case INSN_CONTEXT_SWITCH: if (func && (!next_insn || !next_insn->hint)) { WARN_FUNC("unsupported instruction in callable function", From e25eea89bb8853763a22fa2547199cf96b571ba1 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Wed, 1 Apr 2020 16:38:19 +0200 Subject: [PATCH 14/54] objtool: Introduce HINT_RET_OFFSET Normally objtool ensures a function keeps the stack layout invariant. But there is a useful exception, it is possible to stuff the return stack in order to 'inject' a 'call': push $fun ret In this case the invariant mentioned above is violated. Add an objtool HINT to annotate this and allow a function exit with a modified stack frame. Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115118.690601403@infradead.org Signed-off-by: Ingo Molnar --- arch/x86/include/asm/orc_types.h | 1 + arch/x86/include/asm/unwind_hints.h | 10 ++++++++++ tools/arch/x86/include/asm/orc_types.h | 1 + tools/objtool/check.c | 24 ++++++++++++++++-------- tools/objtool/check.h | 4 +++- 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/arch/x86/include/asm/orc_types.h b/arch/x86/include/asm/orc_types.h index 6e060907c163..5f18ca7ac51a 100644 --- a/arch/x86/include/asm/orc_types.h +++ b/arch/x86/include/asm/orc_types.h @@ -60,6 +60,7 @@ #define ORC_TYPE_REGS_IRET 2 #define UNWIND_HINT_TYPE_SAVE 3 #define UNWIND_HINT_TYPE_RESTORE 4 +#define UNWIND_HINT_TYPE_RET_OFFSET 5 #ifndef __ASSEMBLY__ /* diff --git a/arch/x86/include/asm/unwind_hints.h b/arch/x86/include/asm/unwind_hints.h index f5e2eb12cb71..aabf7ace0476 100644 --- a/arch/x86/include/asm/unwind_hints.h +++ b/arch/x86/include/asm/unwind_hints.h @@ -94,6 +94,16 @@ UNWIND_HINT type=UNWIND_HINT_TYPE_RESTORE .endm + +/* + * RET_OFFSET: Used on instructions that terminate a function; mostly RETURN + * and sibling calls. On these, sp_offset denotes the expected offset from + * initial_func_cfi. + */ +.macro UNWIND_HINT_RET_OFFSET sp_offset=8 + UNWIND_HINT type=UNWIND_HINT_TYPE_RET_OFFSET sp_offset=\sp_offset +.endm + #else /* !__ASSEMBLY__ */ #define UNWIND_HINT(sp_reg, sp_offset, type, end) \ diff --git a/tools/arch/x86/include/asm/orc_types.h b/tools/arch/x86/include/asm/orc_types.h index 6e060907c163..5f18ca7ac51a 100644 --- a/tools/arch/x86/include/asm/orc_types.h +++ b/tools/arch/x86/include/asm/orc_types.h @@ -60,6 +60,7 @@ #define ORC_TYPE_REGS_IRET 2 #define UNWIND_HINT_TYPE_SAVE 3 #define UNWIND_HINT_TYPE_RESTORE 4 +#define UNWIND_HINT_TYPE_RET_OFFSET 5 #ifndef __ASSEMBLY__ /* diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 781b3a3c2ba6..93c88ac51f0f 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1261,6 +1261,9 @@ static int read_unwind_hints(struct objtool_file *file) } else if (hint->type == UNWIND_HINT_TYPE_RESTORE) { insn->restore = true; insn->hint = true; + + } else if (hint->type == UNWIND_HINT_TYPE_RET_OFFSET) { + insn->ret_offset = hint->sp_offset; continue; } @@ -1424,20 +1427,25 @@ static bool is_fentry_call(struct instruction *insn) return false; } -static bool has_modified_stack_frame(struct insn_state *state) +static bool has_modified_stack_frame(struct instruction *insn, struct insn_state *state) { + u8 ret_offset = insn->ret_offset; int i; - if (state->cfa.base != initial_func_cfi.cfa.base || - state->cfa.offset != initial_func_cfi.cfa.offset || - state->stack_size != initial_func_cfi.cfa.offset || - state->drap) + if (state->cfa.base != initial_func_cfi.cfa.base || state->drap) return true; - for (i = 0; i < CFI_NUM_REGS; i++) + if (state->cfa.offset != initial_func_cfi.cfa.offset + ret_offset) + return true; + + if (state->stack_size != initial_func_cfi.cfa.offset + ret_offset) + return true; + + for (i = 0; i < CFI_NUM_REGS; i++) { if (state->regs[i].base != initial_func_cfi.regs[i].base || state->regs[i].offset != initial_func_cfi.regs[i].offset) return true; + } return false; } @@ -2014,7 +2022,7 @@ static int validate_call(struct instruction *insn, struct insn_state *state) static int validate_sibling_call(struct instruction *insn, struct insn_state *state) { - if (has_modified_stack_frame(state)) { + if (has_modified_stack_frame(insn, state)) { WARN_FUNC("sibling call from callable instruction with modified stack frame", insn->sec, insn->offset); return 1; @@ -2043,7 +2051,7 @@ static int validate_return(struct symbol *func, struct instruction *insn, struct return 1; } - if (func && has_modified_stack_frame(state)) { + if (func && has_modified_stack_frame(insn, state)) { WARN_FUNC("return with modified stack frame", insn->sec, insn->offset); return 1; diff --git a/tools/objtool/check.h b/tools/objtool/check.h index 2c55f7591443..81ce27e62c04 100644 --- a/tools/objtool/check.h +++ b/tools/objtool/check.h @@ -33,9 +33,11 @@ struct instruction { unsigned int len; enum insn_type type; unsigned long immediate; - bool alt_group, dead_end, ignore, hint, save, restore, ignore_alts; + bool alt_group, dead_end, ignore, ignore_alts; + bool hint, save, restore; bool retpoline_safe; u8 visited; + u8 ret_offset; struct symbol *call_dest; struct instruction *jump_dest; struct instruction *first_jump_src; From 0298739b7983cf9bf4fcfb4bfb815c539bdb87ca Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Wed, 1 Apr 2020 16:53:19 +0200 Subject: [PATCH 15/54] x86,ftrace: Fix ftrace_regs_caller() unwind The ftrace_regs_caller() trampoline does something 'funny' when there is a direct-caller present. In that case it stuffs the 'direct-caller' address on the return stack and then exits the function. This then results in 'returning' to the direct-caller with the exact registers we came in with -- an indirect tail-call without using a register. This however (rightfully) confuses objtool because the function shares a few instruction in order to have a single exit path, but the stack layout is different for them, depending through which path we came there. This is currently cludged by forcing the stack state to the non-direct case, but this generates actively wrong (ORC) unwind information for the direct case, leading to potential broken unwinds. Fix this issue by fully separating the exit paths. This results in having to poke a second RET into the trampoline copy, see ftrace_regs_caller_ret. This brings us to a second objtool problem, in order for it to perceive the 'jmp ftrace_epilogue' as a function exit, it needs to be recognised as a tail call. In order to make that happen, ftrace_epilogue needs to be the start of an STT_FUNC, so re-arrange code to make this so. Finally, a third issue is that objtool requires functions to exit with the same stack layout they started with, which is obviously violated in the direct case, employ the new HINT_RET_OFFSET to tell objtool this is an expected exception. Together, this results in generating correct ORC unwind information for the ftrace_regs_caller() function and it's trampoline copies. Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115118.749606694@infradead.org Signed-off-by: Ingo Molnar --- arch/x86/kernel/ftrace.c | 12 ++++++++++-- arch/x86/kernel/ftrace_64.S | 32 +++++++++++++++----------------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index 37a0aeaf89e7..867c126ddabe 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -282,7 +282,8 @@ static inline void tramp_free(void *tramp) { } /* Defined as markers to the end of the ftrace default trampolines */ extern void ftrace_regs_caller_end(void); -extern void ftrace_epilogue(void); +extern void ftrace_regs_caller_ret(void); +extern void ftrace_caller_end(void); extern void ftrace_caller_op_ptr(void); extern void ftrace_regs_caller_op_ptr(void); @@ -334,7 +335,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size) call_offset = (unsigned long)ftrace_regs_call; } else { start_offset = (unsigned long)ftrace_caller; - end_offset = (unsigned long)ftrace_epilogue; + end_offset = (unsigned long)ftrace_caller_end; op_offset = (unsigned long)ftrace_caller_op_ptr; call_offset = (unsigned long)ftrace_call; } @@ -366,6 +367,13 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size) if (WARN_ON(ret < 0)) goto fail; + if (ops->flags & FTRACE_OPS_FL_SAVE_REGS) { + ip = trampoline + (ftrace_regs_caller_ret - ftrace_regs_caller); + ret = probe_kernel_read(ip, (void *)retq, RET_SIZE); + if (WARN_ON(ret < 0)) + goto fail; + } + /* * The address of the ftrace_ops that is used for this trampoline * is stored at the end of the trampoline. This will be used to diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S index 369e61faacfe..7657dc782828 100644 --- a/arch/x86/kernel/ftrace_64.S +++ b/arch/x86/kernel/ftrace_64.S @@ -157,8 +157,12 @@ SYM_INNER_LABEL(ftrace_call, SYM_L_GLOBAL) * think twice before adding any new code or changing the * layout here. */ -SYM_INNER_LABEL(ftrace_epilogue, SYM_L_GLOBAL) +SYM_INNER_LABEL(ftrace_caller_end, SYM_L_GLOBAL) + jmp ftrace_epilogue +SYM_FUNC_END(ftrace_caller); + +SYM_FUNC_START(ftrace_epilogue) #ifdef CONFIG_FUNCTION_GRAPH_TRACER SYM_INNER_LABEL(ftrace_graph_call, SYM_L_GLOBAL) jmp ftrace_stub @@ -170,14 +174,12 @@ SYM_INNER_LABEL(ftrace_graph_call, SYM_L_GLOBAL) */ SYM_INNER_LABEL_ALIGN(ftrace_stub, SYM_L_WEAK) retq -SYM_FUNC_END(ftrace_caller) +SYM_FUNC_END(ftrace_epilogue) SYM_FUNC_START(ftrace_regs_caller) /* Save the current flags before any operations that can change them */ pushfq - UNWIND_HINT_SAVE - /* added 8 bytes to save flags */ save_mcount_regs 8 /* save_mcount_regs fills in first two parameters */ @@ -233,7 +235,10 @@ SYM_INNER_LABEL(ftrace_regs_call, SYM_L_GLOBAL) movq ORIG_RAX(%rsp), %rax movq %rax, MCOUNT_REG_SIZE-8(%rsp) - /* If ORIG_RAX is anything but zero, make this a call to that */ + /* + * If ORIG_RAX is anything but zero, make this a call to that. + * See arch_ftrace_set_direct_caller(). + */ movq ORIG_RAX(%rsp), %rax cmpq $0, %rax je 1f @@ -244,20 +249,14 @@ SYM_INNER_LABEL(ftrace_regs_call, SYM_L_GLOBAL) movq %rax, MCOUNT_REG_SIZE(%rsp) restore_mcount_regs 8 + /* Restore flags */ + popfq - jmp 2f +SYM_INNER_LABEL(ftrace_regs_caller_ret, SYM_L_GLOBAL); + UNWIND_HINT_RET_OFFSET + jmp ftrace_epilogue 1: restore_mcount_regs - - -2: - /* - * The stack layout is nondetermistic here, depending on which path was - * taken. This confuses objtool and ORC, rightfully so. For now, - * pretend the stack always looks like the non-direct case. - */ - UNWIND_HINT_RESTORE - /* Restore flags */ popfq @@ -268,7 +267,6 @@ SYM_INNER_LABEL(ftrace_regs_call, SYM_L_GLOBAL) * to the return. */ SYM_INNER_LABEL(ftrace_regs_caller_end, SYM_L_GLOBAL) - jmp ftrace_epilogue SYM_FUNC_END(ftrace_regs_caller) From dc2745b61907cf6faeb72cc25f2cc4b38d4a3cac Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Wed, 1 Apr 2020 16:50:40 +0200 Subject: [PATCH 16/54] x86,ftrace: Use SIZEOF_PTREGS There's a convenient macro for 'SS+8' called FRAME_SIZE. Use it to clarify things. (entry/calling.h calls this SIZEOF_PTREGS but we're using asm/ptrace-abi.h) Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115118.808485515@infradead.org Signed-off-by: Ingo Molnar --- arch/x86/kernel/ftrace_64.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S index 7657dc782828..be9aff20dd5f 100644 --- a/arch/x86/kernel/ftrace_64.S +++ b/arch/x86/kernel/ftrace_64.S @@ -23,7 +23,7 @@ #endif /* CONFIG_FRAME_POINTER */ /* Size of stack used to save mcount regs in save_mcount_regs */ -#define MCOUNT_REG_SIZE (SS+8 + MCOUNT_FRAME_SIZE) +#define MCOUNT_REG_SIZE (FRAME_SIZE + MCOUNT_FRAME_SIZE) /* * gcc -pg option adds a call to 'mcount' in most functions. @@ -77,7 +77,7 @@ /* * We add enough stack to save all regs. */ - subq $(MCOUNT_REG_SIZE - MCOUNT_FRAME_SIZE), %rsp + subq $(FRAME_SIZE), %rsp movq %rax, RAX(%rsp) movq %rcx, RCX(%rsp) movq %rdx, RDX(%rsp) From 9f2dfd61dd022d4559d42a832fb03e76aad36c5f Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Wed, 1 Apr 2020 16:51:11 +0200 Subject: [PATCH 17/54] x86,ftrace: Shrink ftrace_regs_caller() by one byte 'Optimize' ftrace_regs_caller. Instead of comparing against an immediate, the more natural way to test for zero on x86 is: 'test %r,%r'. 48 83 f8 00 cmp $0x0,%rax 74 49 je 226 48 85 c0 test %rax,%rax 74 49 je 225 Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115118.867411350@infradead.org Signed-off-by: Ingo Molnar --- arch/x86/kernel/ftrace_64.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S index be9aff20dd5f..9738ed23964e 100644 --- a/arch/x86/kernel/ftrace_64.S +++ b/arch/x86/kernel/ftrace_64.S @@ -240,8 +240,8 @@ SYM_INNER_LABEL(ftrace_regs_call, SYM_L_GLOBAL) * See arch_ftrace_set_direct_caller(). */ movq ORIG_RAX(%rsp), %rax - cmpq $0, %rax - je 1f + testq %rax, %rax + jz 1f /* Swap the flags with orig_rax */ movq MCOUNT_REG_SIZE(%rsp), %rdi From c536ed2fffd5dbf81fe2dede8ef294e0cbb08f72 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Wed, 1 Apr 2020 16:54:26 +0200 Subject: [PATCH 18/54] objtool: Remove SAVE/RESTORE hints The SAVE/RESTORE hints are now unused; remove them. Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115118.926738768@infradead.org Signed-off-by: Ingo Molnar --- arch/x86/include/asm/orc_types.h | 4 +-- arch/x86/include/asm/unwind_hints.h | 27 ----------------- tools/arch/x86/include/asm/orc_types.h | 4 +-- tools/objtool/check.c | 42 ++------------------------ tools/objtool/check.h | 2 +- 5 files changed, 6 insertions(+), 73 deletions(-) diff --git a/arch/x86/include/asm/orc_types.h b/arch/x86/include/asm/orc_types.h index 5f18ca7ac51a..d25534940bde 100644 --- a/arch/x86/include/asm/orc_types.h +++ b/arch/x86/include/asm/orc_types.h @@ -58,9 +58,7 @@ #define ORC_TYPE_CALL 0 #define ORC_TYPE_REGS 1 #define ORC_TYPE_REGS_IRET 2 -#define UNWIND_HINT_TYPE_SAVE 3 -#define UNWIND_HINT_TYPE_RESTORE 4 -#define UNWIND_HINT_TYPE_RET_OFFSET 5 +#define UNWIND_HINT_TYPE_RET_OFFSET 3 #ifndef __ASSEMBLY__ /* diff --git a/arch/x86/include/asm/unwind_hints.h b/arch/x86/include/asm/unwind_hints.h index aabf7ace0476..7d903fdb3f43 100644 --- a/arch/x86/include/asm/unwind_hints.h +++ b/arch/x86/include/asm/unwind_hints.h @@ -86,15 +86,6 @@ UNWIND_HINT sp_offset=\sp_offset .endm -.macro UNWIND_HINT_SAVE - UNWIND_HINT type=UNWIND_HINT_TYPE_SAVE -.endm - -.macro UNWIND_HINT_RESTORE - UNWIND_HINT type=UNWIND_HINT_TYPE_RESTORE -.endm - - /* * RET_OFFSET: Used on instructions that terminate a function; mostly RETURN * and sibling calls. On these, sp_offset denotes the expected offset from @@ -104,24 +95,6 @@ UNWIND_HINT type=UNWIND_HINT_TYPE_RET_OFFSET sp_offset=\sp_offset .endm -#else /* !__ASSEMBLY__ */ - -#define UNWIND_HINT(sp_reg, sp_offset, type, end) \ - "987: \n\t" \ - ".pushsection .discard.unwind_hints\n\t" \ - /* struct unwind_hint */ \ - ".long 987b - .\n\t" \ - ".short " __stringify(sp_offset) "\n\t" \ - ".byte " __stringify(sp_reg) "\n\t" \ - ".byte " __stringify(type) "\n\t" \ - ".byte " __stringify(end) "\n\t" \ - ".balign 4 \n\t" \ - ".popsection\n\t" - -#define UNWIND_HINT_SAVE UNWIND_HINT(0, 0, UNWIND_HINT_TYPE_SAVE, 0) - -#define UNWIND_HINT_RESTORE UNWIND_HINT(0, 0, UNWIND_HINT_TYPE_RESTORE, 0) - #endif /* __ASSEMBLY__ */ #endif /* _ASM_X86_UNWIND_HINTS_H */ diff --git a/tools/arch/x86/include/asm/orc_types.h b/tools/arch/x86/include/asm/orc_types.h index 5f18ca7ac51a..d25534940bde 100644 --- a/tools/arch/x86/include/asm/orc_types.h +++ b/tools/arch/x86/include/asm/orc_types.h @@ -58,9 +58,7 @@ #define ORC_TYPE_CALL 0 #define ORC_TYPE_REGS 1 #define ORC_TYPE_REGS_IRET 2 -#define UNWIND_HINT_TYPE_SAVE 3 -#define UNWIND_HINT_TYPE_RESTORE 4 -#define UNWIND_HINT_TYPE_RET_OFFSET 5 +#define UNWIND_HINT_TYPE_RET_OFFSET 3 #ifndef __ASSEMBLY__ /* diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 93c88ac51f0f..464f10c0a5ac 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1254,15 +1254,7 @@ static int read_unwind_hints(struct objtool_file *file) cfa = &insn->state.cfa; - if (hint->type == UNWIND_HINT_TYPE_SAVE) { - insn->save = true; - continue; - - } else if (hint->type == UNWIND_HINT_TYPE_RESTORE) { - insn->restore = true; - insn->hint = true; - - } else if (hint->type == UNWIND_HINT_TYPE_RET_OFFSET) { + if (hint->type == UNWIND_HINT_TYPE_RET_OFFSET) { insn->ret_offset = hint->sp_offset; continue; } @@ -2113,37 +2105,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, return 0; } - if (insn->hint) { - if (insn->restore) { - struct instruction *save_insn, *i; - - i = insn; - save_insn = NULL; - sym_for_each_insn_continue_reverse(file, func, i) { - if (i->save) { - save_insn = i; - break; - } - } - - if (!save_insn) { - WARN_FUNC("no corresponding CFI save for CFI restore", - sec, insn->offset); - return 1; - } - - if (!save_insn->visited) { - WARN_FUNC("objtool isn't smart enough to handle this CFI save/restore combo", - sec, insn->offset); - return 1; - } - - insn->state = save_insn->state; - } - + if (insn->hint) state = insn->state; - - } else + else insn->state = state; insn->visited |= visited; diff --git a/tools/objtool/check.h b/tools/objtool/check.h index 81ce27e62c04..7c30760bee74 100644 --- a/tools/objtool/check.h +++ b/tools/objtool/check.h @@ -34,7 +34,7 @@ struct instruction { enum insn_type type; unsigned long immediate; bool alt_group, dead_end, ignore, ignore_alts; - bool hint, save, restore; + bool hint; bool retpoline_safe; u8 visited; u8 ret_offset; From a3608f5954d07a40fb93764dc6d06195fa52eb14 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Wed, 25 Mar 2020 15:34:50 +0100 Subject: [PATCH 19/54] objtool: Rename struct cfi_state There's going to be a new struct cfi_state, rename this one to make place. Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115118.986441913@infradead.org Signed-off-by: Ingo Molnar --- tools/objtool/arch.h | 2 +- tools/objtool/arch/x86/decode.c | 2 +- tools/objtool/cfi.h | 2 +- tools/objtool/check.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h index 55396dfe0d07..561c3162d177 100644 --- a/tools/objtool/arch.h +++ b/tools/objtool/arch.h @@ -70,7 +70,7 @@ struct stack_op { struct instruction; -void arch_initial_func_cfi_state(struct cfi_state *state); +void arch_initial_func_cfi_state(struct cfi_init_state *state); int arch_decode_instruction(struct elf *elf, struct section *sec, unsigned long offset, unsigned int maxlen, diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index 32736383ead1..f0d42ad7d5ff 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -512,7 +512,7 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, return 0; } -void arch_initial_func_cfi_state(struct cfi_state *state) +void arch_initial_func_cfi_state(struct cfi_init_state *state) { int i; diff --git a/tools/objtool/cfi.h b/tools/objtool/cfi.h index 1a3e7b807994..6faf97666d6b 100644 --- a/tools/objtool/cfi.h +++ b/tools/objtool/cfi.h @@ -18,7 +18,7 @@ struct cfi_reg { int offset; }; -struct cfi_state { +struct cfi_init_state { struct cfi_reg cfa; struct cfi_reg regs[CFI_NUM_REGS]; }; diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 464f10c0a5ac..538b462fe7ea 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -27,7 +27,7 @@ struct alternative { }; const char *objname; -struct cfi_state initial_func_cfi; +struct cfi_init_state initial_func_cfi; struct instruction *find_insn(struct objtool_file *file, struct section *sec, unsigned long offset) From e7c0219b328c96746767f21b9532eed6a48f61c5 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Wed, 25 Mar 2020 14:04:45 +0100 Subject: [PATCH 20/54] objtool: Fix !CFI insn_state propagation Objtool keeps per instruction CFI state in struct insn_state and will save/restore this where required. However, insn_state has grown some !CFI state, and this must not be saved/restored (that would loose/destroy state). Fix this by moving the CFI specific parts of insn_state into struct cfi_state. Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115119.045821071@infradead.org Signed-off-by: Ingo Molnar --- tools/objtool/cfi.h | 14 ++- tools/objtool/check.c | 266 +++++++++++++++++++++------------------- tools/objtool/check.h | 13 +- tools/objtool/orc_gen.c | 8 +- 4 files changed, 159 insertions(+), 142 deletions(-) diff --git a/tools/objtool/cfi.h b/tools/objtool/cfi.h index 6faf97666d6b..c7c59c6a44ee 100644 --- a/tools/objtool/cfi.h +++ b/tools/objtool/cfi.h @@ -19,8 +19,20 @@ struct cfi_reg { }; struct cfi_init_state { - struct cfi_reg cfa; struct cfi_reg regs[CFI_NUM_REGS]; + struct cfi_reg cfa; +}; + +struct cfi_state { + struct cfi_reg regs[CFI_NUM_REGS]; + struct cfi_reg vals[CFI_NUM_REGS]; + struct cfi_reg cfa; + int stack_size; + int drap_reg, drap_offset; + unsigned char type; + bool bp_scratch; + bool drap; + bool end; }; #endif /* _OBJTOOL_CFI_H */ diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 538b462fe7ea..abf97159bf1e 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -215,18 +215,23 @@ static bool dead_end_function(struct objtool_file *file, struct symbol *func) return __dead_end_function(file, func, 0); } -static void clear_insn_state(struct insn_state *state) +static void init_cfi_state(struct cfi_state *cfi) { int i; - memset(state, 0, sizeof(*state)); - state->cfa.base = CFI_UNDEFINED; for (i = 0; i < CFI_NUM_REGS; i++) { - state->regs[i].base = CFI_UNDEFINED; - state->vals[i].base = CFI_UNDEFINED; + cfi->regs[i].base = CFI_UNDEFINED; + cfi->vals[i].base = CFI_UNDEFINED; } - state->drap_reg = CFI_UNDEFINED; - state->drap_offset = -1; + cfi->cfa.base = CFI_UNDEFINED; + cfi->drap_reg = CFI_UNDEFINED; + cfi->drap_offset = -1; +} + +static void clear_insn_state(struct insn_state *state) +{ + memset(state, 0, sizeof(*state)); + init_cfi_state(&state->cfi); } /* @@ -261,7 +266,7 @@ static int decode_instructions(struct objtool_file *file) memset(insn, 0, sizeof(*insn)); INIT_LIST_HEAD(&insn->alts); INIT_LIST_HEAD(&insn->stack_ops); - clear_insn_state(&insn->state); + init_cfi_state(&insn->cfi); insn->sec = sec; insn->offset = offset; @@ -756,7 +761,7 @@ static int handle_group_alt(struct objtool_file *file, memset(fake_jump, 0, sizeof(*fake_jump)); INIT_LIST_HEAD(&fake_jump->alts); INIT_LIST_HEAD(&fake_jump->stack_ops); - clear_insn_state(&fake_jump->state); + init_cfi_state(&fake_jump->cfi); fake_jump->sec = special_alt->new_sec; fake_jump->offset = FAKE_JUMP_OFFSET; @@ -1252,7 +1257,7 @@ static int read_unwind_hints(struct objtool_file *file) return -1; } - cfa = &insn->state.cfa; + cfa = &insn->cfi.cfa; if (hint->type == UNWIND_HINT_TYPE_RET_OFFSET) { insn->ret_offset = hint->sp_offset; @@ -1293,8 +1298,8 @@ static int read_unwind_hints(struct objtool_file *file) } cfa->offset = hint->sp_offset; - insn->state.type = hint->type; - insn->state.end = hint->end; + insn->cfi.type = hint->type; + insn->cfi.end = hint->end; } return 0; @@ -1422,20 +1427,21 @@ static bool is_fentry_call(struct instruction *insn) static bool has_modified_stack_frame(struct instruction *insn, struct insn_state *state) { u8 ret_offset = insn->ret_offset; + struct cfi_state *cfi = &state->cfi; int i; - if (state->cfa.base != initial_func_cfi.cfa.base || state->drap) + if (cfi->cfa.base != initial_func_cfi.cfa.base || cfi->drap) return true; - if (state->cfa.offset != initial_func_cfi.cfa.offset + ret_offset) + if (cfi->cfa.offset != initial_func_cfi.cfa.offset + ret_offset) return true; - if (state->stack_size != initial_func_cfi.cfa.offset + ret_offset) + if (cfi->stack_size != initial_func_cfi.cfa.offset + ret_offset) return true; for (i = 0; i < CFI_NUM_REGS; i++) { - if (state->regs[i].base != initial_func_cfi.regs[i].base || - state->regs[i].offset != initial_func_cfi.regs[i].offset) + if (cfi->regs[i].base != initial_func_cfi.regs[i].base || + cfi->regs[i].offset != initial_func_cfi.regs[i].offset) return true; } @@ -1444,21 +1450,23 @@ static bool has_modified_stack_frame(struct instruction *insn, struct insn_state static bool has_valid_stack_frame(struct insn_state *state) { - if (state->cfa.base == CFI_BP && state->regs[CFI_BP].base == CFI_CFA && - state->regs[CFI_BP].offset == -16) + struct cfi_state *cfi = &state->cfi; + + if (cfi->cfa.base == CFI_BP && cfi->regs[CFI_BP].base == CFI_CFA && + cfi->regs[CFI_BP].offset == -16) return true; - if (state->drap && state->regs[CFI_BP].base == CFI_BP) + if (cfi->drap && cfi->regs[CFI_BP].base == CFI_BP) return true; return false; } -static int update_insn_state_regs(struct instruction *insn, - struct insn_state *state, +static int update_cfi_state_regs(struct instruction *insn, + struct cfi_state *cfi, struct stack_op *op) { - struct cfi_reg *cfa = &state->cfa; + struct cfi_reg *cfa = &cfi->cfa; if (cfa->base != CFI_SP) return 0; @@ -1479,20 +1487,19 @@ static int update_insn_state_regs(struct instruction *insn, return 0; } -static void save_reg(struct insn_state *state, unsigned char reg, int base, - int offset) +static void save_reg(struct cfi_state *cfi, unsigned char reg, int base, int offset) { if (arch_callee_saved_reg(reg) && - state->regs[reg].base == CFI_UNDEFINED) { - state->regs[reg].base = base; - state->regs[reg].offset = offset; + cfi->regs[reg].base == CFI_UNDEFINED) { + cfi->regs[reg].base = base; + cfi->regs[reg].offset = offset; } } -static void restore_reg(struct insn_state *state, unsigned char reg) +static void restore_reg(struct cfi_state *cfi, unsigned char reg) { - state->regs[reg].base = initial_func_cfi.regs[reg].base; - state->regs[reg].offset = initial_func_cfi.regs[reg].offset; + cfi->regs[reg].base = initial_func_cfi.regs[reg].base; + cfi->regs[reg].offset = initial_func_cfi.regs[reg].offset; } /* @@ -1548,11 +1555,11 @@ static void restore_reg(struct insn_state *state, unsigned char reg) * 41 5d pop %r13 * c3 retq */ -static int update_insn_state(struct instruction *insn, struct insn_state *state, +static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, struct stack_op *op) { - struct cfi_reg *cfa = &state->cfa; - struct cfi_reg *regs = state->regs; + struct cfi_reg *cfa = &cfi->cfa; + struct cfi_reg *regs = cfi->regs; /* stack operations don't make sense with an undefined CFA */ if (cfa->base == CFI_UNDEFINED) { @@ -1563,8 +1570,8 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state, return 0; } - if (state->type == ORC_TYPE_REGS || state->type == ORC_TYPE_REGS_IRET) - return update_insn_state_regs(insn, state, op); + if (cfi->type == ORC_TYPE_REGS || cfi->type == ORC_TYPE_REGS_IRET) + return update_cfi_state_regs(insn, cfi, op); switch (op->dest.type) { @@ -1579,16 +1586,16 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state, /* mov %rsp, %rbp */ cfa->base = op->dest.reg; - state->bp_scratch = false; + cfi->bp_scratch = false; } else if (op->src.reg == CFI_SP && - op->dest.reg == CFI_BP && state->drap) { + op->dest.reg == CFI_BP && cfi->drap) { /* drap: mov %rsp, %rbp */ regs[CFI_BP].base = CFI_BP; - regs[CFI_BP].offset = -state->stack_size; - state->bp_scratch = false; + regs[CFI_BP].offset = -cfi->stack_size; + cfi->bp_scratch = false; } else if (op->src.reg == CFI_SP && cfa->base == CFI_SP) { @@ -1603,8 +1610,8 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state, * ... * mov %rax, %rsp */ - state->vals[op->dest.reg].base = CFI_CFA; - state->vals[op->dest.reg].offset = -state->stack_size; + cfi->vals[op->dest.reg].base = CFI_CFA; + cfi->vals[op->dest.reg].offset = -cfi->stack_size; } else if (op->src.reg == CFI_BP && op->dest.reg == CFI_SP && @@ -1615,14 +1622,14 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state, * * Restore the original stack pointer (Clang). */ - state->stack_size = -state->regs[CFI_BP].offset; + cfi->stack_size = -cfi->regs[CFI_BP].offset; } else if (op->dest.reg == cfa->base) { /* mov %reg, %rsp */ if (cfa->base == CFI_SP && - state->vals[op->src.reg].base == CFI_CFA) { + cfi->vals[op->src.reg].base == CFI_CFA) { /* * This is needed for the rare case @@ -1632,8 +1639,8 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state, * ... * mov %rcx, %rsp */ - cfa->offset = -state->vals[op->src.reg].offset; - state->stack_size = cfa->offset; + cfa->offset = -cfi->vals[op->src.reg].offset; + cfi->stack_size = cfa->offset; } else { cfa->base = CFI_UNDEFINED; @@ -1647,7 +1654,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state, if (op->dest.reg == CFI_SP && op->src.reg == CFI_SP) { /* add imm, %rsp */ - state->stack_size -= op->src.offset; + cfi->stack_size -= op->src.offset; if (cfa->base == CFI_SP) cfa->offset -= op->src.offset; break; @@ -1656,14 +1663,14 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state, if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP) { /* lea disp(%rbp), %rsp */ - state->stack_size = -(op->src.offset + regs[CFI_BP].offset); + cfi->stack_size = -(op->src.offset + regs[CFI_BP].offset); break; } if (op->src.reg == CFI_SP && cfa->base == CFI_SP) { /* drap: lea disp(%rsp), %drap */ - state->drap_reg = op->dest.reg; + cfi->drap_reg = op->dest.reg; /* * lea disp(%rsp), %reg @@ -1675,25 +1682,25 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state, * ... * mov %rcx, %rsp */ - state->vals[op->dest.reg].base = CFI_CFA; - state->vals[op->dest.reg].offset = \ - -state->stack_size + op->src.offset; + cfi->vals[op->dest.reg].base = CFI_CFA; + cfi->vals[op->dest.reg].offset = \ + -cfi->stack_size + op->src.offset; break; } - if (state->drap && op->dest.reg == CFI_SP && - op->src.reg == state->drap_reg) { + if (cfi->drap && op->dest.reg == CFI_SP && + op->src.reg == cfi->drap_reg) { /* drap: lea disp(%drap), %rsp */ cfa->base = CFI_SP; - cfa->offset = state->stack_size = -op->src.offset; - state->drap_reg = CFI_UNDEFINED; - state->drap = false; + cfa->offset = cfi->stack_size = -op->src.offset; + cfi->drap_reg = CFI_UNDEFINED; + cfi->drap = false; break; } - if (op->dest.reg == state->cfa.base) { + if (op->dest.reg == cfi->cfa.base) { WARN_FUNC("unsupported stack register modification", insn->sec, insn->offset); return -1; @@ -1703,18 +1710,18 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state, case OP_SRC_AND: if (op->dest.reg != CFI_SP || - (state->drap_reg != CFI_UNDEFINED && cfa->base != CFI_SP) || - (state->drap_reg == CFI_UNDEFINED && cfa->base != CFI_BP)) { + (cfi->drap_reg != CFI_UNDEFINED && cfa->base != CFI_SP) || + (cfi->drap_reg == CFI_UNDEFINED && cfa->base != CFI_BP)) { WARN_FUNC("unsupported stack pointer realignment", insn->sec, insn->offset); return -1; } - if (state->drap_reg != CFI_UNDEFINED) { + if (cfi->drap_reg != CFI_UNDEFINED) { /* drap: and imm, %rsp */ - cfa->base = state->drap_reg; - cfa->offset = state->stack_size = 0; - state->drap = true; + cfa->base = cfi->drap_reg; + cfa->offset = cfi->stack_size = 0; + cfi->drap = true; } /* @@ -1726,55 +1733,55 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state, case OP_SRC_POP: case OP_SRC_POPF: - if (!state->drap && op->dest.reg == cfa->base) { + if (!cfi->drap && op->dest.reg == cfa->base) { /* pop %rbp */ cfa->base = CFI_SP; } - if (state->drap && cfa->base == CFI_BP_INDIRECT && - op->dest.reg == state->drap_reg && - state->drap_offset == -state->stack_size) { + if (cfi->drap && cfa->base == CFI_BP_INDIRECT && + op->dest.reg == cfi->drap_reg && + cfi->drap_offset == -cfi->stack_size) { /* drap: pop %drap */ - cfa->base = state->drap_reg; + cfa->base = cfi->drap_reg; cfa->offset = 0; - state->drap_offset = -1; + cfi->drap_offset = -1; - } else if (regs[op->dest.reg].offset == -state->stack_size) { + } else if (regs[op->dest.reg].offset == -cfi->stack_size) { /* pop %reg */ - restore_reg(state, op->dest.reg); + restore_reg(cfi, op->dest.reg); } - state->stack_size -= 8; + cfi->stack_size -= 8; if (cfa->base == CFI_SP) cfa->offset -= 8; break; case OP_SRC_REG_INDIRECT: - if (state->drap && op->src.reg == CFI_BP && - op->src.offset == state->drap_offset) { + if (cfi->drap && op->src.reg == CFI_BP && + op->src.offset == cfi->drap_offset) { /* drap: mov disp(%rbp), %drap */ - cfa->base = state->drap_reg; + cfa->base = cfi->drap_reg; cfa->offset = 0; - state->drap_offset = -1; + cfi->drap_offset = -1; } - if (state->drap && op->src.reg == CFI_BP && + if (cfi->drap && op->src.reg == CFI_BP && op->src.offset == regs[op->dest.reg].offset) { /* drap: mov disp(%rbp), %reg */ - restore_reg(state, op->dest.reg); + restore_reg(cfi, op->dest.reg); } else if (op->src.reg == cfa->base && op->src.offset == regs[op->dest.reg].offset + cfa->offset) { /* mov disp(%rbp), %reg */ /* mov disp(%rsp), %reg */ - restore_reg(state, op->dest.reg); + restore_reg(cfi, op->dest.reg); } break; @@ -1789,78 +1796,78 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state, case OP_DEST_PUSH: case OP_DEST_PUSHF: - state->stack_size += 8; + cfi->stack_size += 8; if (cfa->base == CFI_SP) cfa->offset += 8; if (op->src.type != OP_SRC_REG) break; - if (state->drap) { - if (op->src.reg == cfa->base && op->src.reg == state->drap_reg) { + if (cfi->drap) { + if (op->src.reg == cfa->base && op->src.reg == cfi->drap_reg) { /* drap: push %drap */ cfa->base = CFI_BP_INDIRECT; - cfa->offset = -state->stack_size; + cfa->offset = -cfi->stack_size; /* save drap so we know when to restore it */ - state->drap_offset = -state->stack_size; + cfi->drap_offset = -cfi->stack_size; - } else if (op->src.reg == CFI_BP && cfa->base == state->drap_reg) { + } else if (op->src.reg == CFI_BP && cfa->base == cfi->drap_reg) { /* drap: push %rbp */ - state->stack_size = 0; + cfi->stack_size = 0; } else if (regs[op->src.reg].base == CFI_UNDEFINED) { /* drap: push %reg */ - save_reg(state, op->src.reg, CFI_BP, -state->stack_size); + save_reg(cfi, op->src.reg, CFI_BP, -cfi->stack_size); } } else { /* push %reg */ - save_reg(state, op->src.reg, CFI_CFA, -state->stack_size); + save_reg(cfi, op->src.reg, CFI_CFA, -cfi->stack_size); } /* detect when asm code uses rbp as a scratch register */ if (!no_fp && insn->func && op->src.reg == CFI_BP && cfa->base != CFI_BP) - state->bp_scratch = true; + cfi->bp_scratch = true; break; case OP_DEST_REG_INDIRECT: - if (state->drap) { - if (op->src.reg == cfa->base && op->src.reg == state->drap_reg) { + if (cfi->drap) { + if (op->src.reg == cfa->base && op->src.reg == cfi->drap_reg) { /* drap: mov %drap, disp(%rbp) */ cfa->base = CFI_BP_INDIRECT; cfa->offset = op->dest.offset; /* save drap offset so we know when to restore it */ - state->drap_offset = op->dest.offset; + cfi->drap_offset = op->dest.offset; } else if (regs[op->src.reg].base == CFI_UNDEFINED) { /* drap: mov reg, disp(%rbp) */ - save_reg(state, op->src.reg, CFI_BP, op->dest.offset); + save_reg(cfi, op->src.reg, CFI_BP, op->dest.offset); } } else if (op->dest.reg == cfa->base) { /* mov reg, disp(%rbp) */ /* mov reg, disp(%rsp) */ - save_reg(state, op->src.reg, CFI_CFA, - op->dest.offset - state->cfa.offset); + save_reg(cfi, op->src.reg, CFI_CFA, + op->dest.offset - cfi->cfa.offset); } break; case OP_DEST_LEAVE: - if ((!state->drap && cfa->base != CFI_BP) || - (state->drap && cfa->base != state->drap_reg)) { + if ((!cfi->drap && cfa->base != CFI_BP) || + (cfi->drap && cfa->base != cfi->drap_reg)) { WARN_FUNC("leave instruction with modified stack frame", insn->sec, insn->offset); return -1; @@ -1868,10 +1875,10 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state, /* leave (mov %rbp, %rsp; pop %rbp) */ - state->stack_size = -state->regs[CFI_BP].offset - 8; - restore_reg(state, CFI_BP); + cfi->stack_size = -cfi->regs[CFI_BP].offset - 8; + restore_reg(cfi, CFI_BP); - if (!state->drap) { + if (!cfi->drap) { cfa->base = CFI_SP; cfa->offset -= 8; } @@ -1886,7 +1893,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state, } /* pop mem */ - state->stack_size -= 8; + cfi->stack_size -= 8; if (cfa->base == CFI_SP) cfa->offset -= 8; @@ -1908,7 +1915,7 @@ static int handle_insn_ops(struct instruction *insn, struct insn_state *state) list_for_each_entry(op, &insn->stack_ops, list) { int res; - res = update_insn_state(insn, state, op); + res = update_cfi_state(insn, &state->cfi, op); if (res) return res; @@ -1937,41 +1944,44 @@ static int handle_insn_ops(struct instruction *insn, struct insn_state *state) return 0; } -static bool insn_state_match(struct instruction *insn, struct insn_state *state) +static bool insn_cfi_match(struct instruction *insn, struct cfi_state *cfi2) { - struct insn_state *state1 = &insn->state, *state2 = state; + struct cfi_state *cfi1 = &insn->cfi; int i; - if (memcmp(&state1->cfa, &state2->cfa, sizeof(state1->cfa))) { + if (memcmp(&cfi1->cfa, &cfi2->cfa, sizeof(cfi1->cfa))) { + WARN_FUNC("stack state mismatch: cfa1=%d%+d cfa2=%d%+d", insn->sec, insn->offset, - state1->cfa.base, state1->cfa.offset, - state2->cfa.base, state2->cfa.offset); + cfi1->cfa.base, cfi1->cfa.offset, + cfi2->cfa.base, cfi2->cfa.offset); - } else if (memcmp(&state1->regs, &state2->regs, sizeof(state1->regs))) { + } else if (memcmp(&cfi1->regs, &cfi2->regs, sizeof(cfi1->regs))) { for (i = 0; i < CFI_NUM_REGS; i++) { - if (!memcmp(&state1->regs[i], &state2->regs[i], + if (!memcmp(&cfi1->regs[i], &cfi2->regs[i], sizeof(struct cfi_reg))) continue; WARN_FUNC("stack state mismatch: reg1[%d]=%d%+d reg2[%d]=%d%+d", insn->sec, insn->offset, - i, state1->regs[i].base, state1->regs[i].offset, - i, state2->regs[i].base, state2->regs[i].offset); + i, cfi1->regs[i].base, cfi1->regs[i].offset, + i, cfi2->regs[i].base, cfi2->regs[i].offset); break; } - } else if (state1->type != state2->type) { - WARN_FUNC("stack state mismatch: type1=%d type2=%d", - insn->sec, insn->offset, state1->type, state2->type); + } else if (cfi1->type != cfi2->type) { + + WARN_FUNC("stack state mismatch: type1=%d type2=%d", + insn->sec, insn->offset, cfi1->type, cfi2->type); + + } else if (cfi1->drap != cfi2->drap || + (cfi1->drap && cfi1->drap_reg != cfi2->drap_reg) || + (cfi1->drap && cfi1->drap_offset != cfi2->drap_offset)) { - } else if (state1->drap != state2->drap || - (state1->drap && state1->drap_reg != state2->drap_reg) || - (state1->drap && state1->drap_offset != state2->drap_offset)) { WARN_FUNC("stack state mismatch: drap1=%d(%d,%d) drap2=%d(%d,%d)", insn->sec, insn->offset, - state1->drap, state1->drap_reg, state1->drap_offset, - state2->drap, state2->drap_reg, state2->drap_offset); + cfi1->drap, cfi1->drap_reg, cfi1->drap_offset, + cfi2->drap, cfi2->drap_reg, cfi2->drap_offset); } else return true; @@ -2049,7 +2059,7 @@ static int validate_return(struct symbol *func, struct instruction *insn, struct return 1; } - if (state->bp_scratch) { + if (state->cfi.bp_scratch) { WARN_FUNC("BP used as a scratch register", insn->sec, insn->offset); return 1; @@ -2098,7 +2108,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, visited = 1 << state.uaccess; if (insn->visited) { - if (!insn->hint && !insn_state_match(insn, &state)) + if (!insn->hint && !insn_cfi_match(insn, &state.cfi)) return 1; if (insn->visited & visited) @@ -2106,9 +2116,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, } if (insn->hint) - state = insn->state; + state.cfi = insn->cfi; else - insn->state = state; + insn->cfi = state.cfi; insn->visited |= visited; @@ -2261,7 +2271,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, return 0; if (!next_insn) { - if (state.cfa.base == CFI_UNDEFINED) + if (state.cfi.cfa.base == CFI_UNDEFINED) return 0; WARN("%s: unexpected end of section", sec->name); return 1; @@ -2430,10 +2440,10 @@ static int validate_section(struct objtool_file *file, struct section *sec) continue; clear_insn_state(&state); - state.cfa = initial_func_cfi.cfa; - memcpy(&state.regs, &initial_func_cfi.regs, + state.cfi.cfa = initial_func_cfi.cfa; + memcpy(&state.cfi.regs, &initial_func_cfi.regs, CFI_NUM_REGS * sizeof(struct cfi_reg)); - state.stack_size = initial_func_cfi.cfa.offset; + state.cfi.stack_size = initial_func_cfi.cfa.offset; state.uaccess = func->uaccess_safe; diff --git a/tools/objtool/check.h b/tools/objtool/check.h index 7c30760bee74..99413d4ca4b1 100644 --- a/tools/objtool/check.h +++ b/tools/objtool/check.h @@ -14,15 +14,10 @@ #include struct insn_state { - struct cfi_reg cfa; - struct cfi_reg regs[CFI_NUM_REGS]; - int stack_size; - unsigned char type; - bool bp_scratch; - bool drap, end, uaccess, df; + struct cfi_state cfi; unsigned int uaccess_stack; - int drap_reg, drap_offset; - struct cfi_reg vals[CFI_NUM_REGS]; + bool uaccess; + bool df; }; struct instruction { @@ -45,7 +40,7 @@ struct instruction { struct list_head alts; struct symbol *func; struct list_head stack_ops; - struct insn_state state; + struct cfi_state cfi; struct orc_entry orc; }; diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c index 4c0dabd28000..2cf640fca01d 100644 --- a/tools/objtool/orc_gen.c +++ b/tools/objtool/orc_gen.c @@ -16,10 +16,10 @@ int create_orc(struct objtool_file *file) for_each_insn(file, insn) { struct orc_entry *orc = &insn->orc; - struct cfi_reg *cfa = &insn->state.cfa; - struct cfi_reg *bp = &insn->state.regs[CFI_BP]; + struct cfi_reg *cfa = &insn->cfi.cfa; + struct cfi_reg *bp = &insn->cfi.regs[CFI_BP]; - orc->end = insn->state.end; + orc->end = insn->cfi.end; if (cfa->base == CFI_UNDEFINED) { orc->sp_reg = ORC_REG_UNDEFINED; @@ -75,7 +75,7 @@ int create_orc(struct objtool_file *file) orc->sp_offset = cfa->offset; orc->bp_offset = bp->offset; - orc->type = insn->state.type; + orc->type = insn->cfi.type; } return 0; From c4a33939a7eb396acbb05569e57eebe4374cc57c Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Tue, 10 Mar 2020 18:57:41 +0100 Subject: [PATCH 21/54] objtool: Implement noinstr validation Validate that any call out of .noinstr.text is in between instr_begin() and instr_end() annotations. This annotation is useful to ensure correct behaviour wrt tracing sensitive code like entry/exit and idle code. When we run code in a sensitive context we want a guarantee no unknown code is ran. Since this validation relies on knowing the section of call destination symbols, we must run it on vmlinux.o instead of on individual object files. Add two options: -d/--duplicate "duplicate validation for vmlinux" -l/--vmlinux "vmlinux.o validation" Where the latter auto-detects when objname ends with "vmlinux.o" and the former will force all validations, also those already done on !vmlinux object files. Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115119.106268040@infradead.org Signed-off-by: Ingo Molnar --- tools/objtool/builtin-check.c | 11 +++- tools/objtool/builtin.h | 2 +- tools/objtool/check.c | 98 +++++++++++++++++++++++++++++++++++ tools/objtool/check.h | 3 ++ tools/objtool/elf.h | 2 +- 5 files changed, 112 insertions(+), 4 deletions(-) diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c index 10fbe75ab43d..be42b716166b 100644 --- a/tools/objtool/builtin-check.c +++ b/tools/objtool/builtin-check.c @@ -14,10 +14,11 @@ */ #include +#include #include "builtin.h" #include "check.h" -bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats; +bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats, validate_dup, vmlinux; static const char * const check_usage[] = { "objtool check [] file.o", @@ -32,12 +33,14 @@ const struct option check_options[] = { OPT_BOOLEAN('b', "backtrace", &backtrace, "unwind on error"), OPT_BOOLEAN('a', "uaccess", &uaccess, "enable uaccess checking"), OPT_BOOLEAN('s', "stats", &stats, "print statistics"), + OPT_BOOLEAN('d', "duplicate", &validate_dup, "duplicate validation for vmlinux.o"), + OPT_BOOLEAN('l', "vmlinux", &vmlinux, "vmlinux.o validation"), OPT_END(), }; int cmd_check(int argc, const char **argv) { - const char *objname; + const char *objname, *s; argc = parse_options(argc, argv, check_options, check_usage, 0); @@ -46,5 +49,9 @@ int cmd_check(int argc, const char **argv) objname = argv[0]; + s = strstr(objname, "vmlinux.o"); + if (s && !s[9]) + vmlinux = true; + return check(objname, false); } diff --git a/tools/objtool/builtin.h b/tools/objtool/builtin.h index 0b907902ee79..85c979caa367 100644 --- a/tools/objtool/builtin.h +++ b/tools/objtool/builtin.h @@ -8,7 +8,7 @@ #include extern const struct option check_options[]; -extern bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats; +extern bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats, validate_dup, vmlinux; extern int cmd_check(int argc, const char **argv); extern int cmd_orc(int argc, const char **argv); diff --git a/tools/objtool/check.c b/tools/objtool/check.c index abf97159bf1e..87e528c2840c 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -257,6 +257,9 @@ static int decode_instructions(struct objtool_file *file) strncmp(sec->name, ".discard.", 9)) sec->text = true; + if (!strcmp(sec->name, ".noinstr.text")) + sec->noinstr = true; + for (offset = 0; offset < sec->len; offset += insn->len) { insn = malloc(sizeof(*insn)); if (!insn) { @@ -1340,6 +1343,53 @@ static int read_retpoline_hints(struct objtool_file *file) return 0; } +static int read_instr_hints(struct objtool_file *file) +{ + struct section *sec; + struct instruction *insn; + struct rela *rela; + + sec = find_section_by_name(file->elf, ".rela.discard.instr_end"); + if (!sec) + return 0; + + list_for_each_entry(rela, &sec->rela_list, list) { + if (rela->sym->type != STT_SECTION) { + WARN("unexpected relocation symbol type in %s", sec->name); + return -1; + } + + insn = find_insn(file, rela->sym->sec, rela->addend); + if (!insn) { + WARN("bad .discard.instr_end entry"); + return -1; + } + + insn->instr--; + } + + sec = find_section_by_name(file->elf, ".rela.discard.instr_begin"); + if (!sec) + return 0; + + list_for_each_entry(rela, &sec->rela_list, list) { + if (rela->sym->type != STT_SECTION) { + WARN("unexpected relocation symbol type in %s", sec->name); + return -1; + } + + insn = find_insn(file, rela->sym->sec, rela->addend); + if (!insn) { + WARN("bad .discard.instr_begin entry"); + return -1; + } + + insn->instr++; + } + + return 0; +} + static void mark_rodata(struct objtool_file *file) { struct section *sec; @@ -1411,6 +1461,10 @@ static int decode_sections(struct objtool_file *file) if (ret) return ret; + ret = read_instr_hints(file); + if (ret) + return ret; + return 0; } @@ -2007,6 +2061,13 @@ static inline const char *call_dest_name(struct instruction *insn) static int validate_call(struct instruction *insn, struct insn_state *state) { + if (state->noinstr && state->instr <= 0 && + (!insn->call_dest || insn->call_dest->sec != insn->sec)) { + WARN_FUNC("call to %s() leaves .noinstr.text section", + insn->sec, insn->offset, call_dest_name(insn)); + return 1; + } + if (state->uaccess && !func_uaccess_safe(insn->call_dest)) { WARN_FUNC("call to %s() with UACCESS enabled", insn->sec, insn->offset, call_dest_name(insn)); @@ -2035,6 +2096,12 @@ static int validate_sibling_call(struct instruction *insn, struct insn_state *st static int validate_return(struct symbol *func, struct instruction *insn, struct insn_state *state) { + if (state->noinstr && state->instr > 0) { + WARN_FUNC("return with instrumentation enabled", + insn->sec, insn->offset); + return 1; + } + if (state->uaccess && !func_uaccess_safe(func)) { WARN_FUNC("return with UACCESS enabled", insn->sec, insn->offset); @@ -2115,6 +2182,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, return 0; } + if (state.noinstr) + state.instr += insn->instr; + if (insn->hint) state.cfi = insn->cfi; else @@ -2422,6 +2492,14 @@ static int validate_section(struct objtool_file *file, struct section *sec) struct insn_state state; int ret, warnings = 0; + /* + * We need the full vmlinux for noinstr validation, otherwise we can + * not correctly determine insn->call_dest->sec (external symbols do + * not have a section). + */ + if (vmlinux) + state.noinstr = sec->noinstr; + list_for_each_entry(func, &sec->symbol_list, list) { if (func->type != STT_FUNC) continue; @@ -2456,6 +2534,17 @@ static int validate_section(struct objtool_file *file, struct section *sec) return warnings; } +static int validate_vmlinux_functions(struct objtool_file *file) +{ + struct section *sec; + + sec = find_section_by_name(file->elf, ".noinstr.text"); + if (!sec) + return 0; + + return validate_section(file, sec); +} + static int validate_functions(struct objtool_file *file) { struct section *sec; @@ -2513,6 +2602,15 @@ int check(const char *_objname, bool orc) if (list_empty(&file.insn_list)) goto out; + if (vmlinux && !validate_dup) { + ret = validate_vmlinux_functions(&file); + if (ret < 0) + goto out; + + warnings += ret; + goto out; + } + if (retpoline) { ret = validate_retpoline(&file); if (ret < 0) diff --git a/tools/objtool/check.h b/tools/objtool/check.h index 99413d4ca4b1..12a9660c960b 100644 --- a/tools/objtool/check.h +++ b/tools/objtool/check.h @@ -18,6 +18,8 @@ struct insn_state { unsigned int uaccess_stack; bool uaccess; bool df; + bool noinstr; + s8 instr; }; struct instruction { @@ -31,6 +33,7 @@ struct instruction { bool alt_group, dead_end, ignore, ignore_alts; bool hint; bool retpoline_safe; + s8 instr; u8 visited; u8 ret_offset; struct symbol *call_dest; diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h index 0b79c2353a21..eb79cb999209 100644 --- a/tools/objtool/elf.h +++ b/tools/objtool/elf.h @@ -39,7 +39,7 @@ struct section { char *name; int idx; unsigned int len; - bool changed, text, rodata; + bool changed, text, rodata, noinstr; }; struct symbol { From 34f7c96d96d5e11b03a612017fcc3a6e645bb481 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Thu, 12 Mar 2020 14:29:38 +0100 Subject: [PATCH 22/54] objtool: Optimize !vmlinux.o again When doing kbuild tests to see if the objtool changes affected those I found that there was a measurable regression: pre post real 1m13.594 1m16.488s user 34m58.246s 35m23.947s sys 4m0.393s 4m27.312s Perf showed that for small files the increased hash-table sizes were a measurable difference. Since we already have -l "vmlinux" to distinguish between the modes, make it also use a smaller portion of the hash-tables. This flips it into a small win: real 1m14.143s user 34m49.292s sys 3m44.746s Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115119.167588731@infradead.org Signed-off-by: Ingo Molnar --- tools/objtool/elf.c | 62 ++++++++++++++++++++++++++++------------- tools/objtool/elf.h | 13 +++++---- tools/objtool/orc_gen.c | 3 +- 3 files changed, 52 insertions(+), 26 deletions(-) diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index c4857fa3f1d1..f26bb3e8db7b 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -27,6 +27,22 @@ static inline u32 str_hash(const char *str) return jhash(str, strlen(str), 0); } +static inline int elf_hash_bits(void) +{ + return vmlinux ? ELF_HASH_BITS : 16; +} + +#define elf_hash_add(hashtable, node, key) \ + hlist_add_head(node, &hashtable[hash_min(key, elf_hash_bits())]) + +static void elf_hash_init(struct hlist_head *table) +{ + __hash_init(table, 1U << elf_hash_bits()); +} + +#define elf_hash_for_each_possible(name, obj, member, key) \ + hlist_for_each_entry(obj, &name[hash_min(key, elf_hash_bits())], member) + static void rb_add(struct rb_root *tree, struct rb_node *node, int (*cmp)(struct rb_node *, const struct rb_node *)) { @@ -115,7 +131,7 @@ struct section *find_section_by_name(struct elf *elf, const char *name) { struct section *sec; - hash_for_each_possible(elf->section_name_hash, sec, name_hash, str_hash(name)) + elf_hash_for_each_possible(elf->section_name_hash, sec, name_hash, str_hash(name)) if (!strcmp(sec->name, name)) return sec; @@ -127,7 +143,7 @@ static struct section *find_section_by_index(struct elf *elf, { struct section *sec; - hash_for_each_possible(elf->section_hash, sec, hash, idx) + elf_hash_for_each_possible(elf->section_hash, sec, hash, idx) if (sec->idx == idx) return sec; @@ -138,7 +154,7 @@ static struct symbol *find_symbol_by_index(struct elf *elf, unsigned int idx) { struct symbol *sym; - hash_for_each_possible(elf->symbol_hash, sym, hash, idx) + elf_hash_for_each_possible(elf->symbol_hash, sym, hash, idx) if (sym->idx == idx) return sym; @@ -205,7 +221,7 @@ struct symbol *find_symbol_by_name(struct elf *elf, const char *name) { struct symbol *sym; - hash_for_each_possible(elf->symbol_name_hash, sym, name_hash, str_hash(name)) + elf_hash_for_each_possible(elf->symbol_name_hash, sym, name_hash, str_hash(name)) if (!strcmp(sym->name, name)) return sym; @@ -224,7 +240,7 @@ struct rela *find_rela_by_dest_range(struct elf *elf, struct section *sec, sec = sec->rela; for_offset_range(o, offset, offset + len) { - hash_for_each_possible(elf->rela_hash, rela, hash, + elf_hash_for_each_possible(elf->rela_hash, rela, hash, sec_offset_hash(sec, o)) { if (rela->sec != sec) continue; @@ -309,8 +325,8 @@ static int read_sections(struct elf *elf) sec->len = sec->sh.sh_size; list_add_tail(&sec->list, &elf->sections); - hash_add(elf->section_hash, &sec->hash, sec->idx); - hash_add(elf->section_name_hash, &sec->name_hash, str_hash(sec->name)); + elf_hash_add(elf->section_hash, &sec->hash, sec->idx); + elf_hash_add(elf->section_name_hash, &sec->name_hash, str_hash(sec->name)); } if (stats) @@ -394,8 +410,8 @@ static int read_symbols(struct elf *elf) else entry = &sym->sec->symbol_list; list_add(&sym->list, entry); - hash_add(elf->symbol_hash, &sym->hash, sym->idx); - hash_add(elf->symbol_name_hash, &sym->name_hash, str_hash(sym->name)); + elf_hash_add(elf->symbol_hash, &sym->hash, sym->idx); + elf_hash_add(elf->symbol_name_hash, &sym->name_hash, str_hash(sym->name)); } if (stats) @@ -456,6 +472,14 @@ err: return -1; } +void elf_add_rela(struct elf *elf, struct rela *rela) +{ + struct section *sec = rela->sec; + + list_add_tail(&rela->list, &sec->rela_list); + elf_hash_add(elf->rela_hash, &rela->hash, rela_hash(rela)); +} + static int read_relas(struct elf *elf) { struct section *sec; @@ -503,8 +527,7 @@ static int read_relas(struct elf *elf) return -1; } - list_add_tail(&rela->list, &sec->rela_list); - hash_add(elf->rela_hash, &rela->hash, rela_hash(rela)); + elf_add_rela(elf, rela); nr_rela++; } max_rela = max(max_rela, nr_rela); @@ -531,15 +554,16 @@ struct elf *elf_read(const char *name, int flags) perror("malloc"); return NULL; } - memset(elf, 0, sizeof(*elf)); + memset(elf, 0, offsetof(struct elf, sections)); - hash_init(elf->symbol_hash); - hash_init(elf->symbol_name_hash); - hash_init(elf->section_hash); - hash_init(elf->section_name_hash); - hash_init(elf->rela_hash); INIT_LIST_HEAD(&elf->sections); + elf_hash_init(elf->symbol_hash); + elf_hash_init(elf->symbol_name_hash); + elf_hash_init(elf->section_hash); + elf_hash_init(elf->section_name_hash); + elf_hash_init(elf->rela_hash); + elf->fd = open(name, flags); if (elf->fd == -1) { fprintf(stderr, "objtool: Can't open '%s': %s\n", @@ -676,8 +700,8 @@ struct section *elf_create_section(struct elf *elf, const char *name, shstrtab->changed = true; list_add_tail(&sec->list, &elf->sections); - hash_add(elf->section_hash, &sec->hash, sec->idx); - hash_add(elf->section_name_hash, &sec->name_hash, str_hash(sec->name)); + elf_hash_add(elf->section_hash, &sec->hash, sec->idx); + elf_hash_add(elf->section_name_hash, &sec->name_hash, str_hash(sec->name)); return sec; } diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h index eb79cb999209..2811d04346c9 100644 --- a/tools/objtool/elf.h +++ b/tools/objtool/elf.h @@ -70,17 +70,19 @@ struct rela { bool jump_table_start; }; +#define ELF_HASH_BITS 20 + struct elf { Elf *elf; GElf_Ehdr ehdr; int fd; char *name; struct list_head sections; - DECLARE_HASHTABLE(symbol_hash, 20); - DECLARE_HASHTABLE(symbol_name_hash, 20); - DECLARE_HASHTABLE(section_hash, 16); - DECLARE_HASHTABLE(section_name_hash, 16); - DECLARE_HASHTABLE(rela_hash, 20); + DECLARE_HASHTABLE(symbol_hash, ELF_HASH_BITS); + DECLARE_HASHTABLE(symbol_name_hash, ELF_HASH_BITS); + DECLARE_HASHTABLE(section_hash, ELF_HASH_BITS); + DECLARE_HASHTABLE(section_name_hash, ELF_HASH_BITS); + DECLARE_HASHTABLE(rela_hash, ELF_HASH_BITS); }; #define OFFSET_STRIDE_BITS 4 @@ -127,6 +129,7 @@ struct section *elf_create_rela_section(struct elf *elf, struct section *base); int elf_rebuild_rela_section(struct section *sec); int elf_write(struct elf *elf); void elf_close(struct elf *elf); +void elf_add_rela(struct elf *elf, struct rela *rela); #define for_each_sec(file, sec) \ list_for_each_entry(sec, &file->elf->sections, list) diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c index 2cf640fca01d..9d2bf2daaaa6 100644 --- a/tools/objtool/orc_gen.c +++ b/tools/objtool/orc_gen.c @@ -130,8 +130,7 @@ static int create_orc_entry(struct elf *elf, struct section *u_sec, struct secti rela->offset = idx * sizeof(int); rela->sec = ip_relasec; - list_add_tail(&rela->list, &ip_relasec->rela_list); - hash_add(elf->rela_hash, &rela->hash, rela_hash(rela)); + elf_add_rela(elf, rela); return 0; } From 87ecb582f0ac85886398dde8c3cdb2225cac7786 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Mon, 16 Mar 2020 15:47:27 +0100 Subject: [PATCH 23/54] objtool: Use sec_offset_hash() for insn_hash In preparation for find_insn_containing(), change insn_hash to use sec_offset_hash(). This actually reduces runtime; probably because mixing in the section index reduces the collisions due to text sections all starting their instructions at offset 0. Runtime on vmlinux.o from 3.1 to 2.5 seconds. Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115119.227240432@infradead.org Signed-off-by: Ingo Molnar --- tools/objtool/check.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 87e528c2840c..923652ba5f9a 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -34,9 +34,10 @@ struct instruction *find_insn(struct objtool_file *file, { struct instruction *insn; - hash_for_each_possible(file->insn_hash, insn, hash, offset) + hash_for_each_possible(file->insn_hash, insn, hash, sec_offset_hash(sec, offset)) { if (insn->sec == sec && insn->offset == offset) return insn; + } return NULL; } @@ -282,7 +283,7 @@ static int decode_instructions(struct objtool_file *file) if (ret) goto err; - hash_add(file->insn_hash, &insn->hash, insn->offset); + hash_add(file->insn_hash, &insn->hash, sec_offset_hash(sec, insn->offset)); list_add_tail(&insn->list, &file->insn_list); nr_insns++; } From 6804c1afd794c8135351faaa69e1503ee11393ac Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Wed, 18 Mar 2020 13:33:54 +0100 Subject: [PATCH 24/54] kbuild/objtool: Add objtool-vmlinux.o pass Now that objtool is capable of processing vmlinux.o and actually has something useful to do there, (conditionally) add it to the final link pass. This will increase build time by a few seconds. Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115119.287494491@infradead.org Signed-off-by: Ingo Molnar --- lib/Kconfig.debug | 5 +++++ scripts/link-vmlinux.sh | 24 ++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 21d9c5f6e7ec..977b5a190297 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -369,6 +369,11 @@ config STACK_VALIDATION For more information, see tools/objtool/Documentation/stack-validation.txt. +config VMLINUX_VALIDATION + bool + depends on STACK_VALIDATION && DEBUG_ENTRY && !PARAVIRT + default y + config DEBUG_FORCE_WEAK_PER_CPU bool "Force weak per-cpu definitions" depends on DEBUG_KERNEL diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index d09ab4afbda4..3adef49250af 100755 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -55,6 +55,29 @@ modpost_link() ${LD} ${KBUILD_LDFLAGS} -r -o ${1} ${objects} } +objtool_link() +{ + local objtoolopt; + + if [ -n "${CONFIG_VMLINUX_VALIDATION}" ]; then + objtoolopt="check" + if [ -z "${CONFIG_FRAME_POINTER}" ]; then + objtoolopt="${objtoolopt} --no-fp" + fi + if [ -n "${CONFIG_GCOV_KERNEL}" ]; then + objtoolopt="${objtoolopt} --no-unreachable" + fi + if [ -n "${CONFIG_RETPOLINE}" ]; then + objtoolopt="${objtoolopt} --retpoline" + fi + if [ -n "${CONFIG_X86_SMAP}" ]; then + objtoolopt="${objtoolopt} --uaccess" + fi + info OBJTOOL ${1} + tools/objtool/objtool ${objtoolopt} ${1} + fi +} + # Link of vmlinux # ${1} - output file # ${2}, ${3}, ... - optional extra .o files @@ -251,6 +274,7 @@ ${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init need-builtin=1 #link vmlinux.o info LD vmlinux.o modpost_link vmlinux.o +objtool_link vmlinux.o # modpost vmlinux.o to check for section mismatches ${MAKE} -f "${srctree}/scripts/Makefile.modpost" MODPOST_VMLINUX=1 From da837bd6f1994f780325649e8eee7d9b01c5ee4d Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Mon, 23 Mar 2020 21:11:14 +0100 Subject: [PATCH 25/54] objtool: Avoid iterating !text section symbols validate_functions() iterates all sections their symbols; this is pointless to do for !text sections as they won't have instructions anyway. Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115119.346582716@infradead.org Signed-off-by: Ingo Molnar --- tools/objtool/check.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 923652ba5f9a..e201aa1d01f5 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2551,8 +2551,12 @@ static int validate_functions(struct objtool_file *file) struct section *sec; int warnings = 0; - for_each_sec(file, sec) + for_each_sec(file, sec) { + if (!(sec->sh.sh_flags & SHF_EXECINSTR)) + continue; + warnings += validate_section(file, sec); + } return warnings; } From 4b5e2e7ffef87ae864f3f4546ee5753556e7550b Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Mon, 23 Mar 2020 21:17:50 +0100 Subject: [PATCH 26/54] objtool: Rearrange validate_section() In preparation of further changes, once again break out the loop body. No functional changes intended. Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115119.405863817@infradead.org Signed-off-by: Ingo Molnar --- tools/objtool/check.c | 51 ++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index e201aa1d01f5..f49bf83e0501 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2486,12 +2486,37 @@ static bool ignore_unreachable_insn(struct instruction *insn) return false; } +static int validate_symbol(struct objtool_file *file, struct section *sec, + struct symbol *sym, struct insn_state *state) +{ + struct instruction *insn; + int ret; + + if (!sym->len) { + WARN("%s() is missing an ELF size annotation", sym->name); + return 1; + } + + if (sym->pfunc != sym || sym->alias != sym) + return 0; + + insn = find_insn(file, sec, sym->offset); + if (!insn || insn->ignore || insn->visited) + return 0; + + state->uaccess = sym->uaccess_safe; + + ret = validate_branch(file, insn->func, insn, *state); + if (ret && backtrace) + BT_FUNC("<=== (sym)", insn); + return ret; +} + static int validate_section(struct objtool_file *file, struct section *sec) { - struct symbol *func; - struct instruction *insn; struct insn_state state; - int ret, warnings = 0; + struct symbol *func; + int warnings = 0; /* * We need the full vmlinux for noinstr validation, otherwise we can @@ -2505,31 +2530,13 @@ static int validate_section(struct objtool_file *file, struct section *sec) if (func->type != STT_FUNC) continue; - if (!func->len) { - WARN("%s() is missing an ELF size annotation", - func->name); - warnings++; - } - - if (func->pfunc != func || func->alias != func) - continue; - - insn = find_insn(file, sec, func->offset); - if (!insn || insn->ignore || insn->visited) - continue; - clear_insn_state(&state); state.cfi.cfa = initial_func_cfi.cfa; memcpy(&state.cfi.regs, &initial_func_cfi.regs, CFI_NUM_REGS * sizeof(struct cfi_reg)); state.cfi.stack_size = initial_func_cfi.cfa.offset; - state.uaccess = func->uaccess_safe; - - ret = validate_branch(file, func, insn, state); - if (ret && backtrace) - BT_FUNC("<=== (func)", insn); - warnings += ret; + warnings += validate_symbol(file, sec, func, &state); } return warnings; From 932f8e987bfdcfc2365177978a30fdc0d6f6bd60 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Mon, 23 Mar 2020 18:26:03 +0100 Subject: [PATCH 27/54] objtool: Add STT_NOTYPE noinstr validation Make sure to also check STT_NOTYPE symbols for noinstr violations. Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115119.465335884@infradead.org Signed-off-by: Ingo Molnar --- tools/objtool/check.c | 46 ++++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index f49bf83e0501..0d9f9cfc27be 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -229,10 +229,18 @@ static void init_cfi_state(struct cfi_state *cfi) cfi->drap_offset = -1; } -static void clear_insn_state(struct insn_state *state) +static void init_insn_state(struct insn_state *state, struct section *sec) { memset(state, 0, sizeof(*state)); init_cfi_state(&state->cfi); + + /* + * We need the full vmlinux for noinstr validation, otherwise we can + * not correctly determine insn->call_dest->sec (external symbols do + * not have a section). + */ + if (vmlinux && sec) + state->noinstr = sec->noinstr; } /* @@ -2354,24 +2362,34 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, return 0; } -static int validate_unwind_hints(struct objtool_file *file) +static int validate_unwind_hints(struct objtool_file *file, struct section *sec) { struct instruction *insn; - int ret, warnings = 0; struct insn_state state; + int ret, warnings = 0; if (!file->hints) return 0; - clear_insn_state(&state); + init_insn_state(&state, sec); - for_each_insn(file, insn) { + if (sec) { + insn = find_insn(file, sec, 0); + if (!insn) + return 0; + } else { + insn = list_first_entry(&file->insn_list, typeof(*insn), list); + } + + while (&insn->list != &file->insn_list && (!sec || insn->sec == sec)) { if (insn->hint && !insn->visited) { ret = validate_branch(file, insn->func, insn, state); if (ret && backtrace) BT_FUNC("<=== (hint)", insn); warnings += ret; } + + insn = list_next_entry(insn, list); } return warnings; @@ -2518,19 +2536,11 @@ static int validate_section(struct objtool_file *file, struct section *sec) struct symbol *func; int warnings = 0; - /* - * We need the full vmlinux for noinstr validation, otherwise we can - * not correctly determine insn->call_dest->sec (external symbols do - * not have a section). - */ - if (vmlinux) - state.noinstr = sec->noinstr; - list_for_each_entry(func, &sec->symbol_list, list) { if (func->type != STT_FUNC) continue; - clear_insn_state(&state); + init_insn_state(&state, sec); state.cfi.cfa = initial_func_cfi.cfa; memcpy(&state.cfi.regs, &initial_func_cfi.regs, CFI_NUM_REGS * sizeof(struct cfi_reg)); @@ -2545,12 +2555,16 @@ static int validate_section(struct objtool_file *file, struct section *sec) static int validate_vmlinux_functions(struct objtool_file *file) { struct section *sec; + int warnings = 0; sec = find_section_by_name(file->elf, ".noinstr.text"); if (!sec) return 0; - return validate_section(file, sec); + warnings += validate_section(file, sec); + warnings += validate_unwind_hints(file, sec); + + return warnings; } static int validate_functions(struct objtool_file *file) @@ -2635,7 +2649,7 @@ int check(const char *_objname, bool orc) goto out; warnings += ret; - ret = validate_unwind_hints(&file); + ret = validate_unwind_hints(&file, NULL); if (ret < 0) goto out; warnings += ret; From 0cc9ac8db0b447922d9af77916cd7941fc784b64 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 25 Mar 2020 17:18:17 +0100 Subject: [PATCH 28/54] objtool: Also consider .entry.text as noinstr Consider all of .entry.text as noinstr. This gets us coverage across the PTI boundary. While we could add everything .noinstr.text into .entry.text that would bloat the amount of code in the user mapping. Signed-off-by: Thomas Gleixner Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200416115119.525037514@infradead.org Signed-off-by: Ingo Molnar --- tools/objtool/check.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 0d9f9cfc27be..f2a84271e807 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -266,7 +266,8 @@ static int decode_instructions(struct objtool_file *file) strncmp(sec->name, ".discard.", 9)) sec->text = true; - if (!strcmp(sec->name, ".noinstr.text")) + if (!strcmp(sec->name, ".noinstr.text") || + !strcmp(sec->name, ".entry.text")) sec->noinstr = true; for (offset = 0; offset < sec->len; offset += insn->len) { @@ -2071,7 +2072,7 @@ static inline const char *call_dest_name(struct instruction *insn) static int validate_call(struct instruction *insn, struct insn_state *state) { if (state->noinstr && state->instr <= 0 && - (!insn->call_dest || insn->call_dest->sec != insn->sec)) { + (!insn->call_dest || !insn->call_dest->sec->noinstr)) { WARN_FUNC("call to %s() leaves .noinstr.text section", insn->sec, insn->offset, call_dest_name(insn)); return 1; @@ -2558,11 +2559,16 @@ static int validate_vmlinux_functions(struct objtool_file *file) int warnings = 0; sec = find_section_by_name(file->elf, ".noinstr.text"); - if (!sec) - return 0; + if (sec) { + warnings += validate_section(file, sec); + warnings += validate_unwind_hints(file, sec); + } - warnings += validate_section(file, sec); - warnings += validate_unwind_hints(file, sec); + sec = find_section_by_name(file->elf, ".entry.text"); + if (sec) { + warnings += validate_section(file, sec); + warnings += validate_unwind_hints(file, sec); + } return warnings; } From 894e48cada64ec384873fad4fe1b0d0c7de31a29 Mon Sep 17 00:00:00 2001 From: Ingo Molnar Date: Wed, 22 Apr 2020 12:32:03 +0200 Subject: [PATCH 29/54] objtool: Constify 'struct elf *' parameters In preparation to parallelize certain parts of objtool, map out which uses of various data structures are read-only vs. read-write. As a first step constify 'struct elf' pointer passing, most of the secondary uses of it in find_symbol_*() methods are read-only. Also, while at it, better group the 'struct elf' handling methods in elf.h. Acked-by: Josh Poimboeuf Signed-off-by: Ingo Molnar Cc: Borislav Petkov Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Sami Tolvanen Cc: Thomas Gleixner Link: https://lore.kernel.org/r/20200422103205.61900-2-mingo@kernel.org --- tools/objtool/elf.c | 10 +++++----- tools/objtool/elf.h | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index f26bb3e8db7b..fab5534c3365 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -127,7 +127,7 @@ static int symbol_by_offset(const void *key, const struct rb_node *node) return 0; } -struct section *find_section_by_name(struct elf *elf, const char *name) +struct section *find_section_by_name(const struct elf *elf, const char *name) { struct section *sec; @@ -217,7 +217,7 @@ struct symbol *find_func_containing(struct section *sec, unsigned long offset) return NULL; } -struct symbol *find_symbol_by_name(struct elf *elf, const char *name) +struct symbol *find_symbol_by_name(const struct elf *elf, const char *name) { struct symbol *sym; @@ -228,7 +228,7 @@ struct symbol *find_symbol_by_name(struct elf *elf, const char *name) return NULL; } -struct rela *find_rela_by_dest_range(struct elf *elf, struct section *sec, +struct rela *find_rela_by_dest_range(const struct elf *elf, struct section *sec, unsigned long offset, unsigned int len) { struct rela *rela, *r = NULL; @@ -257,7 +257,7 @@ struct rela *find_rela_by_dest_range(struct elf *elf, struct section *sec, return NULL; } -struct rela *find_rela_by_dest(struct elf *elf, struct section *sec, unsigned long offset) +struct rela *find_rela_by_dest(const struct elf *elf, struct section *sec, unsigned long offset) { return find_rela_by_dest_range(elf, sec, offset, 1); } @@ -769,7 +769,7 @@ int elf_rebuild_rela_section(struct section *sec) return 0; } -int elf_write(struct elf *elf) +int elf_write(const struct elf *elf) { struct section *sec; Elf_Scn *s; diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h index 2811d04346c9..a55bcde9f1b1 100644 --- a/tools/objtool/elf.h +++ b/tools/objtool/elf.h @@ -114,22 +114,22 @@ static inline u32 rela_hash(struct rela *rela) } struct elf *elf_read(const char *name, int flags); -struct section *find_section_by_name(struct elf *elf, const char *name); +struct section *elf_create_section(struct elf *elf, const char *name, size_t entsize, int nr); +struct section *elf_create_rela_section(struct elf *elf, struct section *base); +void elf_add_rela(struct elf *elf, struct rela *rela); +int elf_write(const struct elf *elf); +void elf_close(struct elf *elf); + +struct section *find_section_by_name(const struct elf *elf, const char *name); struct symbol *find_func_by_offset(struct section *sec, unsigned long offset); struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset); -struct symbol *find_symbol_by_name(struct elf *elf, const char *name); +struct symbol *find_symbol_by_name(const struct elf *elf, const char *name); struct symbol *find_symbol_containing(struct section *sec, unsigned long offset); -struct rela *find_rela_by_dest(struct elf *elf, struct section *sec, unsigned long offset); -struct rela *find_rela_by_dest_range(struct elf *elf, struct section *sec, +struct rela *find_rela_by_dest(const struct elf *elf, struct section *sec, unsigned long offset); +struct rela *find_rela_by_dest_range(const struct elf *elf, struct section *sec, unsigned long offset, unsigned int len); struct symbol *find_func_containing(struct section *sec, unsigned long offset); -struct section *elf_create_section(struct elf *elf, const char *name, size_t - entsize, int nr); -struct section *elf_create_rela_section(struct elf *elf, struct section *base); int elf_rebuild_rela_section(struct section *sec); -int elf_write(struct elf *elf); -void elf_close(struct elf *elf); -void elf_add_rela(struct elf *elf, struct rela *rela); #define for_each_sec(file, sec) \ list_for_each_entry(sec, &file->elf->sections, list) From bc359ff2f6f3e8a9df38c39017e269bc442357c7 Mon Sep 17 00:00:00 2001 From: Ingo Molnar Date: Wed, 22 Apr 2020 12:32:04 +0200 Subject: [PATCH 30/54] objtool: Rename elf_read() to elf_open_read() 'struct elf *' handling is an open/close paradigm, make sure the naming matches that: elf_open_read() elf_write() elf_close() Acked-by: Josh Poimboeuf Signed-off-by: Ingo Molnar Cc: Borislav Petkov Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Sami Tolvanen Cc: Thomas Gleixner Link: https://lore.kernel.org/r/20200422103205.61900-3-mingo@kernel.org --- tools/objtool/check.c | 2 +- tools/objtool/elf.c | 2 +- tools/objtool/elf.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index f2a84271e807..12e2aea42bb2 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2614,7 +2614,7 @@ int check(const char *_objname, bool orc) objname = _objname; - file.elf = elf_read(objname, orc ? O_RDWR : O_RDONLY); + file.elf = elf_open_read(objname, orc ? O_RDWR : O_RDONLY); if (!file.elf) return 1; diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index fab5534c3365..453b723c89d5 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -542,7 +542,7 @@ static int read_relas(struct elf *elf) return 0; } -struct elf *elf_read(const char *name, int flags) +struct elf *elf_open_read(const char *name, int flags) { struct elf *elf; Elf_Cmd cmd; diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h index a55bcde9f1b1..5e76ac38cf99 100644 --- a/tools/objtool/elf.h +++ b/tools/objtool/elf.h @@ -113,7 +113,7 @@ static inline u32 rela_hash(struct rela *rela) return sec_offset_hash(rela->sec, rela->offset); } -struct elf *elf_read(const char *name, int flags); +struct elf *elf_open_read(const char *name, int flags); struct section *elf_create_section(struct elf *elf, const char *name, size_t entsize, int nr); struct section *elf_create_rela_section(struct elf *elf, struct section *base); void elf_add_rela(struct elf *elf, struct rela *rela); From 0c98be8118221a8d3de572740f29dd02ed9686a5 Mon Sep 17 00:00:00 2001 From: Ingo Molnar Date: Wed, 22 Apr 2020 12:32:05 +0200 Subject: [PATCH 31/54] objtool: Constify arch_decode_instruction() Mostly straightforward constification, except that WARN_FUNC() needs a writable pointer while we have read-only pointers, so deflect this to WARN(). Acked-by: Josh Poimboeuf Signed-off-by: Ingo Molnar Cc: Borislav Petkov Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Sami Tolvanen Cc: Thomas Gleixner Link: https://lore.kernel.org/r/20200422103205.61900-4-mingo@kernel.org --- tools/objtool/arch.h | 2 +- tools/objtool/arch/x86/decode.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h index 561c3162d177..445b8fa73ccb 100644 --- a/tools/objtool/arch.h +++ b/tools/objtool/arch.h @@ -72,7 +72,7 @@ struct instruction; void arch_initial_func_cfi_state(struct cfi_init_state *state); -int arch_decode_instruction(struct elf *elf, struct section *sec, +int arch_decode_instruction(const struct elf *elf, const struct section *sec, unsigned long offset, unsigned int maxlen, unsigned int *len, enum insn_type *type, unsigned long *immediate, diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index f0d42ad7d5ff..c45a0b4e0760 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -27,7 +27,7 @@ static unsigned char op_to_cfi_reg[][2] = { {CFI_DI, CFI_R15}, }; -static int is_x86_64(struct elf *elf) +static int is_x86_64(const struct elf *elf) { switch (elf->ehdr.e_machine) { case EM_X86_64: @@ -77,7 +77,7 @@ unsigned long arch_jump_destination(struct instruction *insn) return insn->offset + insn->len + insn->immediate; } -int arch_decode_instruction(struct elf *elf, struct section *sec, +int arch_decode_instruction(const struct elf *elf, const struct section *sec, unsigned long offset, unsigned int maxlen, unsigned int *len, enum insn_type *type, unsigned long *immediate, @@ -98,7 +98,7 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, insn_get_length(&insn); if (!insn_complete(&insn)) { - WARN_FUNC("can't decode instruction", sec, offset); + WARN("can't decode instruction at %s:0x%lx", sec->name, offset); return -1; } From 9e98d62aa7ea1375052895650f3e6d362336c5c9 Mon Sep 17 00:00:00 2001 From: Julien Thierry Date: Fri, 27 Mar 2020 15:28:42 +0000 Subject: [PATCH 32/54] objtool: Remove check preventing branches within alternative While jumping from outside an alternative region to the middle of an alternative region is very likely wrong, jumping from an alternative region into the same region is valid. It is a common pattern on arm64. The first pattern is unlikely to happen in practice and checking only for this adds a lot of complexity. Just remove the current check. Suggested-by: Josh Poimboeuf Signed-off-by: Julien Thierry Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Link: https://lkml.kernel.org/r/20200327152847.15294-6-jthierry@redhat.com --- tools/objtool/check.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 12e2aea42bb2..cc52da61eda3 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2162,12 +2162,6 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, sec = insn->sec; - if (insn->alt_group && list_empty(&insn->alts)) { - WARN_FUNC("don't know how to handle branch to middle of alternative instruction group", - sec, insn->offset); - return 1; - } - while (1) { next_insn = next_insn_same_sec(file, insn); From 13fab06d9a3ad3afdfd51c7f8f87f2ae28444648 Mon Sep 17 00:00:00 2001 From: Alexandre Chartre Date: Tue, 14 Apr 2020 12:36:11 +0200 Subject: [PATCH 33/54] objtool: Uniquely identify alternative instruction groups Assign a unique identifier to every alternative instruction group in order to be able to tell which instructions belong to what alternative. [peterz: extracted from a larger patch] Signed-off-by: Alexandre Chartre Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes --- tools/objtool/check.c | 6 +++++- tools/objtool/check.h | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index cc52da61eda3..4da6bfb8a98d 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -752,7 +752,9 @@ static int handle_group_alt(struct objtool_file *file, struct instruction *orig_insn, struct instruction **new_insn) { + static unsigned int alt_group_next_index = 1; struct instruction *last_orig_insn, *last_new_insn, *insn, *fake_jump = NULL; + unsigned int alt_group = alt_group_next_index++; unsigned long dest_off; last_orig_insn = NULL; @@ -761,7 +763,7 @@ static int handle_group_alt(struct objtool_file *file, if (insn->offset >= special_alt->orig_off + special_alt->orig_len) break; - insn->alt_group = true; + insn->alt_group = alt_group; last_orig_insn = insn; } @@ -795,6 +797,7 @@ static int handle_group_alt(struct objtool_file *file, } last_new_insn = NULL; + alt_group = alt_group_next_index++; insn = *new_insn; sec_for_each_insn_from(file, insn) { if (insn->offset >= special_alt->new_off + special_alt->new_len) @@ -804,6 +807,7 @@ static int handle_group_alt(struct objtool_file *file, insn->ignore = orig_insn->ignore_alts; insn->func = orig_insn->func; + insn->alt_group = alt_group; /* * Since alternative replacement code is copy/pasted by the diff --git a/tools/objtool/check.h b/tools/objtool/check.h index 12a9660c960b..24280227ef21 100644 --- a/tools/objtool/check.h +++ b/tools/objtool/check.h @@ -30,12 +30,13 @@ struct instruction { unsigned int len; enum insn_type type; unsigned long immediate; - bool alt_group, dead_end, ignore, ignore_alts; + bool dead_end, ignore, ignore_alts; bool hint; bool retpoline_safe; s8 instr; u8 visited; u8 ret_offset; + int alt_group; struct symbol *call_dest; struct instruction *jump_dest; struct instruction *first_jump_src; From 7117f16bf460ef8cd132e6e80c989677397b4868 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Tue, 28 Apr 2020 19:37:01 +0200 Subject: [PATCH 34/54] objtool: Fix ORC vs alternatives Jann reported that (for instance) entry_64.o:general_protection has very odd ORC data: 0000000000000f40 : #######sp:sp+8 bp:(und) type:iret end:0 f40: 90 nop #######sp:(und) bp:(und) type:call end:0 f41: 90 nop f42: 90 nop #######sp:sp+8 bp:(und) type:iret end:0 f43: e8 a8 01 00 00 callq 10f0 #######sp:sp+0 bp:(und) type:regs end:0 f48: f6 84 24 88 00 00 00 testb $0x3,0x88(%rsp) f4f: 03 f50: 74 00 je f52 f52: 48 89 e7 mov %rsp,%rdi f55: 48 8b 74 24 78 mov 0x78(%rsp),%rsi f5a: 48 c7 44 24 78 ff ff movq $0xffffffffffffffff,0x78(%rsp) f61: ff ff f63: e8 00 00 00 00 callq f68 f68: e9 73 02 00 00 jmpq 11e0 #######sp:(und) bp:(und) type:call end:0 f6d: 0f 1f 00 nopl (%rax) Note the entry at 0xf41. Josh found this was the result of commit: 764eef4b109a ("objtool: Rewrite alt->skip_orig") Due to the early return in validate_branch() we no longer set insn->cfi of the original instruction stream (the NOPs at 0xf41 and 0xf42) and we'll end up with the above weirdness. In other discussions we realized alternatives should be ORC invariant; that is, due to there being only a single ORC table, it must be valid for all alternatives. The easiest way to ensure this is to not allow any stack modifications in alternatives. When we enforce this latter observation, we get the property that the whole alternative must have the same CFI, which we can employ to fix the former report. Fixes: 764eef4b109a ("objtool: Rewrite alt->skip_orig") Reported-by: Jann Horn Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200428191659.499074346@infradead.org --- .../Documentation/stack-validation.txt | 7 ++++ tools/objtool/check.c | 34 ++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/tools/objtool/Documentation/stack-validation.txt b/tools/objtool/Documentation/stack-validation.txt index faa47c3aafae..0189039489e9 100644 --- a/tools/objtool/Documentation/stack-validation.txt +++ b/tools/objtool/Documentation/stack-validation.txt @@ -315,6 +315,13 @@ they mean, and suggestions for how to fix them. function tracing inserts additional calls, which is not obvious from the sources). +10. file.o: warning: func()+0x5c: alternative modifies stack + + This means that an alternative includes instructions that modify the + stack. The problem is that there is only one ORC unwind table, this means + that the ORC unwind entries must be valid for each of the alternatives. + The easiest way to enforce this is to ensure alternatives do not contain + any ORC entries, which in turn implies the above constraint. If the error doesn't seem to make sense, it could be a bug in objtool. Feel free to ask the objtool maintainer for help. diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 4da6bfb8a98d..fa9bf364545c 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1983,6 +1983,11 @@ static int handle_insn_ops(struct instruction *insn, struct insn_state *state) list_for_each_entry(op, &insn->stack_ops, list) { int res; + if (insn->alt_group) { + WARN_FUNC("alternative modifies stack", insn->sec, insn->offset); + return -1; + } + res = update_cfi_state(insn, &state->cfi, op); if (res) return res; @@ -2149,6 +2154,30 @@ static int validate_return(struct symbol *func, struct instruction *insn, struct return 0; } +/* + * Alternatives should not contain any ORC entries, this in turn means they + * should not contain any CFI ops, which implies all instructions should have + * the same same CFI state. + * + * It is possible to constuct alternatives that have unreachable holes that go + * unreported (because they're NOPs), such holes would result in CFI_UNDEFINED + * states which then results in ORC entries, which we just said we didn't want. + * + * Avoid them by copying the CFI entry of the first instruction into the whole + * alternative. + */ +static void fill_alternative_cfi(struct objtool_file *file, struct instruction *insn) +{ + struct instruction *first_insn = insn; + int alt_group = insn->alt_group; + + sec_for_each_insn_continue(file, insn) { + if (insn->alt_group != alt_group) + break; + insn->cfi = first_insn->cfi; + } +} + /* * Follow the branch starting at the given instruction, and recursively follow * any other branches (jumps). Meanwhile, track the frame pointer state at @@ -2200,7 +2229,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, insn->visited |= visited; - if (!insn->ignore_alts) { + if (!insn->ignore_alts && !list_empty(&insn->alts)) { bool skip_orig = false; list_for_each_entry(alt, &insn->alts, list) { @@ -2215,6 +2244,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, } } + if (insn->alt_group) + fill_alternative_cfi(file, insn); + if (skip_orig) return 0; } From 1ff865e343c2b59469d7e41d370a980a3f972c71 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Tue, 28 Apr 2020 19:57:59 +0200 Subject: [PATCH 35/54] x86,smap: Fix smap_{save,restore}() alternatives As reported by objtool: lib/ubsan.o: warning: objtool: .altinstr_replacement+0x0: alternative modifies stack lib/ubsan.o: warning: objtool: .altinstr_replacement+0x7: alternative modifies stack the smap_{save,restore}() alternatives violate (the newly enforced) rule on stack invariance. That is, due to there only being a single ORC table it must be valid to any alternative. These alternatives violate this with the direct result that unwinds will not be correct when it hits between the PUSH and POP instructions. Rewrite the functions to only have a conditional jump. Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200429101802.GI13592@hirez.programming.kicks-ass.net --- arch/x86/include/asm/smap.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/arch/x86/include/asm/smap.h b/arch/x86/include/asm/smap.h index 27c47d183f4b..8b58d6975d5d 100644 --- a/arch/x86/include/asm/smap.h +++ b/arch/x86/include/asm/smap.h @@ -57,8 +57,10 @@ static __always_inline unsigned long smap_save(void) { unsigned long flags; - asm volatile (ALTERNATIVE("", "pushf; pop %0; " __ASM_CLAC, - X86_FEATURE_SMAP) + asm volatile ("# smap_save\n\t" + ALTERNATIVE("jmp 1f", "", X86_FEATURE_SMAP) + "pushf; pop %0; " __ASM_CLAC "\n\t" + "1:" : "=rm" (flags) : : "memory", "cc"); return flags; @@ -66,7 +68,10 @@ static __always_inline unsigned long smap_save(void) static __always_inline void smap_restore(unsigned long flags) { - asm volatile (ALTERNATIVE("", "push %0; popf", X86_FEATURE_SMAP) + asm volatile ("# smap_restore\n\t" + ALTERNATIVE("jmp 1f", "", X86_FEATURE_SMAP) + "push %0; popf\n\t" + "1:" : : "g" (flags) : "memory", "cc"); } From 87cf61fe848ca8ddf091548671e168f52e8a718e Mon Sep 17 00:00:00 2001 From: Alexandre Chartre Date: Tue, 14 Apr 2020 12:36:10 +0200 Subject: [PATCH 36/54] objtool: is_fentry_call() crashes if call has no destination Fix is_fentry_call() so that it works if a call has no destination set (call_dest). This needs to be done in order to support intra- function calls. Signed-off-by: Alexandre Chartre Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200414103618.12657-2-alexandre.chartre@oracle.com --- 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 fa9bf364545c..8af8de2299e8 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1484,7 +1484,7 @@ static int decode_sections(struct objtool_file *file) static bool is_fentry_call(struct instruction *insn) { - if (insn->type == INSN_CALL && + if (insn->type == INSN_CALL && insn->call_dest && insn->call_dest->type == STT_NOTYPE && !strcmp(insn->call_dest->name, "__fentry__")) return true; From c721b3f80faebc7891211fa82de303eebadfed15 Mon Sep 17 00:00:00 2001 From: Alexandre Chartre Date: Tue, 7 Apr 2020 09:31:35 +0200 Subject: [PATCH 37/54] objtool: UNWIND_HINT_RET_OFFSET should not check registers UNWIND_HINT_RET_OFFSET will adjust a modified stack. However if a callee-saved register was pushed on the stack then the stack frame will still appear modified. So stop checking registers when UNWIND_HINT_RET_OFFSET is used. Signed-off-by: Alexandre Chartre Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200407073142.20659-3-alexandre.chartre@oracle.com --- tools/objtool/check.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 8af8de2299e8..068897d5d956 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1507,6 +1507,14 @@ static bool has_modified_stack_frame(struct instruction *insn, struct insn_state if (cfi->stack_size != initial_func_cfi.cfa.offset + ret_offset) return true; + /* + * If there is a ret offset hint then don't check registers + * because a callee-saved register might have been pushed on + * the stack. + */ + if (ret_offset) + return false; + for (i = 0; i < CFI_NUM_REGS; i++) { if (cfi->regs[i].base != initial_func_cfi.regs[i].base || cfi->regs[i].offset != initial_func_cfi.regs[i].offset) From 7d989fcadd6e225a61d6490dd15bdbdfc8a53d5c Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Thu, 23 Apr 2020 13:22:10 +0200 Subject: [PATCH 38/54] objtool: Rework allocating stack_ops on decode Wrap each stack_op in a macro that allocates and adds it to the list. This simplifies trying to figure out what to do with the pre-allocated stack_op at the end. Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Alexandre Chartre Reviewed-by: Miroslav Benes Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200428191659.736151601@infradead.org --- tools/objtool/arch/x86/decode.c | 251 +++++++++++++++++++------------- 1 file changed, 147 insertions(+), 104 deletions(-) diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index c45a0b4e0760..97e66c779520 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -77,6 +77,11 @@ unsigned long arch_jump_destination(struct instruction *insn) return insn->offset + insn->len + insn->immediate; } +#define ADD_OP(op) \ + if (!(op = calloc(1, sizeof(*op)))) \ + return -1; \ + else for (list_add_tail(&op->list, ops_list); op; op = NULL) + int arch_decode_instruction(const struct elf *elf, const struct section *sec, unsigned long offset, unsigned int maxlen, unsigned int *len, enum insn_type *type, @@ -88,7 +93,7 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, unsigned char op1, op2, rex = 0, rex_b = 0, rex_r = 0, rex_w = 0, rex_x = 0, modrm = 0, modrm_mod = 0, modrm_rm = 0, modrm_reg = 0, sib = 0; - struct stack_op *op; + struct stack_op *op = NULL; x86_64 = is_x86_64(elf); if (x86_64 == -1) @@ -129,10 +134,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, if (insn.sib.nbytes) sib = insn.sib.bytes[0]; - op = calloc(1, sizeof(*op)); - if (!op) - return -1; - switch (op1) { case 0x1: @@ -141,10 +142,12 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, /* add/sub reg, %rsp */ *type = INSN_STACK; - op->src.type = OP_SRC_ADD; - op->src.reg = op_to_cfi_reg[modrm_reg][rex_r]; - op->dest.type = OP_DEST_REG; - op->dest.reg = CFI_SP; + ADD_OP(op) { + op->src.type = OP_SRC_ADD; + op->src.reg = op_to_cfi_reg[modrm_reg][rex_r]; + op->dest.type = OP_DEST_REG; + op->dest.reg = CFI_SP; + } } break; @@ -152,9 +155,11 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, /* push reg */ *type = INSN_STACK; - op->src.type = OP_SRC_REG; - op->src.reg = op_to_cfi_reg[op1 & 0x7][rex_b]; - op->dest.type = OP_DEST_PUSH; + ADD_OP(op) { + op->src.type = OP_SRC_REG; + op->src.reg = op_to_cfi_reg[op1 & 0x7][rex_b]; + op->dest.type = OP_DEST_PUSH; + } break; @@ -162,9 +167,11 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, /* pop reg */ *type = INSN_STACK; - op->src.type = OP_SRC_POP; - op->dest.type = OP_DEST_REG; - op->dest.reg = op_to_cfi_reg[op1 & 0x7][rex_b]; + ADD_OP(op) { + op->src.type = OP_SRC_POP; + op->dest.type = OP_DEST_REG; + op->dest.reg = op_to_cfi_reg[op1 & 0x7][rex_b]; + } break; @@ -172,8 +179,10 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, case 0x6a: /* push immediate */ *type = INSN_STACK; - op->src.type = OP_SRC_CONST; - op->dest.type = OP_DEST_PUSH; + ADD_OP(op) { + op->src.type = OP_SRC_CONST; + op->dest.type = OP_DEST_PUSH; + } break; case 0x70 ... 0x7f: @@ -188,11 +197,13 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, if (modrm == 0xe4) { /* and imm, %rsp */ *type = INSN_STACK; - op->src.type = OP_SRC_AND; - op->src.reg = CFI_SP; - op->src.offset = insn.immediate.value; - op->dest.type = OP_DEST_REG; - op->dest.reg = CFI_SP; + ADD_OP(op) { + op->src.type = OP_SRC_AND; + op->src.reg = CFI_SP; + op->src.offset = insn.immediate.value; + op->dest.type = OP_DEST_REG; + op->dest.reg = CFI_SP; + } break; } @@ -205,11 +216,13 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, /* add/sub imm, %rsp */ *type = INSN_STACK; - op->src.type = OP_SRC_ADD; - op->src.reg = CFI_SP; - op->src.offset = insn.immediate.value * sign; - op->dest.type = OP_DEST_REG; - op->dest.reg = CFI_SP; + ADD_OP(op) { + op->src.type = OP_SRC_ADD; + op->src.reg = CFI_SP; + op->src.offset = insn.immediate.value * sign; + op->dest.type = OP_DEST_REG; + op->dest.reg = CFI_SP; + } break; case 0x89: @@ -217,10 +230,12 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, /* mov %rsp, reg */ *type = INSN_STACK; - op->src.type = OP_SRC_REG; - op->src.reg = CFI_SP; - op->dest.type = OP_DEST_REG; - op->dest.reg = op_to_cfi_reg[modrm_rm][rex_b]; + ADD_OP(op) { + op->src.type = OP_SRC_REG; + op->src.reg = CFI_SP; + op->dest.type = OP_DEST_REG; + op->dest.reg = op_to_cfi_reg[modrm_rm][rex_b]; + } break; } @@ -228,10 +243,12 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, /* mov reg, %rsp */ *type = INSN_STACK; - op->src.type = OP_SRC_REG; - op->src.reg = op_to_cfi_reg[modrm_reg][rex_r]; - op->dest.type = OP_DEST_REG; - op->dest.reg = CFI_SP; + ADD_OP(op) { + op->src.type = OP_SRC_REG; + op->src.reg = op_to_cfi_reg[modrm_reg][rex_r]; + op->dest.type = OP_DEST_REG; + op->dest.reg = CFI_SP; + } break; } @@ -242,21 +259,25 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, /* mov reg, disp(%rbp) */ *type = INSN_STACK; - op->src.type = OP_SRC_REG; - op->src.reg = op_to_cfi_reg[modrm_reg][rex_r]; - op->dest.type = OP_DEST_REG_INDIRECT; - op->dest.reg = CFI_BP; - op->dest.offset = insn.displacement.value; + ADD_OP(op) { + op->src.type = OP_SRC_REG; + op->src.reg = op_to_cfi_reg[modrm_reg][rex_r]; + op->dest.type = OP_DEST_REG_INDIRECT; + op->dest.reg = CFI_BP; + op->dest.offset = insn.displacement.value; + } } else if (rex_w && !rex_b && modrm_rm == 4 && sib == 0x24) { /* mov reg, disp(%rsp) */ *type = INSN_STACK; - op->src.type = OP_SRC_REG; - op->src.reg = op_to_cfi_reg[modrm_reg][rex_r]; - op->dest.type = OP_DEST_REG_INDIRECT; - op->dest.reg = CFI_SP; - op->dest.offset = insn.displacement.value; + ADD_OP(op) { + op->src.type = OP_SRC_REG; + op->src.reg = op_to_cfi_reg[modrm_reg][rex_r]; + op->dest.type = OP_DEST_REG_INDIRECT; + op->dest.reg = CFI_SP; + op->dest.offset = insn.displacement.value; + } } break; @@ -266,22 +287,26 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, /* mov disp(%rbp), reg */ *type = INSN_STACK; - op->src.type = OP_SRC_REG_INDIRECT; - op->src.reg = CFI_BP; - op->src.offset = insn.displacement.value; - op->dest.type = OP_DEST_REG; - op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r]; + ADD_OP(op) { + op->src.type = OP_SRC_REG_INDIRECT; + op->src.reg = CFI_BP; + op->src.offset = insn.displacement.value; + op->dest.type = OP_DEST_REG; + op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r]; + } } else if (rex_w && !rex_b && sib == 0x24 && modrm_mod != 3 && modrm_rm == 4) { /* mov disp(%rsp), reg */ *type = INSN_STACK; - op->src.type = OP_SRC_REG_INDIRECT; - op->src.reg = CFI_SP; - op->src.offset = insn.displacement.value; - op->dest.type = OP_DEST_REG; - op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r]; + ADD_OP(op) { + op->src.type = OP_SRC_REG_INDIRECT; + op->src.reg = CFI_SP; + op->src.offset = insn.displacement.value; + op->dest.type = OP_DEST_REG; + op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r]; + } } break; @@ -290,27 +315,31 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, if (sib == 0x24 && rex_w && !rex_b && !rex_x) { *type = INSN_STACK; - if (!insn.displacement.value) { - /* lea (%rsp), reg */ - op->src.type = OP_SRC_REG; - } else { - /* lea disp(%rsp), reg */ - op->src.type = OP_SRC_ADD; - op->src.offset = insn.displacement.value; + ADD_OP(op) { + if (!insn.displacement.value) { + /* lea (%rsp), reg */ + op->src.type = OP_SRC_REG; + } else { + /* lea disp(%rsp), reg */ + op->src.type = OP_SRC_ADD; + op->src.offset = insn.displacement.value; + } + op->src.reg = CFI_SP; + op->dest.type = OP_DEST_REG; + op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r]; } - op->src.reg = CFI_SP; - op->dest.type = OP_DEST_REG; - op->dest.reg = op_to_cfi_reg[modrm_reg][rex_r]; } else if (rex == 0x48 && modrm == 0x65) { /* lea disp(%rbp), %rsp */ *type = INSN_STACK; - op->src.type = OP_SRC_ADD; - op->src.reg = CFI_BP; - op->src.offset = insn.displacement.value; - op->dest.type = OP_DEST_REG; - op->dest.reg = CFI_SP; + ADD_OP(op) { + op->src.type = OP_SRC_ADD; + op->src.reg = CFI_BP; + op->src.offset = insn.displacement.value; + op->dest.type = OP_DEST_REG; + op->dest.reg = CFI_SP; + } } else if (rex == 0x49 && modrm == 0x62 && insn.displacement.value == -8) { @@ -322,11 +351,13 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, * stack realignment. */ *type = INSN_STACK; - op->src.type = OP_SRC_ADD; - op->src.reg = CFI_R10; - op->src.offset = -8; - op->dest.type = OP_DEST_REG; - op->dest.reg = CFI_SP; + ADD_OP(op) { + op->src.type = OP_SRC_ADD; + op->src.reg = CFI_R10; + op->src.offset = -8; + op->dest.type = OP_DEST_REG; + op->dest.reg = CFI_SP; + } } else if (rex == 0x49 && modrm == 0x65 && insn.displacement.value == -16) { @@ -338,11 +369,13 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, * stack realignment. */ *type = INSN_STACK; - op->src.type = OP_SRC_ADD; - op->src.reg = CFI_R13; - op->src.offset = -16; - op->dest.type = OP_DEST_REG; - op->dest.reg = CFI_SP; + ADD_OP(op) { + op->src.type = OP_SRC_ADD; + op->src.reg = CFI_R13; + op->src.offset = -16; + op->dest.type = OP_DEST_REG; + op->dest.reg = CFI_SP; + } } break; @@ -350,8 +383,10 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, case 0x8f: /* pop to mem */ *type = INSN_STACK; - op->src.type = OP_SRC_POP; - op->dest.type = OP_DEST_MEM; + ADD_OP(op) { + op->src.type = OP_SRC_POP; + op->dest.type = OP_DEST_MEM; + } break; case 0x90: @@ -361,15 +396,19 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, case 0x9c: /* pushf */ *type = INSN_STACK; - op->src.type = OP_SRC_CONST; - op->dest.type = OP_DEST_PUSHF; + ADD_OP(op) { + op->src.type = OP_SRC_CONST; + op->dest.type = OP_DEST_PUSHF; + } break; case 0x9d: /* popf */ *type = INSN_STACK; - op->src.type = OP_SRC_POPF; - op->dest.type = OP_DEST_MEM; + ADD_OP(op) { + op->src.type = OP_SRC_POPF; + op->dest.type = OP_DEST_MEM; + } break; case 0x0f: @@ -405,15 +444,19 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, /* push fs/gs */ *type = INSN_STACK; - op->src.type = OP_SRC_CONST; - op->dest.type = OP_DEST_PUSH; + ADD_OP(op) { + op->src.type = OP_SRC_CONST; + op->dest.type = OP_DEST_PUSH; + } } else if (op2 == 0xa1 || op2 == 0xa9) { /* pop fs/gs */ *type = INSN_STACK; - op->src.type = OP_SRC_POP; - op->dest.type = OP_DEST_MEM; + ADD_OP(op) { + op->src.type = OP_SRC_POP; + op->dest.type = OP_DEST_MEM; + } } break; @@ -427,7 +470,8 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, * pop bp */ *type = INSN_STACK; - op->dest.type = OP_DEST_LEAVE; + ADD_OP(op) + op->dest.type = OP_DEST_LEAVE; break; @@ -449,12 +493,14 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, case 0xcf: /* iret */ *type = INSN_EXCEPTION_RETURN; - /* add $40, %rsp */ - op->src.type = OP_SRC_ADD; - op->src.reg = CFI_SP; - op->src.offset = 5*8; - op->dest.type = OP_DEST_REG; - op->dest.reg = CFI_SP; + ADD_OP(op) { + /* add $40, %rsp */ + op->src.type = OP_SRC_ADD; + op->src.reg = CFI_SP; + op->src.offset = 5*8; + op->dest.type = OP_DEST_REG; + op->dest.reg = CFI_SP; + } break; case 0xca: /* retf */ @@ -492,8 +538,10 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, /* push from mem */ *type = INSN_STACK; - op->src.type = OP_SRC_CONST; - op->dest.type = OP_DEST_PUSH; + ADD_OP(op) { + op->src.type = OP_SRC_CONST; + op->dest.type = OP_DEST_PUSH; + } } break; @@ -504,11 +552,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, *immediate = insn.immediate.nbytes ? insn.immediate.value : 0; - if (*type == INSN_STACK || *type == INSN_EXCEPTION_RETURN) - list_add_tail(&op->list, ops_list); - else - free(op); - return 0; } From 60041bcd8f5ab560dabf44dc384f58bbeb5a6a30 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Fri, 24 Apr 2020 16:16:41 +0200 Subject: [PATCH 39/54] objtool: Make handle_insn_ops() unconditional Now that every instruction has a list of stack_ops; we can trivially distinquish those instructions that do not have stack_ops, their list is empty. This means we can now call handle_insn_ops() unconditionally. Suggested-by: Julien Thierry Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200428191659.795115188@infradead.org --- tools/objtool/check.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 068897d5d956..6591c2dd94f1 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2259,6 +2259,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, return 0; } + if (handle_insn_ops(insn, &state)) + return 1; + switch (insn->type) { case INSN_RETURN: @@ -2318,9 +2321,6 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, break; case INSN_EXCEPTION_RETURN: - if (handle_insn_ops(insn, &state)) - return 1; - /* * This handles x86's sync_core() case, where we use an * IRET to self. All 'normal' IRET instructions are in @@ -2340,8 +2340,6 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, return 0; case INSN_STACK: - if (handle_insn_ops(insn, &state)) - return 1; break; case INSN_STAC: From b09fb65e863733e192d4825a285b4b4998969ce0 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Fri, 24 Apr 2020 16:18:58 +0200 Subject: [PATCH 40/54] objtool: Remove INSN_STACK With the unconditional use of handle_insn_ops(), INSN_STACK has lost its purpose. Remove it. Suggested-by: Julien Thierry Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200428191659.854203028@infradead.org --- tools/objtool/arch.h | 1 - tools/objtool/arch/x86/decode.c | 23 ----------------------- tools/objtool/check.c | 3 --- 3 files changed, 27 deletions(-) diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h index 445b8fa73ccb..25dd4a9b38b2 100644 --- a/tools/objtool/arch.h +++ b/tools/objtool/arch.h @@ -21,7 +21,6 @@ enum insn_type { INSN_RETURN, INSN_EXCEPTION_RETURN, INSN_CONTEXT_SWITCH, - INSN_STACK, INSN_BUG, INSN_NOP, INSN_STAC, diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index 97e66c779520..e26bedbfff95 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -141,7 +141,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, if (rex_w && !rex_b && modrm_mod == 3 && modrm_rm == 4) { /* add/sub reg, %rsp */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_ADD; op->src.reg = op_to_cfi_reg[modrm_reg][rex_r]; @@ -154,7 +153,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, case 0x50 ... 0x57: /* push reg */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_REG; op->src.reg = op_to_cfi_reg[op1 & 0x7][rex_b]; @@ -166,7 +164,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, case 0x58 ... 0x5f: /* pop reg */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_POP; op->dest.type = OP_DEST_REG; @@ -178,7 +175,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, case 0x68: case 0x6a: /* push immediate */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_CONST; op->dest.type = OP_DEST_PUSH; @@ -196,7 +192,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, if (modrm == 0xe4) { /* and imm, %rsp */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_AND; op->src.reg = CFI_SP; @@ -215,7 +210,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, break; /* add/sub imm, %rsp */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_ADD; op->src.reg = CFI_SP; @@ -229,7 +223,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, if (rex_w && !rex_r && modrm_mod == 3 && modrm_reg == 4) { /* mov %rsp, reg */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_REG; op->src.reg = CFI_SP; @@ -242,7 +235,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, if (rex_w && !rex_b && modrm_mod == 3 && modrm_rm == 4) { /* mov reg, %rsp */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_REG; op->src.reg = op_to_cfi_reg[modrm_reg][rex_r]; @@ -258,7 +250,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, (modrm_mod == 1 || modrm_mod == 2) && modrm_rm == 5) { /* mov reg, disp(%rbp) */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_REG; op->src.reg = op_to_cfi_reg[modrm_reg][rex_r]; @@ -270,7 +261,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, } else if (rex_w && !rex_b && modrm_rm == 4 && sib == 0x24) { /* mov reg, disp(%rsp) */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_REG; op->src.reg = op_to_cfi_reg[modrm_reg][rex_r]; @@ -286,7 +276,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, if (rex_w && !rex_b && modrm_mod == 1 && modrm_rm == 5) { /* mov disp(%rbp), reg */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_REG_INDIRECT; op->src.reg = CFI_BP; @@ -299,7 +288,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, modrm_mod != 3 && modrm_rm == 4) { /* mov disp(%rsp), reg */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_REG_INDIRECT; op->src.reg = CFI_SP; @@ -314,7 +302,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, case 0x8d: if (sib == 0x24 && rex_w && !rex_b && !rex_x) { - *type = INSN_STACK; ADD_OP(op) { if (!insn.displacement.value) { /* lea (%rsp), reg */ @@ -332,7 +319,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, } else if (rex == 0x48 && modrm == 0x65) { /* lea disp(%rbp), %rsp */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_ADD; op->src.reg = CFI_BP; @@ -350,7 +336,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, * Restoring rsp back to its original value after a * stack realignment. */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_ADD; op->src.reg = CFI_R10; @@ -368,7 +353,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, * Restoring rsp back to its original value after a * stack realignment. */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_ADD; op->src.reg = CFI_R13; @@ -382,7 +366,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, case 0x8f: /* pop to mem */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_POP; op->dest.type = OP_DEST_MEM; @@ -395,7 +378,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, case 0x9c: /* pushf */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_CONST; op->dest.type = OP_DEST_PUSHF; @@ -404,7 +386,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, case 0x9d: /* popf */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_POPF; op->dest.type = OP_DEST_MEM; @@ -443,7 +424,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, } else if (op2 == 0xa0 || op2 == 0xa8) { /* push fs/gs */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_CONST; op->dest.type = OP_DEST_PUSH; @@ -452,7 +432,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, } else if (op2 == 0xa1 || op2 == 0xa9) { /* pop fs/gs */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_POP; op->dest.type = OP_DEST_MEM; @@ -469,7 +448,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, * mov bp, sp * pop bp */ - *type = INSN_STACK; ADD_OP(op) op->dest.type = OP_DEST_LEAVE; @@ -537,7 +515,6 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, else if (modrm_reg == 6) { /* push from mem */ - *type = INSN_STACK; ADD_OP(op) { op->src.type = OP_SRC_CONST; op->dest.type = OP_DEST_PUSH; diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 6591c2dd94f1..4f3db2f58c56 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2339,9 +2339,6 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, } return 0; - case INSN_STACK: - break; - case INSN_STAC: if (state.uaccess) { WARN_FUNC("recursive UACCESS enable", sec, insn->offset); From b490f45362002fef57996388e395efc974b013f4 Mon Sep 17 00:00:00 2001 From: Miroslav Benes Date: Fri, 24 Apr 2020 16:30:42 +0200 Subject: [PATCH 41/54] objtool: Move the IRET hack into the arch decoder Quoting Julien: "And the other suggestion is my other email was that you don't even need to add INSN_EXCEPTION_RETURN. You can keep IRET as INSN_CONTEXT_SWITCH by default and x86 decoder lookups the symbol conaining an iret. If it's a function symbol, it can just set the type to INSN_OTHER so that it caries on to the next instruction after having handled the stack_op." Suggested-by: Julien Thierry Signed-off-by: Miroslav Benes Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200428191659.913283807@infradead.org --- tools/objtool/arch.h | 1 - tools/objtool/arch/x86/decode.c | 28 ++++++++++++++++++---------- tools/objtool/check.c | 11 ----------- tools/objtool/elf.c | 4 ++-- tools/objtool/elf.h | 2 +- 5 files changed, 21 insertions(+), 25 deletions(-) diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h index 25dd4a9b38b2..cd118eb4248a 100644 --- a/tools/objtool/arch.h +++ b/tools/objtool/arch.h @@ -19,7 +19,6 @@ enum insn_type { INSN_CALL, INSN_CALL_DYNAMIC, INSN_RETURN, - INSN_EXCEPTION_RETURN, INSN_CONTEXT_SWITCH, INSN_BUG, INSN_NOP, diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index e26bedbfff95..d7b5d1096913 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -94,6 +94,7 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, rex_x = 0, modrm = 0, modrm_mod = 0, modrm_rm = 0, modrm_reg = 0, sib = 0; struct stack_op *op = NULL; + struct symbol *sym; x86_64 = is_x86_64(elf); if (x86_64 == -1) @@ -469,17 +470,24 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, break; case 0xcf: /* iret */ - *type = INSN_EXCEPTION_RETURN; - - ADD_OP(op) { - /* add $40, %rsp */ - op->src.type = OP_SRC_ADD; - op->src.reg = CFI_SP; - op->src.offset = 5*8; - op->dest.type = OP_DEST_REG; - op->dest.reg = CFI_SP; + /* + * Handle sync_core(), which has an IRET to self. + * All other IRET are in STT_NONE entry code. + */ + sym = find_symbol_containing(sec, offset); + if (sym && sym->type == STT_FUNC) { + ADD_OP(op) { + /* add $40, %rsp */ + op->src.type = OP_SRC_ADD; + op->src.reg = CFI_SP; + op->src.offset = 5*8; + op->dest.type = OP_DEST_REG; + op->dest.reg = CFI_SP; + } + break; } - break; + + /* fallthrough */ case 0xca: /* retf */ case 0xcb: /* retf */ diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 4f3db2f58c56..d822858764fe 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2320,17 +2320,6 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, break; - case INSN_EXCEPTION_RETURN: - /* - * This handles x86's sync_core() case, where we use an - * IRET to self. All 'normal' IRET instructions are in - * STT_NOTYPE entry symbols. - */ - if (func) - break; - - return 0; - case INSN_CONTEXT_SWITCH: if (func && (!next_insn || !next_insn->hint)) { WARN_FUNC("unsupported instruction in callable function", diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index 453b723c89d5..b6349caaeb5c 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -61,7 +61,7 @@ static void rb_add(struct rb_root *tree, struct rb_node *node, rb_insert_color(node, tree); } -static struct rb_node *rb_find_first(struct rb_root *tree, const void *key, +static struct rb_node *rb_find_first(const struct rb_root *tree, const void *key, int (*cmp)(const void *key, const struct rb_node *)) { struct rb_node *node = tree->rb_node; @@ -189,7 +189,7 @@ struct symbol *find_func_by_offset(struct section *sec, unsigned long offset) return NULL; } -struct symbol *find_symbol_containing(struct section *sec, unsigned long offset) +struct symbol *find_symbol_containing(const struct section *sec, unsigned long offset) { struct rb_node *node; diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h index 5e76ac38cf99..9d6bb072849c 100644 --- a/tools/objtool/elf.h +++ b/tools/objtool/elf.h @@ -124,7 +124,7 @@ struct section *find_section_by_name(const struct elf *elf, const char *name); struct symbol *find_func_by_offset(struct section *sec, unsigned long offset); struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset); struct symbol *find_symbol_by_name(const struct elf *elf, const char *name); -struct symbol *find_symbol_containing(struct section *sec, unsigned long offset); +struct symbol *find_symbol_containing(const struct section *sec, unsigned long offset); struct rela *find_rela_by_dest(const struct elf *elf, struct section *sec, unsigned long offset); struct rela *find_rela_by_dest_range(const struct elf *elf, struct section *sec, unsigned long offset, unsigned int len); From 8aa8eb2a8f5b3305a95f39957dd2b715fa668e21 Mon Sep 17 00:00:00 2001 From: Alexandre Chartre Date: Tue, 14 Apr 2020 12:36:12 +0200 Subject: [PATCH 42/54] objtool: Add support for intra-function calls Change objtool to support intra-function calls. On x86, an intra-function call is represented in objtool as a push onto the stack (of the return address), and a jump to the destination address. That way the stack information is correctly updated and the call flow is still accurate. Signed-off-by: Alexandre Chartre Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Miroslav Benes Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200414103618.12657-4-alexandre.chartre@oracle.com --- include/linux/frame.h | 11 +++ .../Documentation/stack-validation.txt | 8 ++ tools/objtool/arch/x86/decode.c | 8 ++ tools/objtool/check.c | 79 ++++++++++++++++++- 4 files changed, 102 insertions(+), 4 deletions(-) diff --git a/include/linux/frame.h b/include/linux/frame.h index 02d3ca2d9598..303cda600e56 100644 --- a/include/linux/frame.h +++ b/include/linux/frame.h @@ -15,9 +15,20 @@ static void __used __section(.discard.func_stack_frame_non_standard) \ *__func_stack_frame_non_standard_##func = func +/* + * This macro indicates that the following intra-function call is valid. + * Any non-annotated intra-function call will cause objtool to issue a warning. + */ +#define ANNOTATE_INTRA_FUNCTION_CALL \ + 999: \ + .pushsection .discard.intra_function_calls; \ + .long 999b; \ + .popsection; + #else /* !CONFIG_STACK_VALIDATION */ #define STACK_FRAME_NON_STANDARD(func) +#define ANNOTATE_INTRA_FUNCTION_CALL #endif /* CONFIG_STACK_VALIDATION */ diff --git a/tools/objtool/Documentation/stack-validation.txt b/tools/objtool/Documentation/stack-validation.txt index 0189039489e9..0542e46c7552 100644 --- a/tools/objtool/Documentation/stack-validation.txt +++ b/tools/objtool/Documentation/stack-validation.txt @@ -323,6 +323,14 @@ they mean, and suggestions for how to fix them. The easiest way to enforce this is to ensure alternatives do not contain any ORC entries, which in turn implies the above constraint. +11. file.o: warning: unannotated intra-function call + + This warning means that a direct call is done to a destination which + is not at the beginning of a function. If this is a legit call, you + can remove this warning by putting the ANNOTATE_INTRA_FUNCTION_CALL + directive right before the call. + + If the error doesn't seem to make sense, it could be a bug in objtool. Feel free to ask the objtool maintainer for help. diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index d7b5d1096913..4b504fc90bbb 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -496,6 +496,14 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, case 0xe8: *type = INSN_CALL; + /* + * For the impact on the stack, a CALL behaves like + * a PUSH of an immediate value (the return address). + */ + ADD_OP(op) { + op->src.type = OP_SRC_CONST; + op->dest.type = OP_DEST_PUSH; + } break; case 0xfc: diff --git a/tools/objtool/check.c b/tools/objtool/check.c index d822858764fe..32dea5f3feed 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -674,6 +674,16 @@ static int add_jump_destinations(struct objtool_file *file) return 0; } +static void remove_insn_ops(struct instruction *insn) +{ + struct stack_op *op, *tmp; + + list_for_each_entry_safe(op, tmp, &insn->stack_ops, list) { + list_del(&op->list); + free(op); + } +} + /* * Find the destination instructions for all calls. */ @@ -699,10 +709,7 @@ static int add_call_destinations(struct objtool_file *file) continue; if (!insn->call_dest) { - WARN_FUNC("unsupported intra-function call", - insn->sec, insn->offset); - if (retpoline) - WARN("If this is a retpoline, please patch it in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE."); + WARN_FUNC("unannotated intra-function call", insn->sec, insn->offset); return -1; } @@ -725,6 +732,15 @@ static int add_call_destinations(struct objtool_file *file) } } else insn->call_dest = rela->sym; + + /* + * Whatever stack impact regular CALLs have, should be undone + * by the RETURN of the called function. + * + * Annotated intra-function calls retain the stack_ops but + * are converted to JUMP, see read_intra_function_calls(). + */ + remove_insn_ops(insn); } return 0; @@ -1404,6 +1420,57 @@ static int read_instr_hints(struct objtool_file *file) return 0; } +static int read_intra_function_calls(struct objtool_file *file) +{ + struct instruction *insn; + struct section *sec; + struct rela *rela; + + sec = find_section_by_name(file->elf, ".rela.discard.intra_function_calls"); + if (!sec) + return 0; + + list_for_each_entry(rela, &sec->rela_list, list) { + unsigned long dest_off; + + if (rela->sym->type != STT_SECTION) { + WARN("unexpected relocation symbol type in %s", + sec->name); + return -1; + } + + insn = find_insn(file, rela->sym->sec, rela->addend); + if (!insn) { + WARN("bad .discard.intra_function_call entry"); + return -1; + } + + if (insn->type != INSN_CALL) { + WARN_FUNC("intra_function_call not a direct call", + insn->sec, insn->offset); + return -1; + } + + /* + * Treat intra-function CALLs as JMPs, but with a stack_op. + * See add_call_destinations(), which strips stack_ops from + * normal CALLs. + */ + insn->type = INSN_JUMP_UNCONDITIONAL; + + dest_off = insn->offset + insn->len + insn->immediate; + insn->jump_dest = find_insn(file, insn->sec, dest_off); + if (!insn->jump_dest) { + WARN_FUNC("can't find call dest at %s+0x%lx", + insn->sec, insn->offset, + insn->sec->name, dest_off); + return -1; + } + } + + return 0; +} + static void mark_rodata(struct objtool_file *file) { struct section *sec; @@ -1459,6 +1526,10 @@ static int decode_sections(struct objtool_file *file) if (ret) return ret; + ret = read_intra_function_calls(file); + if (ret) + return ret; + ret = add_call_destinations(file); if (ret) return ret; From 089dd8e53126ebaf506e2dc0bf89d652c36bfc12 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Tue, 14 Apr 2020 12:36:16 +0200 Subject: [PATCH 43/54] x86/speculation: Change FILL_RETURN_BUFFER to work with objtool Change FILL_RETURN_BUFFER so that objtool groks it and can generate correct ORC unwind information. - Since ORC is alternative invariant; that is, all alternatives should have the same ORC entries, the __FILL_RETURN_BUFFER body can not be part of an alternative. Therefore, move it out of the alternative and keep the alternative as a sort of jump_label around it. - Use the ANNOTATE_INTRA_FUNCTION_CALL annotation to white-list these 'funny' call instructions to nowhere. - Use UNWIND_HINT_EMPTY to 'fill' the speculation traps, otherwise objtool will consider them unreachable. - Move the RSP adjustment into the loop, such that the loop has a deterministic stack layout. Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Alexandre Chartre Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200428191700.032079304@infradead.org --- arch/x86/include/asm/nospec-branch.h | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h index 7e9a281e2660..b8890e18f624 100644 --- a/arch/x86/include/asm/nospec-branch.h +++ b/arch/x86/include/asm/nospec-branch.h @@ -4,11 +4,13 @@ #define _ASM_X86_NOSPEC_BRANCH_H_ #include +#include #include #include #include #include +#include /* * This should be used immediately before a retpoline alternative. It tells @@ -46,21 +48,25 @@ #define __FILL_RETURN_BUFFER(reg, nr, sp) \ mov $(nr/2), reg; \ 771: \ + ANNOTATE_INTRA_FUNCTION_CALL; \ call 772f; \ 773: /* speculation trap */ \ + UNWIND_HINT_EMPTY; \ pause; \ lfence; \ jmp 773b; \ 772: \ + ANNOTATE_INTRA_FUNCTION_CALL; \ call 774f; \ 775: /* speculation trap */ \ + UNWIND_HINT_EMPTY; \ pause; \ lfence; \ jmp 775b; \ 774: \ + add $(BITS_PER_LONG/8) * 2, sp; \ dec reg; \ - jnz 771b; \ - add $(BITS_PER_LONG/8) * nr, sp; + jnz 771b; #ifdef __ASSEMBLY__ @@ -137,10 +143,8 @@ */ .macro FILL_RETURN_BUFFER reg:req nr:req ftr:req #ifdef CONFIG_RETPOLINE - ANNOTATE_NOSPEC_ALTERNATIVE - ALTERNATIVE "jmp .Lskip_rsb_\@", \ - __stringify(__FILL_RETURN_BUFFER(\reg,\nr,%_ASM_SP)) \ - \ftr + ALTERNATIVE "jmp .Lskip_rsb_\@", "", \ftr + __FILL_RETURN_BUFFER(\reg,\nr,%_ASM_SP) .Lskip_rsb_\@: #endif .endm From ca3f0d80dd57c8828bfb5bc0bc79750ea7a1ba26 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Wed, 22 Apr 2020 17:03:22 +0200 Subject: [PATCH 44/54] x86: Simplify retpoline declaration Because of how KSYM works, we need one declaration per line. Seeing how we're going to be doubling the amount of retpoline symbols, simplify the machinery in order to avoid having to copy/paste even more. Signed-off-by: Peter Zijlstra (Intel) Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200428191700.091696925@infradead.org --- arch/x86/include/asm/GEN-for-each-reg.h | 25 ++++++++++++++++++ arch/x86/include/asm/asm-prototypes.h | 28 ++++++-------------- arch/x86/lib/retpoline.S | 35 +++++++++++-------------- 3 files changed, 48 insertions(+), 40 deletions(-) create mode 100644 arch/x86/include/asm/GEN-for-each-reg.h diff --git a/arch/x86/include/asm/GEN-for-each-reg.h b/arch/x86/include/asm/GEN-for-each-reg.h new file mode 100644 index 000000000000..1b07fb102c4e --- /dev/null +++ b/arch/x86/include/asm/GEN-for-each-reg.h @@ -0,0 +1,25 @@ +#ifdef CONFIG_64BIT +GEN(rax) +GEN(rbx) +GEN(rcx) +GEN(rdx) +GEN(rsi) +GEN(rdi) +GEN(rbp) +GEN(r8) +GEN(r9) +GEN(r10) +GEN(r11) +GEN(r12) +GEN(r13) +GEN(r14) +GEN(r15) +#else +GEN(eax) +GEN(ebx) +GEN(ecx) +GEN(edx) +GEN(esi) +GEN(edi) +GEN(ebp) +#endif diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h index ce92c4acc913..aa7585e1fe67 100644 --- a/arch/x86/include/asm/asm-prototypes.h +++ b/arch/x86/include/asm/asm-prototypes.h @@ -17,24 +17,12 @@ extern void cmpxchg8b_emu(void); #endif #ifdef CONFIG_RETPOLINE -#ifdef CONFIG_X86_32 -#define INDIRECT_THUNK(reg) extern asmlinkage void __x86_indirect_thunk_e ## reg(void); -#else -#define INDIRECT_THUNK(reg) extern asmlinkage void __x86_indirect_thunk_r ## reg(void); -INDIRECT_THUNK(8) -INDIRECT_THUNK(9) -INDIRECT_THUNK(10) -INDIRECT_THUNK(11) -INDIRECT_THUNK(12) -INDIRECT_THUNK(13) -INDIRECT_THUNK(14) -INDIRECT_THUNK(15) -#endif -INDIRECT_THUNK(ax) -INDIRECT_THUNK(bx) -INDIRECT_THUNK(cx) -INDIRECT_THUNK(dx) -INDIRECT_THUNK(si) -INDIRECT_THUNK(di) -INDIRECT_THUNK(bp) + +#define DECL_INDIRECT_THUNK(reg) \ + extern asmlinkage void __x86_indirect_thunk_ ## reg (void); + +#undef GEN +#define GEN(reg) DECL_INDIRECT_THUNK(reg) +#include + #endif /* CONFIG_RETPOLINE */ diff --git a/arch/x86/lib/retpoline.S b/arch/x86/lib/retpoline.S index 363ec132df7e..9cc5480300c9 100644 --- a/arch/x86/lib/retpoline.S +++ b/arch/x86/lib/retpoline.S @@ -24,25 +24,20 @@ SYM_FUNC_END(__x86_indirect_thunk_\reg) * only see one instance of "__x86_indirect_thunk_\reg" rather * than one per register with the correct names. So we do it * the simple and nasty way... + * + * Worse, you can only have a single EXPORT_SYMBOL per line, + * and CPP can't insert newlines, so we have to repeat everything + * at least twice. */ -#define __EXPORT_THUNK(sym) _ASM_NOKPROBE(sym); EXPORT_SYMBOL(sym) -#define EXPORT_THUNK(reg) __EXPORT_THUNK(__x86_indirect_thunk_ ## reg) -#define GENERATE_THUNK(reg) THUNK reg ; EXPORT_THUNK(reg) -GENERATE_THUNK(_ASM_AX) -GENERATE_THUNK(_ASM_BX) -GENERATE_THUNK(_ASM_CX) -GENERATE_THUNK(_ASM_DX) -GENERATE_THUNK(_ASM_SI) -GENERATE_THUNK(_ASM_DI) -GENERATE_THUNK(_ASM_BP) -#ifdef CONFIG_64BIT -GENERATE_THUNK(r8) -GENERATE_THUNK(r9) -GENERATE_THUNK(r10) -GENERATE_THUNK(r11) -GENERATE_THUNK(r12) -GENERATE_THUNK(r13) -GENERATE_THUNK(r14) -GENERATE_THUNK(r15) -#endif +#define __EXPORT_THUNK(sym) _ASM_NOKPROBE(sym); EXPORT_SYMBOL(sym) +#define EXPORT_THUNK(reg) __EXPORT_THUNK(__x86_indirect_thunk_ ## reg) + +#undef GEN +#define GEN(reg) THUNK reg +#include + +#undef GEN +#define GEN(reg) EXPORT_THUNK(reg) +#include + From 34fdce6981b96920ced4e0ee56e9db3fb03a33f0 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Wed, 22 Apr 2020 17:16:40 +0200 Subject: [PATCH 45/54] x86: Change {JMP,CALL}_NOSPEC argument In order to change the {JMP,CALL}_NOSPEC macros to call out-of-line versions of the retpoline magic, we need to remove the '%' from the argument, such that we can paste it onto symbol names. Signed-off-by: Peter Zijlstra (Intel) Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200428191700.151623523@infradead.org --- arch/x86/crypto/aesni-intel_asm.S | 4 +-- arch/x86/crypto/camellia-aesni-avx-asm_64.S | 2 +- arch/x86/crypto/camellia-aesni-avx2-asm_64.S | 2 +- arch/x86/crypto/crc32c-pcl-intel-asm_64.S | 26 ++++++++++---------- arch/x86/entry/entry_32.S | 6 ++--- arch/x86/entry/entry_64.S | 2 +- arch/x86/include/asm/nospec-branch.h | 16 ++++++------ arch/x86/kernel/ftrace_32.S | 2 +- arch/x86/kernel/ftrace_64.S | 4 +-- arch/x86/lib/checksum_32.S | 4 +-- arch/x86/platform/efi/efi_stub_64.S | 2 +- 11 files changed, 35 insertions(+), 35 deletions(-) diff --git a/arch/x86/crypto/aesni-intel_asm.S b/arch/x86/crypto/aesni-intel_asm.S index cad6e1bfa7d5..54e7d15dbd0d 100644 --- a/arch/x86/crypto/aesni-intel_asm.S +++ b/arch/x86/crypto/aesni-intel_asm.S @@ -2758,7 +2758,7 @@ SYM_FUNC_START(aesni_xts_crypt8) pxor INC, STATE4 movdqu IV, 0x30(OUTP) - CALL_NOSPEC %r11 + CALL_NOSPEC r11 movdqu 0x00(OUTP), INC pxor INC, STATE1 @@ -2803,7 +2803,7 @@ SYM_FUNC_START(aesni_xts_crypt8) _aesni_gf128mul_x_ble() movups IV, (IVP) - CALL_NOSPEC %r11 + CALL_NOSPEC r11 movdqu 0x40(OUTP), INC pxor INC, STATE1 diff --git a/arch/x86/crypto/camellia-aesni-avx-asm_64.S b/arch/x86/crypto/camellia-aesni-avx-asm_64.S index d01ddd73de65..ecc0a9a905c4 100644 --- a/arch/x86/crypto/camellia-aesni-avx-asm_64.S +++ b/arch/x86/crypto/camellia-aesni-avx-asm_64.S @@ -1228,7 +1228,7 @@ SYM_FUNC_START_LOCAL(camellia_xts_crypt_16way) vpxor 14 * 16(%rax), %xmm15, %xmm14; vpxor 15 * 16(%rax), %xmm15, %xmm15; - CALL_NOSPEC %r9; + CALL_NOSPEC r9; addq $(16 * 16), %rsp; diff --git a/arch/x86/crypto/camellia-aesni-avx2-asm_64.S b/arch/x86/crypto/camellia-aesni-avx2-asm_64.S index 563ef6e83cdd..0907243c501c 100644 --- a/arch/x86/crypto/camellia-aesni-avx2-asm_64.S +++ b/arch/x86/crypto/camellia-aesni-avx2-asm_64.S @@ -1339,7 +1339,7 @@ SYM_FUNC_START_LOCAL(camellia_xts_crypt_32way) vpxor 14 * 32(%rax), %ymm15, %ymm14; vpxor 15 * 32(%rax), %ymm15, %ymm15; - CALL_NOSPEC %r9; + CALL_NOSPEC r9; addq $(16 * 32), %rsp; diff --git a/arch/x86/crypto/crc32c-pcl-intel-asm_64.S b/arch/x86/crypto/crc32c-pcl-intel-asm_64.S index 0e6690e3618c..8501ec4532f4 100644 --- a/arch/x86/crypto/crc32c-pcl-intel-asm_64.S +++ b/arch/x86/crypto/crc32c-pcl-intel-asm_64.S @@ -75,7 +75,7 @@ .text SYM_FUNC_START(crc_pcl) -#define bufp %rdi +#define bufp rdi #define bufp_dw %edi #define bufp_w %di #define bufp_b %dil @@ -105,9 +105,9 @@ SYM_FUNC_START(crc_pcl) ## 1) ALIGN: ################################################################ - mov bufp, bufptmp # rdi = *buf - neg bufp - and $7, bufp # calculate the unalignment amount of + mov %bufp, bufptmp # rdi = *buf + neg %bufp + and $7, %bufp # calculate the unalignment amount of # the address je proc_block # Skip if aligned @@ -123,13 +123,13 @@ SYM_FUNC_START(crc_pcl) do_align: #### Calculate CRC of unaligned bytes of the buffer (if any) movq (bufptmp), tmp # load a quadward from the buffer - add bufp, bufptmp # align buffer pointer for quadword + add %bufp, bufptmp # align buffer pointer for quadword # processing - sub bufp, len # update buffer length + sub %bufp, len # update buffer length align_loop: crc32b %bl, crc_init_dw # compute crc32 of 1-byte shr $8, tmp # get next byte - dec bufp + dec %bufp jne align_loop proc_block: @@ -169,10 +169,10 @@ continue_block: xor crc2, crc2 ## branch into array - lea jump_table(%rip), bufp - movzxw (bufp, %rax, 2), len - lea crc_array(%rip), bufp - lea (bufp, len, 1), bufp + lea jump_table(%rip), %bufp + movzxw (%bufp, %rax, 2), len + lea crc_array(%rip), %bufp + lea (%bufp, len, 1), %bufp JMP_NOSPEC bufp ################################################################ @@ -218,9 +218,9 @@ LABEL crc_ %i ## 4) Combine three results: ################################################################ - lea (K_table-8)(%rip), bufp # first entry is for idx 1 + lea (K_table-8)(%rip), %bufp # first entry is for idx 1 shlq $3, %rax # rax *= 8 - pmovzxdq (bufp,%rax), %xmm0 # 2 consts: K1:K2 + pmovzxdq (%bufp,%rax), %xmm0 # 2 consts: K1:K2 leal (%eax,%eax,2), %eax # rax *= 3 (total *24) subq %rax, tmp # tmp -= rax*24 diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S index b67bae7091d7..7e7ffb7a5147 100644 --- a/arch/x86/entry/entry_32.S +++ b/arch/x86/entry/entry_32.S @@ -816,7 +816,7 @@ SYM_CODE_START(ret_from_fork) /* kernel thread */ 1: movl %edi, %eax - CALL_NOSPEC %ebx + CALL_NOSPEC ebx /* * A kernel thread is allowed to return here after successfully * calling do_execve(). Exit to userspace to complete the execve() @@ -1501,7 +1501,7 @@ SYM_CODE_START_LOCAL_NOALIGN(common_exception_read_cr2) TRACE_IRQS_OFF movl %esp, %eax # pt_regs pointer - CALL_NOSPEC %edi + CALL_NOSPEC edi jmp ret_from_exception SYM_CODE_END(common_exception_read_cr2) @@ -1522,7 +1522,7 @@ SYM_CODE_START_LOCAL_NOALIGN(common_exception) TRACE_IRQS_OFF movl %esp, %eax # pt_regs pointer - CALL_NOSPEC %edi + CALL_NOSPEC edi jmp ret_from_exception SYM_CODE_END(common_exception) diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S index 0e9504fabe52..168b798913bc 100644 --- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -349,7 +349,7 @@ SYM_CODE_START(ret_from_fork) /* kernel thread */ UNWIND_HINT_EMPTY movq %r12, %rdi - CALL_NOSPEC %rbx + CALL_NOSPEC rbx /* * A kernel thread is allowed to return here after successfully * calling do_execve(). Exit to userspace to complete the execve() diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h index b8890e18f624..d3269b69728a 100644 --- a/arch/x86/include/asm/nospec-branch.h +++ b/arch/x86/include/asm/nospec-branch.h @@ -118,22 +118,22 @@ .macro JMP_NOSPEC reg:req #ifdef CONFIG_RETPOLINE ANNOTATE_NOSPEC_ALTERNATIVE - ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *\reg), \ - __stringify(RETPOLINE_JMP \reg), X86_FEATURE_RETPOLINE, \ - __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *\reg), X86_FEATURE_RETPOLINE_AMD + ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *%\reg), \ + __stringify(RETPOLINE_JMP %\reg), X86_FEATURE_RETPOLINE,\ + __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *%\reg), X86_FEATURE_RETPOLINE_AMD #else - jmp *\reg + jmp *%\reg #endif .endm .macro CALL_NOSPEC reg:req #ifdef CONFIG_RETPOLINE ANNOTATE_NOSPEC_ALTERNATIVE - ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; call *\reg), \ - __stringify(RETPOLINE_CALL \reg), X86_FEATURE_RETPOLINE,\ - __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; call *\reg), X86_FEATURE_RETPOLINE_AMD + ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; call *%\reg),\ + __stringify(RETPOLINE_CALL %\reg), X86_FEATURE_RETPOLINE,\ + __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; call *%\reg), X86_FEATURE_RETPOLINE_AMD #else - call *\reg + call *%\reg #endif .endm diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S index e8a9f8370112..e405fe1a8bf4 100644 --- a/arch/x86/kernel/ftrace_32.S +++ b/arch/x86/kernel/ftrace_32.S @@ -189,5 +189,5 @@ return_to_handler: movl %eax, %ecx popl %edx popl %eax - JMP_NOSPEC %ecx + JMP_NOSPEC ecx #endif diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S index 9738ed23964e..aa5d28aeb31e 100644 --- a/arch/x86/kernel/ftrace_64.S +++ b/arch/x86/kernel/ftrace_64.S @@ -301,7 +301,7 @@ trace: * function tracing is enabled. */ movq ftrace_trace_function, %r8 - CALL_NOSPEC %r8 + CALL_NOSPEC r8 restore_mcount_regs jmp fgraph_trace @@ -338,6 +338,6 @@ SYM_CODE_START(return_to_handler) movq 8(%rsp), %rdx movq (%rsp), %rax addq $24, %rsp - JMP_NOSPEC %rdi + JMP_NOSPEC rdi SYM_CODE_END(return_to_handler) #endif diff --git a/arch/x86/lib/checksum_32.S b/arch/x86/lib/checksum_32.S index 4742e8fa7ee7..d1d768912368 100644 --- a/arch/x86/lib/checksum_32.S +++ b/arch/x86/lib/checksum_32.S @@ -153,7 +153,7 @@ SYM_FUNC_START(csum_partial) negl %ebx lea 45f(%ebx,%ebx,2), %ebx testl %esi, %esi - JMP_NOSPEC %ebx + JMP_NOSPEC ebx # Handle 2-byte-aligned regions 20: addw (%esi), %ax @@ -436,7 +436,7 @@ SYM_FUNC_START(csum_partial_copy_generic) andl $-32,%edx lea 3f(%ebx,%ebx), %ebx testl %esi, %esi - JMP_NOSPEC %ebx + JMP_NOSPEC ebx 1: addl $64,%esi addl $64,%edi SRC(movb -32(%edx),%bl) ; SRC(movb (%edx),%bl) diff --git a/arch/x86/platform/efi/efi_stub_64.S b/arch/x86/platform/efi/efi_stub_64.S index 15da118f04f0..90380a17ab23 100644 --- a/arch/x86/platform/efi/efi_stub_64.S +++ b/arch/x86/platform/efi/efi_stub_64.S @@ -21,7 +21,7 @@ SYM_FUNC_START(__efi_call) mov %r8, %r9 mov %rcx, %r8 mov %rsi, %rcx - CALL_NOSPEC %rdi + CALL_NOSPEC rdi leave ret SYM_FUNC_END(__efi_call) From cc1ac9c792810b93783a7de344f428922af8d98c Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Thu, 16 Apr 2020 14:34:26 +0200 Subject: [PATCH 46/54] x86/retpoline: Fix retpoline unwind Currently objtool cannot understand retpolines, and thus cannot generate ORC unwind information for them. This means that we cannot unwind from the middle of a retpoline. The recent ANNOTATE_INTRA_FUNCTION_CALL and UNWIND_HINT_RET_OFFSET support in objtool enables it to understand the basic retpoline construct. A further problem is that the ORC unwind information is alternative invariant; IOW. every alternative should have the same ORC, retpolines obviously violate this. This means we need to out-of-line them. Since all GCC generated code already uses out-of-line retpolines, this should not affect performance much, if anything. This will enable objtool to generate valid ORC data for the out-of-line copies, which means we can correctly and reliably unwind through a retpoline. Signed-off-by: Peter Zijlstra (Intel) Acked-by: Josh Poimboeuf Link: https://lkml.kernel.org/r/20200428191700.210835357@infradead.org --- arch/x86/include/asm/asm-prototypes.h | 7 ++++ arch/x86/include/asm/nospec-branch.h | 56 ++++----------------------- arch/x86/lib/retpoline.S | 26 +++++++++++-- 3 files changed, 38 insertions(+), 51 deletions(-) diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h index aa7585e1fe67..9bf2620ce817 100644 --- a/arch/x86/include/asm/asm-prototypes.h +++ b/arch/x86/include/asm/asm-prototypes.h @@ -21,8 +21,15 @@ extern void cmpxchg8b_emu(void); #define DECL_INDIRECT_THUNK(reg) \ extern asmlinkage void __x86_indirect_thunk_ ## reg (void); +#define DECL_RETPOLINE(reg) \ + extern asmlinkage void __x86_retpoline_ ## reg (void); + #undef GEN #define GEN(reg) DECL_INDIRECT_THUNK(reg) #include +#undef GEN +#define GEN(reg) DECL_RETPOLINE(reg) +#include + #endif /* CONFIG_RETPOLINE */ diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h index d3269b69728a..d52d1aacdd97 100644 --- a/arch/x86/include/asm/nospec-branch.h +++ b/arch/x86/include/asm/nospec-branch.h @@ -12,15 +12,6 @@ #include #include -/* - * This should be used immediately before a retpoline alternative. It tells - * objtool where the retpolines are so that it can make sense of the control - * flow by just reading the original instruction(s) and ignoring the - * alternatives. - */ -#define ANNOTATE_NOSPEC_ALTERNATIVE \ - ANNOTATE_IGNORE_ALTERNATIVE - /* * Fill the CPU return stack buffer. * @@ -82,34 +73,6 @@ .popsection .endm -/* - * These are the bare retpoline primitives for indirect jmp and call. - * Do not use these directly; they only exist to make the ALTERNATIVE - * invocation below less ugly. - */ -.macro RETPOLINE_JMP reg:req - call .Ldo_rop_\@ -.Lspec_trap_\@: - pause - lfence - jmp .Lspec_trap_\@ -.Ldo_rop_\@: - mov \reg, (%_ASM_SP) - ret -.endm - -/* - * This is a wrapper around RETPOLINE_JMP so the called function in reg - * returns to the instruction after the macro. - */ -.macro RETPOLINE_CALL reg:req - jmp .Ldo_call_\@ -.Ldo_retpoline_jmp_\@: - RETPOLINE_JMP \reg -.Ldo_call_\@: - call .Ldo_retpoline_jmp_\@ -.endm - /* * JMP_NOSPEC and CALL_NOSPEC macros can be used instead of a simple * indirect jmp/call which may be susceptible to the Spectre variant 2 @@ -117,10 +80,9 @@ */ .macro JMP_NOSPEC reg:req #ifdef CONFIG_RETPOLINE - ANNOTATE_NOSPEC_ALTERNATIVE - ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *%\reg), \ - __stringify(RETPOLINE_JMP %\reg), X86_FEATURE_RETPOLINE,\ - __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *%\reg), X86_FEATURE_RETPOLINE_AMD + ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *%\reg), \ + __stringify(jmp __x86_retpoline_\reg), X86_FEATURE_RETPOLINE, \ + __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *%\reg), X86_FEATURE_RETPOLINE_AMD #else jmp *%\reg #endif @@ -128,10 +90,9 @@ .macro CALL_NOSPEC reg:req #ifdef CONFIG_RETPOLINE - ANNOTATE_NOSPEC_ALTERNATIVE - ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; call *%\reg),\ - __stringify(RETPOLINE_CALL %\reg), X86_FEATURE_RETPOLINE,\ - __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; call *%\reg), X86_FEATURE_RETPOLINE_AMD + ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; call *%\reg), \ + __stringify(call __x86_retpoline_\reg), X86_FEATURE_RETPOLINE, \ + __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; call *%\reg), X86_FEATURE_RETPOLINE_AMD #else call *%\reg #endif @@ -165,16 +126,16 @@ * which is ensured when CONFIG_RETPOLINE is defined. */ # define CALL_NOSPEC \ - ANNOTATE_NOSPEC_ALTERNATIVE \ ALTERNATIVE_2( \ ANNOTATE_RETPOLINE_SAFE \ "call *%[thunk_target]\n", \ - "call __x86_indirect_thunk_%V[thunk_target]\n", \ + "call __x86_retpoline_%V[thunk_target]\n", \ X86_FEATURE_RETPOLINE, \ "lfence;\n" \ ANNOTATE_RETPOLINE_SAFE \ "call *%[thunk_target]\n", \ X86_FEATURE_RETPOLINE_AMD) + # define THUNK_TARGET(addr) [thunk_target] "r" (addr) #else /* CONFIG_X86_32 */ @@ -184,7 +145,6 @@ * here, anyway. */ # define CALL_NOSPEC \ - ANNOTATE_NOSPEC_ALTERNATIVE \ ALTERNATIVE_2( \ ANNOTATE_RETPOLINE_SAFE \ "call *%[thunk_target]\n", \ diff --git a/arch/x86/lib/retpoline.S b/arch/x86/lib/retpoline.S index 9cc5480300c9..b4c43a9b1483 100644 --- a/arch/x86/lib/retpoline.S +++ b/arch/x86/lib/retpoline.S @@ -7,15 +7,31 @@ #include #include #include +#include +#include .macro THUNK reg .section .text.__x86.indirect_thunk + .align 32 SYM_FUNC_START(__x86_indirect_thunk_\reg) - CFI_STARTPROC - JMP_NOSPEC %\reg - CFI_ENDPROC + JMP_NOSPEC \reg SYM_FUNC_END(__x86_indirect_thunk_\reg) + +SYM_FUNC_START_NOALIGN(__x86_retpoline_\reg) + ANNOTATE_INTRA_FUNCTION_CALL + call .Ldo_rop_\@ +.Lspec_trap_\@: + UNWIND_HINT_EMPTY + pause + lfence + jmp .Lspec_trap_\@ +.Ldo_rop_\@: + mov %\reg, (%_ASM_SP) + UNWIND_HINT_RET_OFFSET + ret +SYM_FUNC_END(__x86_retpoline_\reg) + .endm /* @@ -32,6 +48,7 @@ SYM_FUNC_END(__x86_indirect_thunk_\reg) #define __EXPORT_THUNK(sym) _ASM_NOKPROBE(sym); EXPORT_SYMBOL(sym) #define EXPORT_THUNK(reg) __EXPORT_THUNK(__x86_indirect_thunk_ ## reg) +#define EXPORT_RETPOLINE(reg) __EXPORT_THUNK(__x86_retpoline_ ## reg) #undef GEN #define GEN(reg) THUNK reg @@ -41,3 +58,6 @@ SYM_FUNC_END(__x86_indirect_thunk_\reg) #define GEN(reg) EXPORT_THUNK(reg) #include +#undef GEN +#define GEN(reg) EXPORT_RETPOLINE(reg) +#include From ab3852ab5cb8fd2e2c5bfa176e5f953353836907 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Fri, 8 May 2020 12:34:33 +0200 Subject: [PATCH 47/54] objtool: Allow no-op CFI ops in alternatives Randy reported a false-positive: arch/x86/hyperv/hv_apic.o: warning: objtool: hv_apic_write()+0x25: alternative modifies stack What happens is that: alternative_io("movl %0, %P1", "xchgl %0, %P1", X86_BUG_11AP, 13d: 89 9d 00 d0 7f ff mov %ebx,-0x803000(%rbp) decodes to an instruction with CFI-ops because it modifies RBP. However, due to this being a !frame-pointer build, that should not in fact change the CFI state. So instead of dis-allowing any CFI-op, verify the op would've actually changed the CFI state. Fixes: 7117f16bf460 ("objtool: Fix ORC vs alternatives") Reported-by: Randy Dunlap Signed-off-by: Peter Zijlstra (Intel) Acked-by: Josh Poimboeuf Acked-by: Randy Dunlap Tested-by: Randy Dunlap --- tools/objtool/check.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 32dea5f3feed..196a55101f3c 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2060,17 +2060,18 @@ static int handle_insn_ops(struct instruction *insn, struct insn_state *state) struct stack_op *op; list_for_each_entry(op, &insn->stack_ops, list) { + struct cfi_state old_cfi = state->cfi; int res; - if (insn->alt_group) { - WARN_FUNC("alternative modifies stack", insn->sec, insn->offset); - return -1; - } - res = update_cfi_state(insn, &state->cfi, op); if (res) return res; + if (insn->alt_group && memcmp(&state->cfi, &old_cfi, sizeof(struct cfi_state))) { + WARN_FUNC("alternative modifies stack", insn->sec, insn->offset); + return -1; + } + if (op->dest.type == OP_DEST_PUSHF) { if (!state->uaccess_stack) { state->uaccess_stack = 1; From 28fe1d7bf89f8ed5be70b98a33932dbaf99345dd Mon Sep 17 00:00:00 2001 From: Sami Tolvanen Date: Tue, 21 Apr 2020 15:08:42 -0700 Subject: [PATCH 48/54] objtool: use gelf_getsymshndx to handle >64k sections Currently, objtool fails to load the correct section for symbols when the index is greater than SHN_LORESERVE. Use gelf_getsymshndx instead of gelf_getsym to handle >64k sections. Signed-off-by: Sami Tolvanen Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Kees Cook Link: https://lkml.kernel.org/r/20200421220843.188260-2-samitolvanen@google.com --- tools/objtool/elf.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index b6349caaeb5c..84225679f96d 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -343,12 +343,14 @@ static int read_sections(struct elf *elf) static int read_symbols(struct elf *elf) { - struct section *symtab, *sec; + struct section *symtab, *symtab_shndx, *sec; struct symbol *sym, *pfunc; struct list_head *entry; struct rb_node *pnode; int symbols_nr, i; char *coldstr; + Elf_Data *shndx_data = NULL; + Elf32_Word shndx; symtab = find_section_by_name(elf, ".symtab"); if (!symtab) { @@ -356,6 +358,10 @@ static int read_symbols(struct elf *elf) return -1; } + symtab_shndx = find_section_by_name(elf, ".symtab_shndx"); + if (symtab_shndx) + shndx_data = symtab_shndx->data; + symbols_nr = symtab->sh.sh_size / symtab->sh.sh_entsize; for (i = 0; i < symbols_nr; i++) { @@ -369,8 +375,9 @@ static int read_symbols(struct elf *elf) sym->idx = i; - if (!gelf_getsym(symtab->data, i, &sym->sym)) { - WARN_ELF("gelf_getsym"); + if (!gelf_getsymshndx(symtab->data, shndx_data, i, &sym->sym, + &shndx)) { + WARN_ELF("gelf_getsymshndx"); goto err; } @@ -384,10 +391,13 @@ static int read_symbols(struct elf *elf) sym->type = GELF_ST_TYPE(sym->sym.st_info); sym->bind = GELF_ST_BIND(sym->sym.st_info); - if (sym->sym.st_shndx > SHN_UNDEF && - sym->sym.st_shndx < SHN_LORESERVE) { - sym->sec = find_section_by_index(elf, - sym->sym.st_shndx); + if ((sym->sym.st_shndx > SHN_UNDEF && + sym->sym.st_shndx < SHN_LORESERVE) || + (shndx_data && sym->sym.st_shndx == SHN_XINDEX)) { + if (sym->sym.st_shndx != SHN_XINDEX) + shndx = sym->sym.st_shndx; + + sym->sec = find_section_by_index(elf, shndx); if (!sym->sec) { WARN("couldn't find section for symbol %s", sym->name); From 6b5dd716da8fc3aba65e6b7d992dea0cee2f9528 Mon Sep 17 00:00:00 2001 From: Sami Tolvanen Date: Tue, 21 Apr 2020 15:08:43 -0700 Subject: [PATCH 49/54] objtool: optimize add_dead_ends for split sections Instead of iterating through all instructions to find the last instruction each time .rela.discard.(un)reachable points beyond the section, use find_insn to locate the last instruction by looking at the last bytes of the section instead. Suggested-by: Josh Poimboeuf Signed-off-by: Sami Tolvanen Signed-off-by: Peter Zijlstra (Intel) Link: https://lkml.kernel.org/r/20200421220843.188260-3-samitolvanen@google.com --- tools/objtool/check.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 196a55101f3c..6b2b458a5b0e 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -322,6 +322,19 @@ err: return ret; } +static struct instruction *find_last_insn(struct objtool_file *file, + struct section *sec) +{ + struct instruction *insn = NULL; + unsigned int offset; + unsigned int end = (sec->len > 10) ? sec->len - 10 : 0; + + for (offset = sec->len - 1; offset >= end && !insn; offset--) + insn = find_insn(file, sec, offset); + + return insn; +} + /* * Mark "ud2" instructions and manually annotated dead ends. */ @@ -330,7 +343,6 @@ static int add_dead_ends(struct objtool_file *file) struct section *sec; struct rela *rela; struct instruction *insn; - bool found; /* * By default, "ud2" is a dead end unless otherwise annotated, because @@ -356,15 +368,8 @@ static int add_dead_ends(struct objtool_file *file) if (insn) insn = list_prev_entry(insn, list); else if (rela->addend == rela->sym->sec->len) { - found = false; - list_for_each_entry_reverse(insn, &file->insn_list, list) { - if (insn->sec == rela->sym->sec) { - found = true; - break; - } - } - - if (!found) { + insn = find_last_insn(file, rela->sym->sec); + if (!insn) { WARN("can't find unreachable insn at %s+0x%x", rela->sym->sec->name, rela->addend); return -1; @@ -398,15 +403,8 @@ reachable: if (insn) insn = list_prev_entry(insn, list); else if (rela->addend == rela->sym->sec->len) { - found = false; - list_for_each_entry_reverse(insn, &file->insn_list, list) { - if (insn->sec == rela->sym->sec) { - found = true; - break; - } - } - - if (!found) { + insn = find_last_insn(file, rela->sym->sec); + if (!insn) { WARN("can't find reachable insn at %s+0x%x", rela->sym->sec->name, rela->addend); return -1; From 9d907f1ae80b8a67d5397e26912b9d56d0b70a02 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Fri, 24 Apr 2020 15:40:43 -0500 Subject: [PATCH 50/54] samples/ftrace: Fix asm function ELF annotations Enable objtool coverage for the sample ftrace modules by adding ELF annotations to the asm trampoline functions. samples/ftrace/ftrace-direct.o: warning: objtool: .text+0x0: unreachable instruction samples/ftrace/ftrace-direct-modify.o: warning: objtool: .text+0x0: unreachable instruction samples/ftrace/ftrace-direct-too.o: warning: objtool: .text+0x0: unreachable instruction Reported-by: Randy Dunlap Signed-off-by: Josh Poimboeuf --- samples/ftrace/ftrace-direct-modify.c | 4 ++++ samples/ftrace/ftrace-direct-too.c | 2 ++ samples/ftrace/ftrace-direct.c | 2 ++ 3 files changed, 8 insertions(+) diff --git a/samples/ftrace/ftrace-direct-modify.c b/samples/ftrace/ftrace-direct-modify.c index e04229d21475..c13a5bc5095b 100644 --- a/samples/ftrace/ftrace-direct-modify.c +++ b/samples/ftrace/ftrace-direct-modify.c @@ -20,18 +20,22 @@ static unsigned long my_ip = (unsigned long)schedule; asm ( " .pushsection .text, \"ax\", @progbits\n" +" .type my_tramp1, @function\n" " my_tramp1:" " pushq %rbp\n" " movq %rsp, %rbp\n" " call my_direct_func1\n" " leave\n" +" .size my_tramp1, .-my_tramp1\n" " ret\n" +" .type my_tramp2, @function\n" " my_tramp2:" " pushq %rbp\n" " movq %rsp, %rbp\n" " call my_direct_func2\n" " leave\n" " ret\n" +" .size my_tramp2, .-my_tramp2\n" " .popsection\n" ); diff --git a/samples/ftrace/ftrace-direct-too.c b/samples/ftrace/ftrace-direct-too.c index 27efa5f6ff52..d5c5022be664 100644 --- a/samples/ftrace/ftrace-direct-too.c +++ b/samples/ftrace/ftrace-direct-too.c @@ -15,6 +15,7 @@ extern void my_tramp(void *); asm ( " .pushsection .text, \"ax\", @progbits\n" +" .type my_tramp, @function\n" " my_tramp:" " pushq %rbp\n" " movq %rsp, %rbp\n" @@ -27,6 +28,7 @@ asm ( " popq %rdi\n" " leave\n" " ret\n" +" .size my_tramp, .-my_tramp\n" " .popsection\n" ); diff --git a/samples/ftrace/ftrace-direct.c b/samples/ftrace/ftrace-direct.c index a2e3063bd306..63ca06d42c80 100644 --- a/samples/ftrace/ftrace-direct.c +++ b/samples/ftrace/ftrace-direct.c @@ -13,6 +13,7 @@ extern void my_tramp(void *); asm ( " .pushsection .text, \"ax\", @progbits\n" +" .type my_tramp, @function\n" " my_tramp:" " pushq %rbp\n" " movq %rsp, %rbp\n" @@ -21,6 +22,7 @@ asm ( " popq %rdi\n" " leave\n" " ret\n" +" .size my_tramp, .-my_tramp\n" " .popsection\n" ); From ae033f088f277efd5b3c6d681ce9e7682380efff Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 29 Apr 2020 14:09:04 -0500 Subject: [PATCH 51/54] objtool: Add check_kcov_mode() to the uaccess safelist check_kcov_mode() is called by write_comp_data() and __sanitizer_cov_trace_pc(), which are already on the uaccess safe list. It's notrace and doesn't call out to anything else, so add it to the list too. This fixes the following warnings: kernel/kcov.o: warning: objtool: __sanitizer_cov_trace_pc()+0x15: call to check_kcov_mode() with UACCESS enabled kernel/kcov.o: warning: objtool: write_comp_data()+0x1b: call to check_kcov_mode() with UACCESS enabled Reported-by: Arnd Bergmann Signed-off-by: Josh Poimboeuf Acked-by: Peter Zijlstra (Intel) --- tools/objtool/check.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index e36a818a2fed..7a47ff9d39f7 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -507,6 +507,7 @@ static const char *uaccess_safe_builtin[] = { "__asan_report_store16_noabort", /* KCOV */ "write_comp_data", + "check_kcov_mode", "__sanitizer_cov_trace_pc", "__sanitizer_cov_trace_const_cmp1", "__sanitizer_cov_trace_const_cmp2", From f15c648f202cd0232d4a9c98627bc08bcd6d11ee Mon Sep 17 00:00:00 2001 From: Matt Helsley Date: Tue, 19 May 2020 13:55:31 -0700 Subject: [PATCH 52/54] objtool: Exit successfully when requesting help When the user requests help it's not an error so do not exit with a non-zero exit code. This is not especially useful for a user but any script that might wish to check that objtool --help is at least available can't rely on the exit code to crudely check that, for example, building an objtool executable succeeds. Signed-off-by: Matt Helsley Signed-off-by: Josh Poimboeuf --- tools/objtool/objtool.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/objtool/objtool.c b/tools/objtool/objtool.c index 0b3528f05053..58fdda510653 100644 --- a/tools/objtool/objtool.c +++ b/tools/objtool/objtool.c @@ -58,7 +58,9 @@ static void cmd_usage(void) printf("\n"); - exit(129); + if (!help) + exit(129); + exit(0); } static void handle_options(int *argc, const char ***argv) From d37c90d47fc4657423d2ff1c3ed3fd70612a9b43 Mon Sep 17 00:00:00 2001 From: Matt Helsley Date: Tue, 19 May 2020 13:55:32 -0700 Subject: [PATCH 53/54] objtool: Move struct objtool_file into arch-independent header The objtool_file structure describes the files objtool works on, is used by the check subcommand, and the check.h header is included by the orc subcommands so it's presently used by all subcommands. Since the structure will be useful in all subcommands besides check, and some subcommands may not want to include check.h to get the definition, split the structure out into a new header meant for use by all objtool subcommands. Signed-off-by: Matt Helsley Reviewed-by: Julien Thierry Signed-off-by: Josh Poimboeuf --- tools/objtool/check.h | 10 +--------- tools/objtool/objtool.h | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 9 deletions(-) create mode 100644 tools/objtool/objtool.h diff --git a/tools/objtool/check.h b/tools/objtool/check.h index 24280227ef21..3b59a1cbcff5 100644 --- a/tools/objtool/check.h +++ b/tools/objtool/check.h @@ -7,11 +7,10 @@ #define _CHECK_H #include -#include "elf.h" +#include "objtool.h" #include "cfi.h" #include "arch.h" #include "orc.h" -#include struct insn_state { struct cfi_state cfi; @@ -48,13 +47,6 @@ struct instruction { struct orc_entry orc; }; -struct objtool_file { - struct elf *elf; - struct list_head insn_list; - DECLARE_HASHTABLE(insn_hash, 20); - bool ignore_unreachables, c_file, hints, rodata; -}; - int check(const char *objname, bool orc); struct instruction *find_insn(struct objtool_file *file, diff --git a/tools/objtool/objtool.h b/tools/objtool/objtool.h new file mode 100644 index 000000000000..d89616b2ca39 --- /dev/null +++ b/tools/objtool/objtool.h @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2020 Matt Helsley + */ + +#ifndef _OBJTOOL_H +#define _OBJTOOL_H + +#include +#include +#include + +#include "elf.h" + +struct objtool_file { + struct elf *elf; + struct list_head insn_list; + DECLARE_HASHTABLE(insn_hash, 20); + bool ignore_unreachables, c_file, hints, rodata; +}; + +#endif /* _OBJTOOL_H */ From 0decf1f8de919782b152daf9c991967a2bac54f0 Mon Sep 17 00:00:00 2001 From: Matt Helsley Date: Tue, 19 May 2020 13:55:33 -0700 Subject: [PATCH 54/54] objtool: Enable compilation of objtool for all architectures Objtool currently only compiles for x86 architectures. This is fine as it presently does not support tooling for other architectures. However, we would like to be able to convert other kernel tools to run as objtool sub commands because they too process ELF object files. This will allow us to convert tools such as recordmcount to use objtool's ELF code. Since much of recordmcount's ELF code is copy-paste code to/from a variety of other kernel tools (look at modpost for example) this means that if we can convert recordmcount we can convert more. We define weak definitions for subcommand entry functions and other weak definitions for shared functions critical to building existing subcommands. These return 127 when the command is missing which signify tools that do not exist on all architectures. In this case the "check" and "orc" tools do not exist on all architectures so we only add them for x86. Future changes adding support for "check", to arm64 for example, can then modify the SUBCMD_CHECK variable when building for arm64. Objtool is not currently wired in to KConfig to be built for other architectures because it's not needed for those architectures and there are no commands it supports other than those for x86. As more command support is enabled on various architectures the necessary KConfig changes can be made (e.g. adding "STACK_VALIDATION") to trigger building objtool. [ jpoimboe: remove aliases, add __weak macro, add error messages ] Cc: Julien Thierry Signed-off-by: Matt Helsley Signed-off-by: Josh Poimboeuf --- tools/objtool/Build | 13 ++++++++---- tools/objtool/Makefile | 10 +++++++++ tools/objtool/arch.h | 4 +++- tools/objtool/builtin-check.c | 2 +- tools/objtool/builtin-orc.c | 3 +-- tools/objtool/check.c | 6 +++--- tools/objtool/check.h | 4 ---- tools/objtool/objtool.h | 5 +++++ tools/objtool/orc.h | 18 ---------------- tools/objtool/orc_dump.c | 3 ++- tools/objtool/orc_gen.c | 1 - tools/objtool/weak.c | 40 +++++++++++++++++++++++++++++++++++ 12 files changed, 74 insertions(+), 35 deletions(-) delete mode 100644 tools/objtool/orc.h create mode 100644 tools/objtool/weak.c diff --git a/tools/objtool/Build b/tools/objtool/Build index 66f44f5cd2a6..b7222d5cc7bc 100644 --- a/tools/objtool/Build +++ b/tools/objtool/Build @@ -1,11 +1,16 @@ objtool-y += arch/$(SRCARCH)/ + +objtool-y += weak.o + +objtool-$(SUBCMD_CHECK) += check.o +objtool-$(SUBCMD_CHECK) += special.o +objtool-$(SUBCMD_ORC) += check.o +objtool-$(SUBCMD_ORC) += orc_gen.o +objtool-$(SUBCMD_ORC) += orc_dump.o + objtool-y += builtin-check.o objtool-y += builtin-orc.o -objtool-y += check.o -objtool-y += orc_gen.o -objtool-y += orc_dump.o objtool-y += elf.o -objtool-y += special.o objtool-y += objtool.o objtool-y += libstring.o diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile index 6b91388aecbb..7770edcda3a0 100644 --- a/tools/objtool/Makefile +++ b/tools/objtool/Makefile @@ -46,6 +46,16 @@ elfshdr := $(shell echo '$(pound)include ' | $(CC) $(CFLAGS) -x c -E - CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED) AWK = awk + +SUBCMD_CHECK := n +SUBCMD_ORC := n + +ifeq ($(SRCARCH),x86) + SUBCMD_CHECK := y + SUBCMD_ORC := y +endif + +export SUBCMD_CHECK SUBCMD_ORC export srctree OUTPUT CFLAGS SRCARCH AWK include $(srctree)/tools/build/Makefile.include diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h index cd118eb4248a..eda15a5a285e 100644 --- a/tools/objtool/arch.h +++ b/tools/objtool/arch.h @@ -8,9 +8,11 @@ #include #include -#include "elf.h" +#include "objtool.h" #include "cfi.h" +#include + enum insn_type { INSN_JUMP_CONDITIONAL, INSN_JUMP_UNCONDITIONAL, diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c index be42b716166b..7a44174967b5 100644 --- a/tools/objtool/builtin-check.c +++ b/tools/objtool/builtin-check.c @@ -16,7 +16,7 @@ #include #include #include "builtin.h" -#include "check.h" +#include "objtool.h" bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats, validate_dup, vmlinux; diff --git a/tools/objtool/builtin-orc.c b/tools/objtool/builtin-orc.c index 5f7cc6157edd..b1dfe2007962 100644 --- a/tools/objtool/builtin-orc.c +++ b/tools/objtool/builtin-orc.c @@ -14,8 +14,7 @@ #include #include "builtin.h" -#include "check.h" - +#include "objtool.h" static const char *orc_usage[] = { "objtool orc generate [] file.o", diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 7a47ff9d39f7..63d65a702900 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -7,10 +7,10 @@ #include #include "builtin.h" -#include "check.h" -#include "elf.h" -#include "special.h" +#include "cfi.h" #include "arch.h" +#include "check.h" +#include "special.h" #include "warn.h" #include diff --git a/tools/objtool/check.h b/tools/objtool/check.h index 3b59a1cbcff5..906b5210f7ca 100644 --- a/tools/objtool/check.h +++ b/tools/objtool/check.h @@ -7,10 +7,8 @@ #define _CHECK_H #include -#include "objtool.h" #include "cfi.h" #include "arch.h" -#include "orc.h" struct insn_state { struct cfi_state cfi; @@ -47,8 +45,6 @@ struct instruction { struct orc_entry orc; }; -int check(const char *objname, bool orc); - struct instruction *find_insn(struct objtool_file *file, struct section *sec, unsigned long offset); diff --git a/tools/objtool/objtool.h b/tools/objtool/objtool.h index d89616b2ca39..528028a66816 100644 --- a/tools/objtool/objtool.h +++ b/tools/objtool/objtool.h @@ -19,4 +19,9 @@ struct objtool_file { bool ignore_unreachables, c_file, hints, rodata; }; +int check(const char *objname, bool orc); +int orc_dump(const char *objname); +int create_orc(struct objtool_file *file); +int create_orc_sections(struct objtool_file *file); + #endif /* _OBJTOOL_H */ diff --git a/tools/objtool/orc.h b/tools/objtool/orc.h deleted file mode 100644 index ee2832221e62..000000000000 --- a/tools/objtool/orc.h +++ /dev/null @@ -1,18 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ -/* - * Copyright (C) 2017 Josh Poimboeuf - */ - -#ifndef _ORC_H -#define _ORC_H - -#include - -struct objtool_file; - -int create_orc(struct objtool_file *file); -int create_orc_sections(struct objtool_file *file); - -int orc_dump(const char *objname); - -#endif /* _ORC_H */ diff --git a/tools/objtool/orc_dump.c b/tools/objtool/orc_dump.c index ba4cbb1cdd63..fca46e006fc2 100644 --- a/tools/objtool/orc_dump.c +++ b/tools/objtool/orc_dump.c @@ -4,7 +4,8 @@ */ #include -#include "orc.h" +#include +#include "objtool.h" #include "warn.h" static const char *reg_name(unsigned int reg) diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c index 9d2bf2daaaa6..c9549988121a 100644 --- a/tools/objtool/orc_gen.c +++ b/tools/objtool/orc_gen.c @@ -6,7 +6,6 @@ #include #include -#include "orc.h" #include "check.h" #include "warn.h" diff --git a/tools/objtool/weak.c b/tools/objtool/weak.c new file mode 100644 index 000000000000..942ea5e8ac36 --- /dev/null +++ b/tools/objtool/weak.c @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2020 Matt Helsley + * Weak definitions necessary to compile objtool without + * some subcommands (e.g. check, orc). + */ + +#include +#include +#include "objtool.h" + +#define __weak __attribute__((weak)) + +#define UNSUPPORTED(name) \ +({ \ + fprintf(stderr, "error: objtool: " name " not implemented\n"); \ + return ENOSYS; \ +}) + +const char __weak *objname; + +int __weak check(const char *_objname, bool orc) +{ + UNSUPPORTED("check subcommand"); +} + +int __weak orc_dump(const char *_objname) +{ + UNSUPPORTED("orc"); +} + +int __weak create_orc(struct objtool_file *file) +{ + UNSUPPORTED("orc"); +} + +int __weak create_orc_sections(struct objtool_file *file) +{ + UNSUPPORTED("orc"); +}