From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751402AbdFAFq1 (ORCPT ); Thu, 1 Jun 2017 01:46:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46100 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751184AbdFAFoY (ORCPT ); Thu, 1 Jun 2017 01:44:24 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com DA7BC80C0B 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 DA7BC80C0B 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: [RFC PATCH 03/10] objtool: stack validation 2.0 Date: Thu, 1 Jun 2017 00:44:09 -0500 Message-Id: In-Reply-To: References: X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 01 Jun 2017 05:44:24 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is a major rewrite of objtool. Instead of only tracking frame pointer changes, it now tracks all stack-related operations, including all register saves/restores. In addition to making stack validation more robust, this also paves the way for undwarf generation. Signed-off-by: Josh Poimboeuf --- tools/objtool/Documentation/stack-validation.txt | 153 +++--- tools/objtool/Makefile | 2 +- tools/objtool/arch.h | 64 ++- tools/objtool/arch/x86/decode.c | 400 +++++++++++++-- tools/objtool/cfi.h | 55 +++ tools/objtool/check.c | 595 ++++++++++++++++++----- tools/objtool/check.h | 18 +- tools/objtool/elf.c | 23 +- tools/objtool/elf.h | 3 +- tools/objtool/special.c | 6 +- 10 files changed, 1023 insertions(+), 296 deletions(-) create mode 100644 tools/objtool/cfi.h diff --git a/tools/objtool/Documentation/stack-validation.txt b/tools/objtool/Documentation/stack-validation.txt index 55a60d3..17c1195 100644 --- a/tools/objtool/Documentation/stack-validation.txt +++ b/tools/objtool/Documentation/stack-validation.txt @@ -127,28 +127,13 @@ b) 100% reliable stack traces for DWARF enabled kernels c) Higher live patching compatibility rate - (NOTE: This is not yet implemented) - - Currently with CONFIG_LIVEPATCH there's a basic live patching - framework which is safe for roughly 85-90% of "security" fixes. But - patches can't have complex features like function dependency or - prototype changes, or data structure changes. - - There's a strong need to support patches which have the more complex - features so that the patch compatibility rate for security fixes can - eventually approach something resembling 100%. To achieve that, a - "consistency model" is needed, which allows tasks to be safely - transitioned from an unpatched state to a patched state. - - One of the key requirements of the currently proposed livepatch - consistency model [*] is that it needs to walk the stack of each - sleeping task to determine if it can be transitioned to the patched - state. If objtool can ensure that stack traces are reliable, this - consistency model can be used and the live patching compatibility - rate can be improved significantly. - - [*] https://lkml.kernel.org/r/cover.1423499826.git.jpoimboe@redhat.com + Livepatch has an optional "consistency model", which is needed for + more complex patches. In order for the consistency model to work, + stack traces need to be reliable (or an unreliable condition needs to + be detectable). Objtool makes that possible. + For more details, see the livepatch documentation in the Linux kernel + source tree at Documentation/livepatch/livepatch.txt. Rules ----- @@ -201,80 +186,84 @@ To achieve the validation, objtool enforces the following rules: return normally. -Errors in .S files ------------------- +Objtool warnings +---------------- -If you're getting an error in a compiled .S file which you don't -understand, first make sure that the affected code follows the above -rules. +For asm files, if you're getting an error which doesn't make sense, +first make sure that the affected code follows the above rules. + +For C files, the common culprits are inline asm statements and calls to +"noreturn" functions. See below for more details. + +Another possible cause for errors in C code is if the Makefile removes +-fno-omit-frame-pointer or adds -fomit-frame-pointer to the gcc options. Here are some examples of common warnings reported by objtool, what they mean, and suggestions for how to fix them. -1. asm_file.o: warning: objtool: func()+0x128: call without frame pointer save/setup +1. file.o: warning: objtool: func()+0x128: call without frame pointer save/setup The func() function made a function call without first saving and/or - updating the frame pointer. - - If func() is indeed a callable function, add proper frame pointer - logic using the FRAME_BEGIN and FRAME_END macros. Otherwise, remove - its ELF function annotation by changing ENDPROC to END. + updating the frame pointer, and CONFIG_FRAME_POINTER is enabled. - If you're getting this error in a .c file, see the "Errors in .c - files" section. + If the error is for an asm file, and func() is indeed a callable + function, add proper frame pointer logic using the FRAME_BEGIN and + FRAME_END macros. Otherwise, if it's not a callable function, remove + its ELF function annotation by changing ENDPROC to END, and instead + use the manual CFI hint macros in asm/undwarf.h. + If it's a GCC-compiled .c file, the error may be because the function + uses an inline asm() statement which has a "call" instruction. An + asm() statement with a call instruction must declare the use of the + stack pointer in its output operand. For example, on x86_64: -2. asm_file.o: warning: objtool: .text+0x53: return instruction outside of a callable function - - A return instruction was detected, but objtool couldn't find a way - for a callable function to reach the instruction. + register void *__sp asm("rsp"); + asm volatile("call func" : "+r" (__sp)); - If the return instruction is inside (or reachable from) a callable - function, the function needs to be annotated with the ENTRY/ENDPROC - macros. + Otherwise the stack frame may not get created before the call. - If you _really_ need a return instruction outside of a function, and - are 100% sure that it won't affect stack traces, you can tell - objtool to ignore it. See the "Adding exceptions" section below. +2. file.o: warning: objtool: .text+0x53: unreachable instruction -3. asm_file.o: warning: objtool: func()+0x9: function has unreachable instruction + Objtool couldn't find a code path to reach the instruction. - The instruction lives inside of a callable function, but there's no - possible control flow path from the beginning of the function to the - instruction. + If the error is for an asm file, and the instruction is inside (or + reachable from) a callable function, the function should be annotated + with the ENTRY/ENDPROC macros (ENDPROC is the important one). + Otherwise, the code should probably be annotated with the CFI hint + macros in asm/undwarf.h so objtool and the unwinder can know the + stack state associated with the code. - If the instruction is actually needed, and it's actually in a - callable function, ensure that its function is properly annotated - with ENTRY/ENDPROC. + If you're 100% sure the code won't affect stack traces, or if you're + a just a bad person, you can tell objtool to ignore it. See the + "Adding exceptions" section below. If it's not actually in a callable function (e.g. kernel entry code), change ENDPROC to END. -4. asm_file.o: warning: objtool: func(): can't find starting instruction +4. file.o: warning: objtool: func(): can't find starting instruction or - asm_file.o: warning: objtool: func()+0x11dd: can't decode instruction + file.o: warning: objtool: func()+0x11dd: can't decode instruction - Did you put data in a text section? If so, that can confuse + Does the file have data in a text section? If so, that can confuse objtool's instruction decoder. Move the data to a more appropriate section like .data or .rodata. -5. asm_file.o: warning: objtool: func()+0x6: kernel entry/exit from callable instruction - - This is a kernel entry/exit instruction like sysenter or sysret. - Such instructions aren't allowed in a callable function, and are most - likely part of the kernel entry code. +5. file.o: warning: objtool: func()+0x6: unsupported instruction in callable function - If the instruction isn't actually in a callable function, change - ENDPROC to END. + This is a kernel entry/exit instruction like sysenter or iret. Such + instructions aren't allowed in a callable function, and are most + likely part of the kernel entry code. They should usually not have + the callable function annotation (ENDPROC) and should always be + annotated with the CFI hint macros in asm/undwarf.h. -6. asm_file.o: warning: objtool: func()+0x26: sibling call from callable instruction with changed frame pointer +6. file.o: warning: objtool: func()+0x26: sibling call from callable instruction with modified stack frame - This is a dynamic jump or a jump to an undefined symbol. Stacktool + This is a dynamic jump or a jump to an undefined symbol. Objtool assumed it's a sibling call and detected that the frame pointer wasn't first restored to its original state. @@ -282,24 +271,28 @@ they mean, and suggestions for how to fix them. destination code to the local file. If the instruction is not actually in a callable function (e.g. - kernel entry code), change ENDPROC to END. + kernel entry code), change ENDPROC to END and annotate manually with + the CFI hint macros in asm/undwarf.h. -7. asm_file: warning: objtool: func()+0x5c: frame pointer state mismatch +7. file: warning: objtool: func()+0x5c: stack state mismatch The instruction's frame pointer state is inconsistent, depending on which execution path was taken to reach the instruction. - Make sure the function pushes and sets up the frame pointer (for - x86_64, this means rbp) at the beginning of the function and pops it - at the end of the function. Also make sure that no other code in the - function touches the frame pointer. + Make sure that, when CONFIG_FRAME_POINTER is enabled, the function + pushes and sets up the frame pointer (for x86_64, this means rbp) at + the beginning of the function and pops it at the end of the function. + Also make sure that no other code in the function touches the frame + pointer. + Another possibility is that the code has some asm or inline asm which + does some unusual things to the stack or the frame pointer. In such + cases it's probably appropriate to use the CFI hint macros in + asm/undwarf.h. -Errors in .c files ------------------- -1. c_file.o: warning: objtool: funcA() falls through to next function funcB() +8. file.o: warning: objtool: funcA() falls through to next function funcB() This means that funcA() doesn't end with a return instruction or an unconditional jump, and that objtool has determined that the function @@ -318,22 +311,6 @@ Errors in .c files might be corrupt due to a gcc bug. For more details, see: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646 -2. If you're getting any other objtool error in a compiled .c file, it - may be because the file uses an asm() statement which has a "call" - instruction. An asm() statement with a call instruction must declare - the use of the stack pointer in its output operand. For example, on - x86_64: - - register void *__sp asm("rsp"); - asm volatile("call func" : "+r" (__sp)); - - Otherwise the stack frame may not get created before the call. - -3. Another possible cause for errors in C code is if the Makefile removes - -fno-omit-frame-pointer or adds -fomit-frame-pointer to the gcc options. - -Also see the above section for .S file errors for more information what -the individual error messages mean. 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/Makefile b/tools/objtool/Makefile index 27e019c..0e2765e 100644 --- a/tools/objtool/Makefile +++ b/tools/objtool/Makefile @@ -25,7 +25,7 @@ OBJTOOL_IN := $(OBJTOOL)-in.o all: $(OBJTOOL) INCLUDES := -I$(srctree)/tools/include -I$(srctree)/tools/arch/$(HOSTARCH)/include/uapi -CFLAGS += -Wall -Werror $(EXTRA_WARNINGS) -fomit-frame-pointer -O2 -g $(INCLUDES) +CFLAGS += -Wall -Werror $(EXTRA_WARNINGS) -Wno-switch-default -Wno-switch-enum -fomit-frame-pointer -O2 -g $(INCLUDES) LDFLAGS += -lelf $(LIBSUBCMD) # Allow old libelf to be used: diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h index a59e061..21aeca8 100644 --- a/tools/objtool/arch.h +++ b/tools/objtool/arch.h @@ -19,25 +19,63 @@ #define _ARCH_H #include +#include #include "elf.h" +#include "cfi.h" -#define INSN_FP_SAVE 1 -#define INSN_FP_SETUP 2 -#define INSN_FP_RESTORE 3 -#define INSN_JUMP_CONDITIONAL 4 -#define INSN_JUMP_UNCONDITIONAL 5 -#define INSN_JUMP_DYNAMIC 6 -#define INSN_CALL 7 -#define INSN_CALL_DYNAMIC 8 -#define INSN_RETURN 9 -#define INSN_CONTEXT_SWITCH 10 -#define INSN_NOP 11 -#define INSN_OTHER 12 +#define INSN_JUMP_CONDITIONAL 1 +#define INSN_JUMP_UNCONDITIONAL 2 +#define INSN_JUMP_DYNAMIC 3 +#define INSN_CALL 4 +#define INSN_CALL_DYNAMIC 5 +#define INSN_RETURN 6 +#define INSN_CONTEXT_SWITCH 7 +#define INSN_STACK 8 +#define INSN_NOP 9 +#define INSN_OTHER 10 #define INSN_LAST INSN_OTHER +enum op_dest_type { + OP_DEST_REG, + OP_DEST_REG_INDIRECT, + OP_DEST_MEM, + OP_DEST_PUSH, + OP_DEST_LEAVE, +}; + +struct op_dest { + enum op_dest_type type; + unsigned char reg; + int offset; +}; + +enum op_src_type { + OP_SRC_REG, + OP_SRC_REG_INDIRECT, + OP_SRC_CONST, + OP_SRC_POP, + OP_SRC_ADD, + OP_SRC_AND, +}; + +struct op_src { + enum op_src_type type; + unsigned char reg; + int offset; +}; + +struct stack_op { + struct op_dest dest; + struct op_src src; +}; + +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, unsigned char *type, - unsigned long *displacement); + unsigned long *immediate, struct stack_op *op); + +bool arch_callee_saved_reg(unsigned char reg); #endif /* _ARCH_H */ diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index 6ac99e3..a36c2eb 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -27,6 +27,17 @@ #include "../../arch.h" #include "../../warn.h" +static unsigned char op_to_cfi_reg[][2] = { + {CFI_AX, CFI_R8}, + {CFI_CX, CFI_R9}, + {CFI_DX, CFI_R10}, + {CFI_BX, CFI_R11}, + {CFI_SP, CFI_R12}, + {CFI_BP, CFI_R13}, + {CFI_SI, CFI_R14}, + {CFI_DI, CFI_R15}, +}; + static int is_x86_64(struct elf *elf) { switch (elf->ehdr.e_machine) { @@ -40,24 +51,50 @@ static int is_x86_64(struct elf *elf) } } +bool arch_callee_saved_reg(unsigned char reg) +{ + switch (reg) { + case CFI_BP: + case CFI_BX: + case CFI_R12: + case CFI_R13: + case CFI_R14: + case CFI_R15: + return true; + + case CFI_AX: + case CFI_CX: + case CFI_DX: + case CFI_SI: + case CFI_DI: + case CFI_SP: + case CFI_R8: + case CFI_R9: + case CFI_R10: + case CFI_R11: + case CFI_RA: + default: + return false; + } +} + int arch_decode_instruction(struct elf *elf, struct section *sec, unsigned long offset, unsigned int maxlen, unsigned int *len, unsigned char *type, - unsigned long *immediate) + unsigned long *immediate, struct stack_op *op) { struct insn insn; - int x86_64; - unsigned char op1, op2, ext; + int x86_64, sign; + unsigned char op1, op2, rex = 0, rex_b = 0, rex_r = 0, rex_w = 0, + modrm = 0, modrm_mod = 0, modrm_rm = 0, modrm_reg = 0, + sib = 0; x86_64 = is_x86_64(elf); if (x86_64 == -1) return -1; - insn_init(&insn, (void *)(sec->data + offset), maxlen, x86_64); + insn_init(&insn, sec->data->d_buf + offset, maxlen, x86_64); insn_get_length(&insn); - insn_get_opcode(&insn); - insn_get_modrm(&insn); - insn_get_immediate(&insn); if (!insn_complete(&insn)) { WARN_FUNC("can't decode instruction", sec, offset); @@ -73,67 +110,323 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, op1 = insn.opcode.bytes[0]; op2 = insn.opcode.bytes[1]; + if (insn.rex_prefix.nbytes) { + rex = insn.rex_prefix.bytes[0]; + rex_w = X86_REX_W(rex) >> 3; + rex_r = X86_REX_R(rex) >> 2; + rex_b = X86_REX_B(rex); + } + + if (insn.modrm.nbytes) { + modrm = insn.modrm.bytes[0]; + modrm_mod = X86_MODRM_MOD(modrm); + modrm_reg = X86_MODRM_REG(modrm); + modrm_rm = X86_MODRM_RM(modrm); + } + + if (insn.sib.nbytes) + sib = insn.sib.bytes[0]; + switch (op1) { - case 0x55: - if (!insn.rex_prefix.nbytes) - /* push rbp */ - *type = INSN_FP_SAVE; + + case 0x1: + case 0x29: + if (rex_w && !rex_b && modrm_mod == 3 && modrm_rm == 4) { + + /* 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_SRC_REG; + op->dest.reg = CFI_SP; + } + break; + + case 0x50 ... 0x57: + + /* 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; + break; - case 0x5d: - if (!insn.rex_prefix.nbytes) - /* pop rbp */ - *type = INSN_FP_RESTORE; + case 0x58 ... 0x5f: + + /* 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]; + + break; + + case 0x68: + case 0x6a: + /* push immediate */ + *type = INSN_STACK; + op->src.type = OP_SRC_CONST; + op->dest.type = OP_DEST_PUSH; break; case 0x70 ... 0x7f: *type = INSN_JUMP_CONDITIONAL; break; + case 0x81: + case 0x83: + if (rex != 0x48) + break; + + 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; + break; + } + + if (modrm == 0xc4) + sign = 1; + else if (modrm == 0xec) + sign = -1; + else + break; + + /* 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; + break; + case 0x89: - if (insn.rex_prefix.nbytes == 1 && - insn.rex_prefix.bytes[0] == 0x48 && - insn.modrm.nbytes && insn.modrm.bytes[0] == 0xe5) - /* mov rsp, rbp */ - *type = INSN_FP_SETUP; + if (rex == 0x48 && modrm == 0xe5) { + + /* mov %rsp, %rbp */ + *type = INSN_STACK; + op->src.type = OP_SRC_REG; + op->src.reg = CFI_SP; + op->dest.type = OP_DEST_REG; + op->dest.reg = CFI_BP; + break; + } + /* fallthrough */ + case 0x88: + if (!rex_b && + (modrm_mod == 1 || modrm_mod == 2) && modrm_rm == 5) { + + /* 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; + + } 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; + } + + break; + + case 0x8b: + if (rex_w && !rex_b && modrm_mod == 1 && modrm_rm == 5) { + + /* 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]; + + } 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]; + } + break; case 0x8d: - if (insn.rex_prefix.nbytes && - insn.rex_prefix.bytes[0] == 0x48 && - insn.modrm.nbytes && insn.modrm.bytes[0] == 0x2c && - insn.sib.nbytes && insn.sib.bytes[0] == 0x24) - /* lea %(rsp), %rbp */ - *type = INSN_FP_SETUP; + 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; + break; + } + + if (rex == 0x4c && modrm == 0x54 && sib == 0x24 && + insn.displacement.value == 8) { + + /* + * lea 0x8(%rsp), %r10 + * + * Here r10 is the "drap" pointer, used as a stack + * pointer helper when the stack gets realigned. + */ + *type = INSN_STACK; + op->src.type = OP_SRC_ADD; + op->src.reg = CFI_SP; + op->src.offset = 8; + op->dest.type = OP_DEST_REG; + op->dest.reg = CFI_R10; + break; + } + + if (rex == 0x4c && modrm == 0x6c && sib == 0x24 && + insn.displacement.value == 16) { + + /* + * lea 0x10(%rsp), %r13 + * + * Here r13 is the "drap" pointer, used as a stack + * pointer helper when the stack gets realigned. + */ + *type = INSN_STACK; + op->src.type = OP_SRC_ADD; + op->src.reg = CFI_SP; + op->src.offset = 16; + op->dest.type = OP_DEST_REG; + op->dest.reg = CFI_R13; + break; + } + + if (rex == 0x49 && modrm == 0x62 && + insn.displacement.value == -8) { + + /* + * lea -0x8(%r10), %rsp + * + * Restoring rsp back to its original value after a + * 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; + break; + } + + if (rex == 0x49 && modrm == 0x65 && + insn.displacement.value == -16) { + + /* + * lea -0x10(%r13), %rsp + * + * Restoring rsp back to its original value after a + * 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; + break; + } + + break; + + case 0x8f: + /* pop to mem */ + *type = INSN_STACK; + op->src.type = OP_SRC_POP; + op->dest.type = OP_DEST_MEM; break; case 0x90: *type = INSN_NOP; break; + case 0x9c: + /* pushf */ + *type = INSN_STACK; + op->src.type = OP_SRC_CONST; + op->dest.type = OP_DEST_PUSH; + break; + + case 0x9d: + /* popf */ + *type = INSN_STACK; + op->src.type = OP_SRC_POP; + op->dest.type = OP_DEST_MEM; + break; + case 0x0f: + if (op2 >= 0x80 && op2 <= 0x8f) *type = INSN_JUMP_CONDITIONAL; else if (op2 == 0x05 || op2 == 0x07 || op2 == 0x34 || op2 == 0x35) + /* sysenter, sysret */ *type = INSN_CONTEXT_SWITCH; + else if (op2 == 0x0d || op2 == 0x1f) + /* nopl/nopw */ *type = INSN_NOP; - else if (op2 == 0x01 && insn.modrm.nbytes && - (insn.modrm.bytes[0] == 0xc2 || - insn.modrm.bytes[0] == 0xd8)) - /* vmlaunch, vmrun */ - *type = INSN_CONTEXT_SWITCH; + + else if (op2 == 0xa0 || op2 == 0xa8) { + + /* push fs/gs */ + *type = INSN_STACK; + 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; + } break; - case 0xc9: /* leave */ - *type = INSN_FP_RESTORE; + case 0xc9: + /* + * leave + * + * equivalent to: + * mov bp, sp + * pop bp + */ + *type = INSN_STACK; + op->dest.type = OP_DEST_LEAVE; + break; - case 0xe3: /* jecxz/jrcxz */ + case 0xe3: + /* jecxz/jrcxz */ *type = INSN_JUMP_CONDITIONAL; break; @@ -158,14 +451,27 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, break; case 0xff: - ext = X86_MODRM_REG(insn.modrm.bytes[0]); - if (ext == 2 || ext == 3) + if (modrm_reg == 2 || modrm_reg == 3) + *type = INSN_CALL_DYNAMIC; - else if (ext == 4) + + else if (modrm_reg == 4) + *type = INSN_JUMP_DYNAMIC; - else if (ext == 5) /*jmpf */ + + else if (modrm_reg == 5) + + /* jmpf */ *type = INSN_CONTEXT_SWITCH; + else if (modrm_reg == 6) { + + /* push from mem */ + *type = INSN_STACK; + op->src.type = OP_SRC_CONST; + op->dest.type = OP_DEST_PUSH; + } + break; default: @@ -176,3 +482,21 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, return 0; } + +void arch_initial_func_cfi_state(struct cfi_state *state) +{ + int i; + + for (i = 0; i < CFI_NUM_REGS; i++) { + state->regs[i].base = CFI_UNDEFINED; + state->regs[i].offset = 0; + } + + /* initial CFA (call frame address) */ + state->cfa.base = CFI_SP; + state->cfa.offset = 8; + + /* initial RA (return address) */ + state->regs[16].base = CFI_CFA; + state->regs[16].offset = -8; +} diff --git a/tools/objtool/cfi.h b/tools/objtool/cfi.h new file mode 100644 index 0000000..443ab2c --- /dev/null +++ b/tools/objtool/cfi.h @@ -0,0 +1,55 @@ +/* + * Copyright (C) 2015-2017 Josh Poimboeuf + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see . + */ + +#ifndef _OBJTOOL_CFI_H +#define _OBJTOOL_CFI_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; +}; + +struct cfi_state { + struct cfi_reg cfa; + struct cfi_reg regs[CFI_NUM_REGS]; +}; + +#endif /* _OBJTOOL_CFI_H */ diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 91b97db..e924fd5 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -27,10 +27,6 @@ #include #include -#define STATE_FP_SAVED 0x1 -#define STATE_FP_SETUP 0x2 -#define STATE_FENTRY 0x4 - struct alternative { struct list_head list; struct instruction *insn; @@ -38,6 +34,7 @@ struct alternative { const char *objname; static bool nofp; +struct cfi_state initial_func_cfi; static struct instruction *find_insn(struct objtool_file *file, struct section *sec, unsigned long offset) @@ -56,7 +53,7 @@ static struct instruction *next_insn_same_sec(struct objtool_file *file, { struct instruction *next = list_next_entry(insn, list); - if (&next->list == &file->insn_list || next->sec != insn->sec) + if (!next || &next->list == &file->insn_list || next->sec != insn->sec) return NULL; return next; @@ -75,9 +72,6 @@ static bool gcov_enabled(struct objtool_file *file) return false; } -#define for_each_insn(file, insn) \ - list_for_each_entry(insn, &file->insn_list, list) - #define func_for_each_insn(file, func, insn) \ for (insn = find_insn(file, func->sec, func->offset); \ insn && &insn->list != &file->insn_list && \ @@ -94,6 +88,9 @@ static bool gcov_enabled(struct objtool_file *file) #define sec_for_each_insn_from(file, insn) \ for (; insn; insn = next_insn_same_sec(file, insn)) +#define sec_for_each_insn_continue(file, insn) \ + for (insn = next_insn_same_sec(file, insn); insn; \ + insn = next_insn_same_sec(file, insn)) /* * Check if the function has been manually whitelisted with the @@ -103,7 +100,6 @@ static bool gcov_enabled(struct objtool_file *file) static bool ignore_func(struct objtool_file *file, struct symbol *func) { struct rela *rela; - struct instruction *insn; /* check for STACK_FRAME_NON_STANDARD */ if (file->whitelist && file->whitelist->rela) @@ -116,11 +112,6 @@ static bool ignore_func(struct objtool_file *file, struct symbol *func) return true; } - /* check if it has a context switching instruction */ - func_for_each_insn(file, func, insn) - if (insn->type == INSN_CONTEXT_SWITCH) - return true; - return false; } @@ -233,6 +224,17 @@ static int 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) +{ + 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->drap_reg = CFI_UNDEFINED; +} + /* * Call the arch-specific instruction decoder for all the instructions and add * them to the global instruction list. @@ -252,16 +254,22 @@ static int decode_instructions(struct objtool_file *file) for (offset = 0; offset < sec->len; offset += insn->len) { insn = malloc(sizeof(*insn)); + if (!insn) { + WARN("malloc failed"); + return -1; + } memset(insn, 0, sizeof(*insn)); - INIT_LIST_HEAD(&insn->alts); + clear_insn_state(&insn->state); + insn->sec = sec; insn->offset = offset; ret = arch_decode_instruction(file->elf, sec, offset, sec->len - offset, &insn->len, &insn->type, - &insn->immediate); + &insn->immediate, + &insn->stack_op); if (ret) return ret; @@ -360,7 +368,7 @@ static void add_ignores(struct objtool_file *file) continue; func_for_each_insn(file, func, insn) - insn->visited = true; + insn->ignore = true; } } } @@ -380,8 +388,7 @@ static int add_jump_destinations(struct objtool_file *file) insn->type != INSN_JUMP_UNCONDITIONAL) continue; - /* skip ignores */ - if (insn->visited) + if (insn->ignore) continue; rela = find_rela_by_dest_range(insn->sec, insn->offset, @@ -518,10 +525,13 @@ static int handle_group_alt(struct objtool_file *file, } memset(fake_jump, 0, sizeof(*fake_jump)); INIT_LIST_HEAD(&fake_jump->alts); + clear_insn_state(&fake_jump->state); + fake_jump->sec = special_alt->new_sec; fake_jump->offset = -1; fake_jump->type = INSN_JUMP_UNCONDITIONAL; fake_jump->jump_dest = list_next_entry(last_orig_insn, list); + fake_jump->ignore = true; if (!special_alt->new_len) { *new_insn = fake_jump; @@ -900,21 +910,356 @@ static bool is_fentry_call(struct instruction *insn) return false; } -static bool has_modified_stack_frame(struct instruction *insn) +static bool has_modified_stack_frame(struct insn_state *state) { - return (insn->state & STATE_FP_SAVED) || - (insn->state & STATE_FP_SETUP); + 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) + 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; } -static bool has_valid_stack_frame(struct instruction *insn) +static bool has_valid_stack_frame(struct insn_state *state) { - return (insn->state & STATE_FP_SAVED) && - (insn->state & STATE_FP_SETUP); + if (state->cfa.base == CFI_BP && state->regs[CFI_BP].base == CFI_CFA && + state->regs[CFI_BP].offset == -16) + return true; + + if (state->drap && state->regs[CFI_BP].base == CFI_BP) + return true; + + return false; } -static unsigned int frame_state(unsigned long state) +static void save_reg(struct insn_state *state, unsigned char reg, int base, + int offset) { - return (state & (STATE_FP_SAVED | STATE_FP_SETUP)); + if ((arch_callee_saved_reg(reg) || + (state->drap && reg == state->drap_reg)) && + state->regs[reg].base == CFI_UNDEFINED) { + state->regs[reg].base = base; + state->regs[reg].offset = offset; + } +} + +static void restore_reg(struct insn_state *state, unsigned char reg) +{ + state->regs[reg].base = CFI_UNDEFINED; + state->regs[reg].offset = 0; +} + +/* + * A note about DRAP stack alignment: + * + * GCC has the concept of a DRAP register, which is used to help keep track of + * the stack pointer when aligning the stack. r10 or r13 is used as the DRAP + * register. The typical DRAP pattern is: + * + * ffffffff810079c5: 4c 8d 54 24 08 lea 0x8(%rsp),%r10 + * ffffffff810079ca: 48 83 e4 c0 and $0xffffffffffffffc0,%rsp + * ffffffff810079ce: 41 ff 72 f8 pushq -0x8(%r10) + * ffffffff810079d2: 55 push %rbp + * ffffffff810079d3: 48 89 e5 mov %rsp,%rbp + * ... + * ffffffff810079e5: 41 52 push %r10 + * ... + * ffffffff810079e8: 48 81 ec 00 02 00 00 sub $0x200,%rsp + * ... + * ffffffff81007b56: 48 81 c4 00 02 00 00 add $0x200,%rsp + * ... + * ffffffff81007b5e: 41 5a pop %r10 + * ... + * ... + * ffffffff81007b68: 5d pop %rbp + * ffffffff81007b69: 49 8d 62 f8 lea -0x8(%r10),%rsp + * ffffffff81007b6d: c3 retq + * + * When r13 is used as the DRAP pointer, the main difference from the above is + * that r13 gets pushed before the realignment. + */ +static int update_insn_state(struct instruction *insn, struct insn_state *state) +{ + struct stack_op *op = &insn->stack_op; + struct cfi_reg *cfa = &state->cfa; + struct cfi_reg *regs = state->regs; + + /* stack operations don't make sense with an undefined CFA */ + if (cfa->base == CFI_UNDEFINED) { + if (insn->func) { + WARN_FUNC("undefined stack state", insn->sec, insn->offset); + return -1; + } + return 0; + } + + switch (op->dest.type) { + + case OP_DEST_REG: + switch (op->src.type) { + + case OP_SRC_REG: + if (cfa->base == op->src.reg && cfa->base == CFI_SP && + op->dest.reg == CFI_BP && regs[CFI_BP].base == CFI_CFA && + regs[CFI_BP].offset == -cfa->offset) { + + /* mov %rsp, %rbp */ + cfa->base = op->dest.reg; + state->bp_scratch = false; + } else if (state->drap) { + + /* drap: mov %rsp, %rbp */ + regs[CFI_BP].base = CFI_BP; + regs[CFI_BP].offset = -state->stack_size; + state->bp_scratch = false; + } else if (!nofp) { + + WARN_FUNC("unknown stack-related register move", + insn->sec, insn->offset); + return -1; + } + + break; + + case OP_SRC_ADD: + if (op->dest.reg == CFI_SP && op->src.reg == CFI_SP) { + + /* add imm, %rsp */ + state->stack_size -= op->src.offset; + if (cfa->base == CFI_SP) + cfa->offset -= op->src.offset; + break; + } + + 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); + break; + } + + if (op->dest.reg != CFI_BP && op->src.reg == CFI_SP && + cfa->base == CFI_SP) { + + /* drap: lea disp(%rsp), %drap */ + state->drap_reg = op->dest.reg; + break; + } + + if (state->drap && op->dest.reg == CFI_SP && + op->src.reg == state->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; + break; + } + + if (op->dest.reg == state->cfa.base) { + WARN_FUNC("unsupported stack register modification", + insn->sec, insn->offset); + return -1; + } + + break; + + case OP_SRC_AND: + if (op->dest.reg != CFI_SP || state->drap_reg == CFI_UNDEFINED || + cfa->base != CFI_SP) { + WARN_FUNC("unsupported stack pointer realignment", + insn->sec, insn->offset); + return -1; + } + + /* and imm, %rsp */ + cfa->base = state->drap_reg; + cfa->offset = state->stack_size = 0; + state->drap = true; + break; + + case OP_SRC_POP: + if (!state->drap && op->dest.type == OP_DEST_REG && + op->dest.reg == cfa->base) { + + /* pop %rbp */ + cfa->base = CFI_SP; + } + + state->stack_size -= 8; + if (cfa->base == CFI_SP) + cfa->offset -= 8; + + if (regs[op->dest.reg].offset + state->stack_size == -8) { + + if (state->drap && cfa->base == CFI_BP_INDIRECT && + op->dest.type == OP_DEST_REG && + op->dest.reg == state->drap_reg) { + + /* drap: pop %drap */ + cfa->base = state->drap_reg; + cfa->offset = 0; + } + + /* the saved value is popped and gone */ + restore_reg(state, op->dest.reg); + } + + break; + + case OP_SRC_REG_INDIRECT: + 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); + } + + break; + + default: + WARN_FUNC("unknown stack-related instruction", + insn->sec, insn->offset); + return -1; + } + + break; + + case OP_DEST_PUSH: + state->stack_size += 8; + if (cfa->base == CFI_SP) + cfa->offset += 8; + + if (op->src.type != OP_SRC_REG) + break; + + /* fallthrough */ + + case OP_DEST_REG_INDIRECT: + if (state->drap) { + if (op->src.reg == cfa->base) { + + /* drap: push %drap */ + cfa->base = CFI_BP_INDIRECT; + cfa->offset = -state->stack_size; + + /* save drap so we know when to undefine it */ + save_reg(state, op->src.reg, CFI_CFA, -state->stack_size); + + } else if (op->src.reg == CFI_BP) { + + /* drap: push %rbp */ + state->stack_size = 0; + + } else if (regs[op->src.reg].base == CFI_UNDEFINED) { + + /* drap: push reg (other than rbp) */ + save_reg(state, op->src.reg, CFI_BP, -state->stack_size); + } + + } else if (op->dest.type == OP_DEST_PUSH) { + + /* push reg */ + save_reg(state, op->src.reg, CFI_CFA, -state->stack_size); + } 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); + } + + /* detect when asm code uses rbp as a scratch register */ + if (!nofp && insn->func && op->src.reg == CFI_BP && + cfa->base != CFI_BP) + state->bp_scratch = true; + + break; + + case OP_DEST_LEAVE: + if (cfa->base != CFI_BP) { + WARN_FUNC("leave instruction without stack frame pointer setup", + insn->sec, insn->offset); + return -1; + } + + /* leaveq */ + regs[CFI_BP].base = CFI_UNDEFINED; + regs[CFI_BP].offset = 0; + cfa->base = CFI_SP; + cfa->offset -= 8; + state->stack_size = cfa->offset; + + break; + + case OP_DEST_MEM: + if (op->src.type != OP_SRC_POP) { + WARN_FUNC("unknown stack-related memory operation", + insn->sec, insn->offset); + return -1; + } + + /* pop mem */ + state->stack_size -= 8; + if (cfa->base == CFI_SP) + cfa->offset -= 8; + + break; + + default: + WARN_FUNC("unknown stack-related instruction", + insn->sec, insn->offset); + return -1; + } + + return 0; +} + +static bool insn_state_match(struct instruction *insn, struct insn_state *state) +{ + struct insn_state *state1 = &insn->state, *state2 = state; + int i; + + if (memcmp(&state1->cfa, &state2->cfa, sizeof(state1->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); + + } else if (memcmp(&state1->regs, &state2->regs, sizeof(state1->regs))) { + for (i = 0; i < CFI_NUM_REGS; i++) { + if (!memcmp(&state1->regs[i], &state2->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); + break; + } + + } else if (state1->drap != state2->drap || + (state1->drap && state1->drap_reg != state2->drap_reg)) { + WARN_FUNC("stack state mismatch: drap1=%d(%d) drap2=%d(%d)", + insn->sec, insn->offset, + state1->drap, state1->drap_reg, + state2->drap, state2->drap_reg); + + } else + return true; + + return false; } /* @@ -923,24 +1268,22 @@ static unsigned int frame_state(unsigned long state) * each instruction and validate all the rules described in * tools/objtool/Documentation/stack-validation.txt. */ -static int validate_branch(struct objtool_file *file, - struct instruction *first, unsigned char first_state) +static int validate_branch(struct objtool_file *file, struct instruction *first, + struct insn_state state) { struct alternative *alt; struct instruction *insn; struct section *sec; struct symbol *func = NULL; - unsigned char state; int ret; insn = first; sec = insn->sec; - state = first_state; 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; + return -1; } while (1) { @@ -950,23 +1293,21 @@ static int validate_branch(struct objtool_file *file, func->name, insn->func->name); return 1; } - - func = insn->func; } + func = insn->func; + if (insn->visited) { - if (frame_state(insn->state) != frame_state(state)) { - WARN_FUNC("frame pointer state mismatch", - sec, insn->offset); + if (!!insn_state_match(insn, &state)) return 1; - } return 0; } - insn->visited = true; insn->state = state; + insn->visited = true; + list_for_each_entry(alt, &insn->alts, list) { ret = validate_branch(file, alt->insn, state); if (ret) @@ -975,50 +1316,24 @@ static int validate_branch(struct objtool_file *file, switch (insn->type) { - case INSN_FP_SAVE: - if (!nofp) { - if (state & STATE_FP_SAVED) { - WARN_FUNC("duplicate frame pointer save", - sec, insn->offset); - return 1; - } - state |= STATE_FP_SAVED; - } - break; - - case INSN_FP_SETUP: - if (!nofp) { - if (state & STATE_FP_SETUP) { - WARN_FUNC("duplicate frame pointer setup", - sec, insn->offset); - return 1; - } - state |= STATE_FP_SETUP; - } - break; - - case INSN_FP_RESTORE: - if (!nofp) { - if (has_valid_stack_frame(insn)) - state &= ~STATE_FP_SETUP; - - state &= ~STATE_FP_SAVED; - } - break; - case INSN_RETURN: - if (!nofp && has_modified_stack_frame(insn)) { - WARN_FUNC("return without frame pointer restore", + if (func && has_modified_stack_frame(&state)) { + WARN_FUNC("return with modified stack frame", sec, insn->offset); return 1; } + + if (state.bp_scratch) { + WARN("%s uses BP as a scratch register", + insn->func->name); + return 1; + } + return 0; case INSN_CALL: - if (is_fentry_call(insn)) { - state |= STATE_FENTRY; + if (is_fentry_call(insn)) break; - } ret = dead_end_function(file, insn->call_dest); if (ret == 1) @@ -1028,7 +1343,7 @@ static int validate_branch(struct objtool_file *file, /* fallthrough */ case INSN_CALL_DYNAMIC: - if (!nofp && !has_valid_stack_frame(insn)) { + if (!nofp && func && !has_valid_stack_frame(&state)) { WARN_FUNC("call without frame pointer save/setup", sec, insn->offset); return 1; @@ -1042,8 +1357,8 @@ static int validate_branch(struct objtool_file *file, state); if (ret) return 1; - } else if (has_modified_stack_frame(insn)) { - WARN_FUNC("sibling call from callable instruction with changed frame pointer", + } else if (func && has_modified_stack_frame(&state)) { + WARN_FUNC("sibling call from callable instruction with modified stack frame", sec, insn->offset); return 1; } /* else it's a sibling call */ @@ -1054,15 +1369,29 @@ static int validate_branch(struct objtool_file *file, break; case INSN_JUMP_DYNAMIC: - if (list_empty(&insn->alts) && - has_modified_stack_frame(insn)) { - WARN_FUNC("sibling call from callable instruction with changed frame pointer", + if (func && list_empty(&insn->alts) && + has_modified_stack_frame(&state)) { + WARN_FUNC("sibling call from callable instruction with modified stack frame", sec, insn->offset); return 1; } return 0; + case INSN_CONTEXT_SWITCH: + if (func) { + WARN_FUNC("unsupported instruction in callable function", + sec, insn->offset); + return 1; + } + return 0; + + case INSN_STACK: + if (update_insn_state(insn, &state)) + return -1; + + break; + default: break; } @@ -1093,12 +1422,18 @@ static bool is_ubsan_insn(struct instruction *insn) "__ubsan_handle_builtin_unreachable")); } -static bool ignore_unreachable_insn(struct symbol *func, - struct instruction *insn) +static bool ignore_unreachable_insn(struct instruction *insn) { int i; - if (insn->type == INSN_NOP) + if (insn->ignore || insn->type == INSN_NOP) + return true; + + /* + * Ignore any unused exceptions. This can happen when a whitelisted + * function has an exception table entry. + */ + if (!strcmp(insn->sec->name, ".fixup")) return true; /* @@ -1107,6 +1442,8 @@ static bool ignore_unreachable_insn(struct symbol *func, * * End the search at 5 instructions to avoid going into the weeds. */ + if (!insn->func) + return false; for (i = 0; i < 5; i++) { if (is_kasan_insn(insn) || is_ubsan_insn(insn)) @@ -1117,7 +1454,7 @@ static bool ignore_unreachable_insn(struct symbol *func, continue; } - if (insn->offset + insn->len >= func->offset + func->len) + if (insn->offset + insn->len >= insn->func->offset + insn->func->len) break; insn = list_next_entry(insn, list); } @@ -1130,73 +1467,58 @@ static int validate_functions(struct objtool_file *file) struct section *sec; struct symbol *func; struct instruction *insn; + 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(sec, &file->elf->sections, list) { list_for_each_entry(func, &sec->symbol_list, list) { if (func->type != STT_FUNC) continue; insn = find_insn(file, sec, func->offset); - if (!insn) + if (!insn || insn->ignore) continue; - ret = validate_branch(file, insn, 0); + ret = validate_branch(file, insn, state); warnings += ret; } } - list_for_each_entry(sec, &file->elf->sections, list) { - list_for_each_entry(func, &sec->symbol_list, list) { - if (func->type != STT_FUNC) - continue; - - func_for_each_insn(file, func, insn) { - if (insn->visited) - continue; - - insn->visited = true; - - if (file->ignore_unreachables || warnings || - ignore_unreachable_insn(func, insn)) - continue; - - /* - * gcov produces a lot of unreachable - * instructions. If we get an unreachable - * warning and the file has gcov enabled, just - * ignore it, and all other such warnings for - * the file. - */ - if (!file->ignore_unreachables && - gcov_enabled(file)) { - file->ignore_unreachables = true; - continue; - } - - WARN_FUNC("function has unreachable instruction", insn->sec, insn->offset); - warnings++; - } - } - } - return warnings; } -static int validate_uncallable_instructions(struct objtool_file *file) +static int validate_reachable_instructions(struct objtool_file *file) { struct instruction *insn; - int warnings = 0; + + if (file->ignore_unreachables) + return 0; for_each_insn(file, insn) { - if (!insn->visited && insn->type == INSN_RETURN) { - WARN_FUNC("return instruction outside of a callable function", - insn->sec, insn->offset); - warnings++; - } + if (insn->visited || ignore_unreachable_insn(insn)) + continue; + + /* + * gcov produces a lot of unreachable instructions. If we get + * an unreachable warning and the file has gcov enabled, just + * ignore it, and all other such warnings for the file. Do + * this here because this is an expensive function. + */ + if (gcov_enabled(file)) + return 0; + + WARN_FUNC("unreachable instruction", insn->sec, insn->offset); + return 1; } - return warnings; + return 0; } static void cleanup(struct objtool_file *file) @@ -1237,21 +1559,28 @@ int check(const char *_objname, bool _nofp) file.ignore_unreachables = false; file.c_file = find_section_by_name(file.elf, ".comment"); + arch_initial_func_cfi_state(&initial_func_cfi); + ret = decode_sections(&file); if (ret < 0) goto out; warnings += ret; - ret = validate_functions(&file); - if (ret < 0) + if (list_empty(&file.insn_list)) goto out; - warnings += ret; - ret = validate_uncallable_instructions(&file); + ret = validate_functions(&file); if (ret < 0) goto out; warnings += ret; + if (!warnings) { + ret = validate_reachable_instructions(&file); + if (ret < 0) + goto out; + warnings += ret; + } + out: cleanup(&file); diff --git a/tools/objtool/check.h b/tools/objtool/check.h index b0ac3ba..da85f5b 100644 --- a/tools/objtool/check.h +++ b/tools/objtool/check.h @@ -24,19 +24,30 @@ #include "arch.h" #include +struct insn_state { + struct cfi_reg cfa; + struct cfi_reg regs[CFI_NUM_REGS]; + int stack_size; + bool bp_scratch; + bool drap; + int drap_reg; +}; + struct instruction { struct list_head list; struct hlist_node hash; struct section *sec; unsigned long offset; - unsigned int len, state; + unsigned int len; unsigned char type; unsigned long immediate; - bool alt_group, visited, dead_end; + bool alt_group, visited, dead_end, ignore; struct symbol *call_dest; struct instruction *jump_dest; struct list_head alts; struct symbol *func; + struct stack_op stack_op; + struct insn_state state; }; struct objtool_file { @@ -49,4 +60,7 @@ struct objtool_file { int check(const char *objname, bool nofp); +#define for_each_insn(file, insn) \ + list_for_each_entry(insn, &file->insn_list, list) + #endif /* _CHECK_H */ diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index d897702..3fb0747 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -182,20 +182,19 @@ static int read_sections(struct elf *elf) return -1; } - sec->elf_data = elf_getdata(s, NULL); - if (!sec->elf_data) { + sec->data = elf_getdata(s, NULL); + if (!sec->data) { perror("elf_getdata"); return -1; } - if (sec->elf_data->d_off != 0 || - sec->elf_data->d_size != sec->sh.sh_size) { + if (sec->data->d_off != 0 || + sec->data->d_size != sec->sh.sh_size) { WARN("unexpected data attributes for %s", sec->name); return -1; } - sec->data = (unsigned long)sec->elf_data->d_buf; - sec->len = sec->elf_data->d_size; + sec->len = sec->data->d_size; } /* sanity check, one more call to elf_nextscn() should return NULL */ @@ -232,7 +231,7 @@ static int read_symbols(struct elf *elf) sym->idx = i; - if (!gelf_getsym(symtab->elf_data, i, &sym->sym)) { + if (!gelf_getsym(symtab->data, i, &sym->sym)) { perror("gelf_getsym"); goto err; } @@ -322,7 +321,7 @@ static int read_relas(struct elf *elf) } memset(rela, 0, sizeof(*rela)); - if (!gelf_getrela(sec->elf_data, i, &rela->rela)) { + if (!gelf_getrela(sec->data, i, &rela->rela)) { perror("gelf_getrela"); return -1; } @@ -362,12 +361,6 @@ struct elf *elf_open(const char *name) INIT_LIST_HEAD(&elf->sections); - elf->name = strdup(name); - if (!elf->name) { - perror("strdup"); - goto err; - } - elf->fd = open(name, O_RDONLY); if (elf->fd == -1) { perror("open"); @@ -421,8 +414,6 @@ void elf_close(struct elf *elf) list_del(&sec->list); free(sec); } - if (elf->name) - free(elf->name); if (elf->fd > 0) close(elf->fd); if (elf->elf) diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h index 731973e..75e44c7 100644 --- a/tools/objtool/elf.h +++ b/tools/objtool/elf.h @@ -37,10 +37,9 @@ struct section { DECLARE_HASHTABLE(rela_hash, 16); struct section *base, *rela; struct symbol *sym; - Elf_Data *elf_data; + Elf_Data *data; char *name; int idx; - unsigned long data; unsigned int len; }; diff --git a/tools/objtool/special.c b/tools/objtool/special.c index bff8abb..84f001d 100644 --- a/tools/objtool/special.c +++ b/tools/objtool/special.c @@ -91,16 +91,16 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry, alt->jump_or_nop = entry->jump_or_nop; if (alt->group) { - alt->orig_len = *(unsigned char *)(sec->data + offset + + alt->orig_len = *(unsigned char *)(sec->data->d_buf + offset + entry->orig_len); - alt->new_len = *(unsigned char *)(sec->data + offset + + alt->new_len = *(unsigned char *)(sec->data->d_buf + offset + entry->new_len); } if (entry->feature) { unsigned short feature; - feature = *(unsigned short *)(sec->data + offset + + feature = *(unsigned short *)(sec->data->d_buf + offset + entry->feature); /* -- 2.7.4