Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751492AbdF1PN6 (ORCPT ); Wed, 28 Jun 2017 11:13:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39866 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752753AbdF1PMC (ORCPT ); Wed, 28 Jun 2017 11:12:02 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 57D04A021E Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jpoimboe@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 57D04A021E From: Josh Poimboeuf To: x86@kernel.org Cc: linux-kernel@vger.kernel.org, live-patching@vger.kernel.org, Linus Torvalds , Andy Lutomirski , Jiri Slaby , Ingo Molnar , "H. Peter Anvin" , Peter Zijlstra Subject: [PATCH v2 5/8] objtool, x86: add facility for asm code to provide unwind hints Date: Wed, 28 Jun 2017 10:11:09 -0500 Message-Id: <44bb46324a5f98a7a7b366ed9b659a5b007c0034.1498659915.git.jpoimboe@redhat.com> In-Reply-To: References: X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 28 Jun 2017 15:12:01 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 14917 Lines: 510 Some asm (and inline asm) code does special things to the stack which objtool can't understand. (Nor can GCC or GNU assembler, for that matter.) In such cases we need a facility for the code to provide annotations, so the unwinder can unwind through it. This provides such a facility, in the form of unwind hints. They're similar to the GNU assembler .cfi* directives, but they give more information, and are needed in far fewer places, because objtool can fill in the blanks by following branches and adjusting the stack pointer for pushes and pops. Signed-off-by: Josh Poimboeuf --- .../x86/include/asm}/undwarf-types.h | 18 +++ arch/x86/include/asm/undwarf.h | 103 ++++++++++++ tools/objtool/Makefile | 3 + tools/objtool/check.c | 179 ++++++++++++++++++++- tools/objtool/check.h | 4 +- tools/objtool/undwarf-types.h | 18 +++ 6 files changed, 317 insertions(+), 8 deletions(-) copy {tools/objtool => arch/x86/include/asm}/undwarf-types.h (84%) create mode 100644 arch/x86/include/asm/undwarf.h diff --git a/tools/objtool/undwarf-types.h b/arch/x86/include/asm/undwarf-types.h similarity index 84% copy from tools/objtool/undwarf-types.h copy to arch/x86/include/asm/undwarf-types.h index ef92a1d..4e5e283 100644 --- a/tools/objtool/undwarf-types.h +++ b/arch/x86/include/asm/undwarf-types.h @@ -57,11 +57,17 @@ * * UNDWARF_TYPE_REGS_IRET: Used in entry code to indicate that * cfa_reg+cfa_offset points to the iret return frame. + * + * The UNWIND_HINT macros are only used for the unwind_hint struct. They are + * not used for the undwarf struct due to size and complexity constraints. */ #define UNDWARF_TYPE_CFA 0 #define UNDWARF_TYPE_REGS 1 #define UNDWARF_TYPE_REGS_IRET 2 +#define UNWIND_HINT_TYPE_SAVE 3 +#define UNWIND_HINT_TYPE_RESTORE 4 +#ifndef __ASSEMBLY__ /* * This struct contains a simplified version of the DWARF Call Frame * Information standard. It contains only the necessary parts of the real @@ -78,4 +84,16 @@ struct undwarf { unsigned type:2; }; +/* + * This struct is used by asm and inline asm code to manually annotate the + * location of registers on the stack for the undwarf unwinder. + */ +struct unwind_hint { + unsigned int ip; + short cfa_offset; + unsigned char cfa_reg; + unsigned char type; +}; +#endif /* __ASSEMBLY__ */ + #endif /* _UNDWARF_TYPES_H */ diff --git a/arch/x86/include/asm/undwarf.h b/arch/x86/include/asm/undwarf.h new file mode 100644 index 0000000..41384e6 --- /dev/null +++ b/arch/x86/include/asm/undwarf.h @@ -0,0 +1,103 @@ +#ifndef _ASM_X86_UNDWARF_H +#define _ASM_X86_UNDWARF_H + +#include "undwarf-types.h" + +#ifdef __ASSEMBLY__ + +/* + * In asm, there are two kinds of code: normal C-type callable functions and + * the rest. The normal callable functions can be called by other code, and + * don't do anything unusual with the stack. Such normal callable functions + * are annotated with the ENTRY/ENDPROC macros. Most asm code falls in this + * category. In this case, no special debugging annotations are needed because + * objtool can automatically generate the .undwarf section which the undwarf + * unwinder reads at runtime. + * + * Anything which doesn't fall into the above category, such as syscall and + * interrupt handlers, tends to not be called directly by other functions, and + * often does unusual non-C-function-type things with the stack pointer. Such + * code needs to be annotated such that objtool can understand it. The + * following CFI hint macros are for this type of code. + * + * These macros provide hints to objtool about the state of the stack at each + * instruction. Objtool starts from the hints and follows the code flow, + * making automatic CFI adjustments when it sees pushes and pops, filling out + * the debuginfo as necessary. It will also warn if it sees any + * inconsistencies. + */ +.macro UNWIND_HINT cfa_reg=UNDWARF_REG_SP cfa_offset=0 type=UNDWARF_TYPE_CFA +#ifdef CONFIG_STACK_VALIDATION +.Lunwind_hint_ip_\@: + .pushsection .discard.unwind_hints + /* struct unwind_hint */ + .long .Lunwind_hint_ip_\@ - . + .short \cfa_offset + .byte \cfa_reg + .byte \type + .popsection +#endif +.endm + +.macro UNWIND_HINT_EMPTY + UNWIND_HINT cfa_reg=UNDWARF_REG_UNDEFINED +.endm + +.macro UNWIND_HINT_REGS base=rsp offset=0 indirect=0 extra=1 iret=0 + .if \base == rsp && \indirect + .set cfa_reg, UNDWARF_REG_SP_INDIRECT + .elseif \base == rsp + .set cfa_reg, UNDWARF_REG_SP + .elseif \base == rbp + .set cfa_reg, UNDWARF_REG_BP + .elseif \base == rdi + .set cfa_reg, UNDWARF_REG_DI + .elseif \base == rdx + .set cfa_reg, UNDWARF_REG_DX + .else + .error "UNWIND_HINT_REGS: bad base register" + .endif + + .set cfa_offset, \offset + + .if \iret + .set type, UNDWARF_TYPE_REGS_IRET + .elseif \extra == 0 + .set type, UNDWARF_TYPE_REGS_IRET + .set cfa_offset, \offset + (16*8) + .else + .set type, UNDWARF_TYPE_REGS + .endif + + UNWIND_HINT cfa_reg=cfa_reg cfa_offset=cfa_offset type=type +.endm + +.macro UNWIND_HINT_IRET_REGS base=rsp offset=0 + UNWIND_HINT_REGS base=\base offset=\offset iret=1 +.endm + +.macro UNWIND_HINT_FUNC cfa_offset=8 + UNWIND_HINT cfa_offset=\cfa_offset +.endm + +#else /* !__ASSEMBLY__ */ + +#define UNWIND_HINT(cfa_reg, cfa_offset, type) \ + "987: \n\t" \ + ".pushsection .discard.unwind_hints\n\t" \ + /* struct unwind_hint */ \ + ".long 987b - .\n\t" \ + ".short " __stringify(cfa_offset) "\n\t" \ + ".byte " __stringify(cfa_reg) "\n\t" \ + ".byte " __stringify(type) "\n\t" \ + ".popsection\n\t" + +#define UNWIND_HINT_SAVE UNWIND_HINT(0, 0, UNWIND_HINT_TYPE_SAVE) + +#define UNWIND_HINT_RESTORE UNWIND_HINT(0, 0, UNWIND_HINT_TYPE_RESTORE) + + +#endif /* __ASSEMBLY__ */ + + +#endif /* _ASM_X86_UNDWARF_H */ diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile index 0e2765e..7997d5c 100644 --- a/tools/objtool/Makefile +++ b/tools/objtool/Makefile @@ -52,6 +52,9 @@ $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN) diff -I'^#include' arch/x86/insn/inat.h ../../arch/x86/include/asm/inat.h >/dev/null && \ diff -I'^#include' arch/x86/insn/inat_types.h ../../arch/x86/include/asm/inat_types.h >/dev/null) \ || echo "warning: objtool: x86 instruction decoder differs from kernel" >&2 )) || true + @(test -d ../../kernel -a -d ../../tools -a -d ../objtool && (( \ + diff ../../arch/x86/include/asm/undwarf-types.h undwarf-types.h >/dev/null) \ + || echo "warning: objtool: undwarf-types.h differs from kernel" >&2 )) || true $(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@ diff --git a/tools/objtool/check.c b/tools/objtool/check.c index f76ac4c..6f83dad 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -873,6 +873,99 @@ static int add_switch_table_alts(struct objtool_file *file) return 0; } +static int read_unwind_hints(struct objtool_file *file) +{ + struct section *sec, *relasec; + struct rela *rela; + struct unwind_hint *hint; + struct instruction *insn; + struct cfi_reg *cfa; + int i; + + sec = find_section_by_name(file->elf, ".discard.unwind_hints"); + if (!sec) + return 0; + + relasec = sec->rela; + if (!relasec) { + WARN("missing .rela.discard.unwind_hints section"); + return -1; + } + + if (sec->len % sizeof(struct unwind_hint)) { + WARN("struct unwind_hint size mismatch"); + return -1; + } + + file->hints = true; + + for (i = 0; i < sec->len / sizeof(struct unwind_hint); i++) { + hint = (struct unwind_hint *)sec->data->d_buf + i; + + rela = find_rela_by_dest(sec, i * sizeof(*hint)); + if (!rela) { + WARN("can't find rela for unwind_hints[%d]", i); + return -1; + } + + insn = find_insn(file, rela->sym->sec, rela->addend); + if (!insn) { + WARN("can't find insn for unwind_hints[%d]", i); + return -1; + } + + 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; + continue; + } + + insn->hint = true; + + switch (hint->cfa_reg) { + case UNDWARF_REG_UNDEFINED: + cfa->base = CFI_UNDEFINED; + break; + case UNDWARF_REG_SP: + cfa->base = CFI_SP; + break; + case UNDWARF_REG_BP: + cfa->base = CFI_BP; + break; + case UNDWARF_REG_SP_INDIRECT: + cfa->base = CFI_SP_INDIRECT; + break; + case UNDWARF_REG_R10: + cfa->base = CFI_R10; + break; + case UNDWARF_REG_R13: + cfa->base = CFI_R13; + break; + case UNDWARF_REG_DI: + cfa->base = CFI_DI; + break; + case UNDWARF_REG_DX: + cfa->base = CFI_DX; + break; + default: + WARN_FUNC("unsupported unwind_hint cfa base reg %d", + insn->sec, insn->offset, hint->cfa_reg); + return -1; + } + + cfa->offset = hint->cfa_offset; + insn->state.type = hint->type; + } + + return 0; +} + static int decode_sections(struct objtool_file *file) { int ret; @@ -903,6 +996,10 @@ static int decode_sections(struct objtool_file *file) if (ret) return ret; + ret = read_unwind_hints(file); + if (ret) + return ret; + return 0; } @@ -1377,7 +1474,7 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, struct insn_state state) { struct alternative *alt; - struct instruction *insn; + struct instruction *insn, *next_insn; struct section *sec; struct symbol *func = NULL; int ret; @@ -1392,6 +1489,8 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, } while (1) { + next_insn = next_insn_same_sec(file, insn); + if (file->c_file && insn->func) { if (func && func != insn->func) { WARN("%s() falls through to next function %s()", @@ -1403,13 +1502,54 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, func = insn->func; if (insn->visited) { - if (!!insn_state_match(insn, &state)) + if (!insn->hint && !insn_state_match(insn, &state)) return 1; return 0; } - insn->state = state; + if (insn->hint) { + if (insn->restore) { + struct instruction *save_insn, *i; + + i = insn; + save_insn = NULL; + func_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) { + /* + * 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; + } + + insn->state = save_insn->state; + } + + state = insn->state; + + } else + insn->state = state; insn->visited = true; @@ -1484,7 +1624,7 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, return 0; case INSN_CONTEXT_SWITCH: - if (func) { + if (func && (!next_insn || !next_insn->hint)) { WARN_FUNC("unsupported instruction in callable function", sec, insn->offset); return 1; @@ -1504,7 +1644,7 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, if (insn->dead_end) return 0; - insn = next_insn_same_sec(file, insn); + insn = next_insn; if (!insn) { WARN("%s: unexpected end of section", sec->name); return 1; @@ -1514,6 +1654,27 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, return 0; } +static int validate_unwind_hints(struct objtool_file *file) +{ + struct instruction *insn; + int ret, warnings = 0; + struct insn_state state; + + if (!file->hints) + return 0; + + clear_insn_state(&state); + + for_each_insn(file, insn) { + if (insn->hint && !insn->visited) { + ret = validate_branch(file, insn, state); + warnings += ret; + } + } + + return warnings; +} + static bool is_kasan_insn(struct instruction *insn) { return (insn->type == INSN_CALL && @@ -1659,8 +1820,9 @@ int check(const char *_objname, bool _nofp, bool undwarf) hash_init(file.insn_hash); file.whitelist = find_section_by_name(file.elf, ".discard.func_stack_frame_non_standard"); file.rodata = find_section_by_name(file.elf, ".rodata"); - file.ignore_unreachables = false; file.c_file = find_section_by_name(file.elf, ".comment"); + file.ignore_unreachables = false; + file.hints = false; arch_initial_func_cfi_state(&initial_func_cfi); @@ -1677,6 +1839,11 @@ int check(const char *_objname, bool _nofp, bool undwarf) goto out; warnings += ret; + ret = validate_unwind_hints(&file); + if (ret < 0) + goto out; + warnings += ret; + if (!warnings) { ret = validate_reachable_instructions(&file); if (ret < 0) diff --git a/tools/objtool/check.h b/tools/objtool/check.h index 2fe0810..2d7cff4 100644 --- a/tools/objtool/check.h +++ b/tools/objtool/check.h @@ -43,7 +43,7 @@ struct instruction { unsigned int len; unsigned char type; unsigned long immediate; - bool alt_group, visited, dead_end, ignore; + bool alt_group, visited, dead_end, ignore, hint, save, restore; struct symbol *call_dest; struct instruction *jump_dest; struct list_head alts; @@ -58,7 +58,7 @@ struct objtool_file { struct list_head insn_list; DECLARE_HASHTABLE(insn_hash, 16); struct section *rodata, *whitelist; - bool ignore_unreachables, c_file; + bool ignore_unreachables, c_file, hints; }; int check(const char *objname, bool nofp, bool undwarf); diff --git a/tools/objtool/undwarf-types.h b/tools/objtool/undwarf-types.h index ef92a1d..4e5e283 100644 --- a/tools/objtool/undwarf-types.h +++ b/tools/objtool/undwarf-types.h @@ -57,11 +57,17 @@ * * UNDWARF_TYPE_REGS_IRET: Used in entry code to indicate that * cfa_reg+cfa_offset points to the iret return frame. + * + * The UNWIND_HINT macros are only used for the unwind_hint struct. They are + * not used for the undwarf struct due to size and complexity constraints. */ #define UNDWARF_TYPE_CFA 0 #define UNDWARF_TYPE_REGS 1 #define UNDWARF_TYPE_REGS_IRET 2 +#define UNWIND_HINT_TYPE_SAVE 3 +#define UNWIND_HINT_TYPE_RESTORE 4 +#ifndef __ASSEMBLY__ /* * This struct contains a simplified version of the DWARF Call Frame * Information standard. It contains only the necessary parts of the real @@ -78,4 +84,16 @@ struct undwarf { unsigned type:2; }; +/* + * This struct is used by asm and inline asm code to manually annotate the + * location of registers on the stack for the undwarf unwinder. + */ +struct unwind_hint { + unsigned int ip; + short cfa_offset; + unsigned char cfa_reg; + unsigned char type; +}; +#endif /* __ASSEMBLY__ */ + #endif /* _UNDWARF_TYPES_H */ -- 2.7.5