Skip to content

Commit

Permalink
objtool: Implement stack validation 2.0
Browse files Browse the repository at this point in the history 
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 <jpoimboe@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: live-patching@vger.kernel.org
Link: http://lkml.kernel.org/r/678bd94c0566c6129bcc376cddb259c4c5633004.1498659915.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
  • Loading branch information
Josh Poimboeuf authored and Ingo Molnar committed Jun 30, 2017
1 parent c207aee commit baa4146
Show file tree
Hide file tree
Showing 11 changed files with 1,130 additions and 320 deletions.
153 changes: 65 additions & 88 deletions tools/objtool/Documentation/stack-validation.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----
Expand Down Expand Up @@ -201,105 +186,113 @@ 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.

If it's not really a sibling call, you may need to move the
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
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion tools/objtool/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
64 changes: 51 additions & 13 deletions tools/objtool/arch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,63 @@
#define _ARCH_H

#include <stdbool.h>
#include <linux/list.h>
#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 */
Loading

0 comments on commit baa4146

Please sign in to comment.