Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1676800 > unrolled thread
| Started by | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| First post | 2017-06-28 17:20 +0200 |
| Last post | 2017-06-30 20:00 +0200 |
| Articles | 20 on this page of 35 — 5 participants |
Back to article view | Back to linux.kernel
[PATCH v2 0/8] x86: undwarf unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-28 17:20 +0200
[PATCH v2 4/8] objtool: add undwarf debuginfo generation Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-28 17:20 +0200
Re: [PATCH v2 4/8] objtool: add undwarf debuginfo generation Ingo Molnar <mingo@kernel.org> - 2017-06-29 09:20 +0200
Re: [PATCH v2 4/8] objtool: add undwarf debuginfo generation Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-29 15:50 +0200
Re: [PATCH v2 4/8] objtool: add undwarf debuginfo generation Ingo Molnar <mingo@kernel.org> - 2017-06-29 09:30 +0200
Re: [PATCH v2 4/8] objtool: add undwarf debuginfo generation Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-29 16:10 +0200
Re: [PATCH v2 4/8] objtool: add undwarf debuginfo generation Ingo Molnar <mingo@kernel.org> - 2017-06-29 16:50 +0200
Re: [PATCH v2 4/8] objtool: add undwarf debuginfo generation Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-29 17:10 +0200
[PATCH v2 7/8] x86/asm: add unwind hint annotations to sync_core() Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-28 17:20 +0200
[PATCH v2 2/8] objtool, x86: add several functions and files to the objtool whitelist Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-28 17:20 +0200
[tip:core/objtool] objtool, x86: Add several functions and files to the objtool whitelist tip-bot for Josh Poimboeuf <tipbot@zytor.com> - 2017-06-30 15:20 +0200
[PATCH v2 6/8] x86/entry: add unwind hint annotations Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-28 17:20 +0200
Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-29 20:00 +0200
Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations Andy Lutomirski <luto@kernel.org> - 2017-06-29 21:00 +0200
Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-29 21:10 +0200
Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations Andy Lutomirski <luto@kernel.org> - 2017-06-29 23:20 +0200
Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-29 23:50 +0200
Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations Andy Lutomirski <luto@amacapital.net> - 2017-06-30 01:00 +0200
Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-30 04:20 +0200
Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations Andy Lutomirski <luto@kernel.org> - 2017-06-30 07:10 +0200
Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations Andy Lutomirski <luto@kernel.org> - 2017-06-30 07:50 +0200
Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-30 15:20 +0200
Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations Andy Lutomirski <luto@kernel.org> - 2017-06-30 17:50 +0200
Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-30 18:00 +0200
Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations Andy Lutomirski <luto@kernel.org> - 2017-06-30 18:00 +0200
Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-30 18:20 +0200
[PATCH v2 1/8] objtool: move checking code to check.c Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-28 17:20 +0200
Re: [PATCH v2 0/8] x86: undwarf unwinder Ingo Molnar <mingo@kernel.org> - 2017-06-29 10:00 +0200
Re: [PATCH v2 0/8] x86: undwarf unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-29 16:20 +0200
Re: [PATCH v2 0/8] x86: undwarf unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-29 21:20 +0200
Re: [PATCH v2 3/8] objtool: stack validation 2.0 Ingo Molnar <mingo@kernel.org> - 2017-06-30 10:40 +0200
Re: [PATCH v2 3/8] objtool: stack validation 2.0 Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-30 15:30 +0200
Re: [PATCH v2 3/8] objtool: stack validation 2.0 Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-30 15:30 +0200
[PATCH] objtool: silence warnings for functions which use iret Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-30 16:10 +0200
[tip:core/objtool] objtool: Silence warnings for functions which use IRET tip-bot for Josh Poimboeuf <tipbot@zytor.com> - 2017-06-30 20:00 +0200
Page 1 of 2 [1] 2 Next page →
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-06-28 17:20 +0200 |
| Subject | [PATCH v2 0/8] x86: undwarf unwinder |
| Message-ID | <tXmV4-8uj-15@gated-at.bofh.it> |
v2:
- 2x performance improvement by using a fast lookup table and splitting
undwarf array into two parallel arrays (Andy L)
- reduce data size by ~1MB by getting rid of 'len' field
- sort and post-process data at boot time
- don't search vmlinux tables for module addresses (Peter Z)
- disable preemption to prevent module from getting unloaded while
reading its undwarf data (Peter Z)
- avoid unwinding a running task's stack (Jiri S)
- remove '__sp' constraint from inline asm (Jiri S)
- rename "CFI_*" -> "UNWIND_HINT_*" (Andy L)
- replace '999:' label with '.Lunwind_hint_ip_\@' (Andy L)
- entry code annotation fixes: extra=0 fix, symmetrical macro
annotations, ret_from_fork fix (Andy L)
- invalidate all object files when enabling/disabling
CONFIG_UNDWARF_UNWINDER
- pass ip-1 to undwarf_find() for call return addresses to fix stack
traces for sibling calls and noreturn calls at end of function
- docs: clarify benefits vs frame pointers (Ingo)
- docs: improve wording, add more info, add performance info from Mel G
and Jiri S, move to kernel docs dir
- objtool: several minor fixes (Jiri S)
- objtool: append file instead of rewriting it
- objtool: improve elf warnings
- objtool: fix handling of the GCC DRAP register for aligned stacks
- objtool: rewrite 'undwarf dump' command to be much faster and to work
on vmlinux
- objtool: rename undwarf.c -> undwarf_gen.c
-----
Create a new 'undwarf' unwinder, enabled by CONFIG_UNDWARF_UNWINDER, and
plug it into the x86 unwinder framework. Objtool is used to generate
the undwarf debuginfo. The undwarf debuginfo format is basically a
simplified version of DWARF CFI. More details below.
The unwinder works well in my testing. It unwinds through interrupts,
exceptions, and preemption, with and without frame pointers, across
aligned stacks and dynamically allocated stacks. If something goes
wrong during an oops, it successfully falls back to printing the '?'
entries just like the frame pointer unwinder.
I'm not tied to the 'undwarf' name, other naming ideas are welcome.
Some potential future improvements:
- properly annotate or fix whitelisted functions and files
- reduce the number of base CFA registers needed in entry code
- compress undwarf debuginfo to use less memory
- make it easier to disable CONFIG_FRAME_POINTER
- add reliability checks for livepatch
- runtime NMI stack reliability checker
This code can also be found at:
git://github.com/jpoimboe/linux undwarf-v2
Here's the contents of the undwarf.txt file which explains the 'why' in
more detail:
Undwarf unwinder debuginfo generation
=====================================
Overview
--------
The kernel CONFIG_UNDWARF_UNWINDER option enables objtool generation of
undwarf debuginfo, which is out-of-band data which is used by the
in-kernel undwarf unwinder. It's similar in concept to DWARF CFI
debuginfo which would be used by a DWARF unwinder. The difference is
that the format of the undwarf data is simpler than DWARF, which in turn
allows the unwinder to be simpler and faster.
Objtool generates the undwarf data by first doing compile-time stack
metadata validation (CONFIG_STACK_VALIDATION). After analyzing all the
code paths of a .o file, it determines information about the stack state
at each instruction address in the file and outputs that information to
the .undwarf and .undwarf_ip sections.
The undwarf sections are combined at link time and are sorted at boot
time. The unwinder uses the resulting data to correlate instruction
addresses with their stack states at run time.
Undwarf vs frame pointers
-------------------------
With frame pointers enabled, GCC adds instrumentation code to every
function in the kernel. The kernel's .text size increases by about
3.2%, resulting in a broad kernel-wide slowdown. Measurements by Mel
Gorman [1] have shown a slowdown of 5-10% for some workloads.
In contrast, the undwarf unwinder has no effect on text size or runtime
performance, because the debuginfo is out of band. So if you disable
frame pointers and enable undwarf, you get a nice performance
improvement across the board, and still have reliable stack traces.
Another benefit of undwarf compared to frame pointers is that it can
reliably unwind across interrupts and exceptions. Frame pointer based
unwinds can skip the caller of the interrupted function if it was a leaf
function or if the interrupt hit before the frame pointer was saved.
The main disadvantage of undwarf compared to frame pointers is that it
needs more memory to store the undwarf table: roughly 3-5MB depending on
the kernel config.
Undwarf vs DWARF
----------------
Undwarf debuginfo's advantage over DWARF itself is that it's much
simpler. It gets rid of the complex DWARF CFI state machine and also
gets rid of the tracking of unnecessary registers. This allows the
unwinder to be much simpler, meaning fewer bugs, which is especially
important for mission critical oops code.
The simpler debuginfo format also enables the unwinder to be much faster
than DWARF, which is important for perf and lockdep. In a basic
performance test by Jiri Slaby [2], the undwarf unwinder was about 20x
faster than an out-of-tree DWARF unwinder. (Note: that measurement was
taken before some performance tweaks were implemented, so the speedup
may be even higher.)
The undwarf format does have a few downsides compared to DWARF. The
undwarf table takes up ~2MB more memory than an DWARF .eh_frame table.
Another potential downside is that, as GCC evolves, it's conceivable
that the undwarf data may end up being *too* simple to describe the
state of the stack for certain optimizations. But IMO this is unlikely
because GCC saves the frame pointer for any unusual stack adjustments it
does, so I suspect we'll really only ever need to keep track of the
stack pointer and the frame pointer between call frames. But even if we
do end up having to track all the registers DWARF tracks, at least we
will still be able to control the format, e.g. no complex state
machines.
Undwarf debuginfo generation
----------------------------
The undwarf data is generated by objtool. With the existing
compile-time stack metadata validation feature, objtool already follows
all code paths, and so it already has all the information it needs to be
able to generate undwarf data from scratch. So it's an easy step to go
from stack validation to undwarf generation.
It should be possible to instead generate the undwarf data with a simple
tool which converts DWARF to undwarf. However, such a solution would be
incomplete due to the kernel's extensive use of asm, inline asm, and
special sections like exception tables.
That could be rectified by manually annotating those special code paths
using GNU assembler .cfi annotations in .S files, and homegrown
annotations for inline asm in .c files. But asm annotations were tried
in the past and were found to be unmaintainable. They were often
incorrect/incomplete and made the code harder to read and keep updated.
And based on looking at glibc code, annotating inline asm in .c files
might be even worse.
Objtool still needs a few annotations, but only in code which does
unusual things to the stack like entry code. And even then, far fewer
annotations are needed than what DWARF would need, so they're much more
maintainable than DWARF CFI annotations.
So the advantages of using objtool to generate undwarf are that it gives
more accurate debuginfo, with very few annotations. It also insulates
the kernel from toolchain bugs which can be very painful to deal with in
the kernel since we often have to workaround issues in older versions of
the toolchain for years.
The downside is that the unwinder now becomes dependent on objtool's
ability to reverse engineer GCC code paths. If GCC optimizations become
too complicated for objtool to follow, the undwarf generation might stop
working or become incomplete. (It's worth noting that livepatch already
has such a dependency on objtool's ability to follow GCC code paths.)
If newer versions of GCC come up with some optimizations which break
objtool, we may need to revisit the current implementation. Some
possible solutions would be asking GCC to make the optimizations more
palatable, or having objtool use DWARF as an additional input, or
creating a GCC plugin to assist objtool with its analysis. But for now,
objtool follows GCC code quite well.
Unwinder implementation details
-------------------------------
Objtool generates the undwarf data by integrating with the compile-time
stack metadata validation feature, which is described in detail in
tools/objtool/Documentation/stack-validation.txt. After analyzing all
the code paths of a .o file, it creates an array of undwarf structs, and
a parallel array of instruction addresses associated with those structs,
and writes them to the .undwarf and .undwarf_ip sections respectively.
The undwarf data is split into the two arrays for performance reasons,
to make the searchable part of the data (.undwarf_ip) more compact. The
arrays are sorted in parallel at boot time.
Performance is further improved by the use of a fast lookup table which
is created at runtime. The fast lookup table associates a given address
with a range of undwarf table indices, so that only a small subset of
the undwarf table needs to be searched.
[1] https://lkml.kernel.org/r/20170602104048.jkkzssljsompjdwy@suse.de
[2] https://lkml.kernel.org/r/d2ca5435-6386-29b8-db87-7f227c2b713a@suse.cz
Josh Poimboeuf (8):
objtool: move checking code to check.c
objtool, x86: add several functions and files to the objtool whitelist
objtool: stack validation 2.0
objtool: add undwarf debuginfo generation
objtool, x86: add facility for asm code to provide unwind hints
x86/entry: add unwind hint annotations
x86/asm: add unwind hint annotations to sync_core()
x86/unwind: add undwarf unwinder
Documentation/x86/undwarf.txt | 146 +++
arch/um/include/asm/unwind.h | 8 +
arch/x86/Kconfig | 1 +
arch/x86/Kconfig.debug | 25 +
arch/x86/crypto/Makefile | 2 +
arch/x86/crypto/sha1-mb/Makefile | 2 +
arch/x86/crypto/sha256-mb/Makefile | 2 +
arch/x86/entry/Makefile | 1 -
arch/x86/entry/calling.h | 6 +
arch/x86/entry/entry_64.S | 56 +-
arch/x86/include/asm/module.h | 9 +
arch/x86/include/asm/processor.h | 3 +
arch/x86/include/asm/undwarf-types.h | 99 ++
arch/x86/include/asm/undwarf.h | 103 ++
arch/x86/include/asm/unwind.h | 77 +-
arch/x86/kernel/Makefile | 9 +-
arch/x86/kernel/acpi/Makefile | 2 +
arch/x86/kernel/kprobes/opt.c | 9 +-
arch/x86/kernel/module.c | 12 +-
arch/x86/kernel/reboot.c | 2 +
arch/x86/kernel/setup.c | 3 +
arch/x86/kernel/unwind_frame.c | 39 +-
arch/x86/kernel/unwind_guess.c | 5 +
arch/x86/kernel/unwind_undwarf.c | 589 ++++++++++
arch/x86/kernel/vmlinux.lds.S | 2 +
arch/x86/kvm/svm.c | 2 +
arch/x86/kvm/vmx.c | 3 +
arch/x86/lib/msr-reg.S | 8 +-
arch/x86/net/Makefile | 2 +
arch/x86/platform/efi/Makefile | 1 +
arch/x86/power/Makefile | 2 +
arch/x86/xen/Makefile | 3 +
include/asm-generic/vmlinux.lds.h | 20 +-
kernel/kexec_core.c | 4 +-
lib/Kconfig.debug | 3 +
scripts/Makefile.build | 14 +-
tools/objtool/Build | 4 +
tools/objtool/Documentation/stack-validation.txt | 195 ++--
tools/objtool/Makefile | 5 +-
tools/objtool/arch.h | 64 +-
tools/objtool/arch/x86/decode.c | 400 ++++++-
tools/objtool/builtin-check.c | 1281 +---------------------
tools/objtool/builtin-undwarf.c | 70 ++
tools/objtool/builtin.h | 1 +
tools/objtool/cfi.h | 55 +
tools/objtool/{builtin-check.c => check.c} | 954 ++++++++++++----
tools/objtool/check.h | 79 ++
tools/objtool/elf.c | 265 ++++-
tools/objtool/elf.h | 21 +-
tools/objtool/objtool.c | 3 +-
tools/objtool/special.c | 6 +-
tools/objtool/undwarf-types.h | 99 ++
tools/objtool/{builtin.h => undwarf.h} | 18 +-
tools/objtool/undwarf_dump.c | 212 ++++
tools/objtool/undwarf_gen.c | 215 ++++
tools/objtool/warn.h | 10 +
56 files changed, 3466 insertions(+), 1765 deletions(-)
create mode 100644 Documentation/x86/undwarf.txt
create mode 100644 arch/um/include/asm/unwind.h
create mode 100644 arch/x86/include/asm/undwarf-types.h
create mode 100644 arch/x86/include/asm/undwarf.h
create mode 100644 arch/x86/kernel/unwind_undwarf.c
create mode 100644 tools/objtool/builtin-undwarf.c
create mode 100644 tools/objtool/cfi.h
copy tools/objtool/{builtin-check.c => check.c} (59%)
create mode 100644 tools/objtool/check.h
create mode 100644 tools/objtool/undwarf-types.h
copy tools/objtool/{builtin.h => undwarf.h} (67%)
create mode 100644 tools/objtool/undwarf_dump.c
create mode 100644 tools/objtool/undwarf_gen.c
--
2.7.5
[toc] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-06-28 17:20 +0200 |
| Subject | [PATCH v2 4/8] objtool: add undwarf debuginfo generation |
| Message-ID | <tXmV4-8uj-19@gated-at.bofh.it> |
| In reply to | #1676800 |
Now that objtool knows the states of all registers on the stack for each
instruction, it's straightforward to generate debuginfo for an unwinder
to use.
Instead of generating DWARF, generate a new format called undwarf, which
is more suitable for an in-kernel unwinder. See
tools/objtool/Documentation/undwarf.txt for a more detailed description
of this new debuginfo format and why it's preferable to DWARF.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
tools/objtool/Build | 3 +
tools/objtool/Documentation/stack-validation.txt | 46 ++---
tools/objtool/builtin-check.c | 2 +-
tools/objtool/builtin-undwarf.c | 70 ++++++++
tools/objtool/builtin.h | 1 +
tools/objtool/check.c | 59 ++++++-
tools/objtool/check.h | 15 +-
tools/objtool/elf.c | 212 ++++++++++++++++++++--
tools/objtool/elf.h | 15 +-
tools/objtool/objtool.c | 3 +-
tools/objtool/undwarf-types.h | 81 +++++++++
tools/objtool/{builtin.h => undwarf.h} | 18 +-
tools/objtool/undwarf_dump.c | 212 ++++++++++++++++++++++
tools/objtool/undwarf_gen.c | 215 +++++++++++++++++++++++
14 files changed, 892 insertions(+), 60 deletions(-)
create mode 100644 tools/objtool/builtin-undwarf.c
create mode 100644 tools/objtool/undwarf-types.h
copy tools/objtool/{builtin.h => undwarf.h} (67%)
create mode 100644 tools/objtool/undwarf_dump.c
create mode 100644 tools/objtool/undwarf_gen.c
diff --git a/tools/objtool/Build b/tools/objtool/Build
index 6f2e198..9fb3f2f 100644
--- a/tools/objtool/Build
+++ b/tools/objtool/Build
@@ -1,6 +1,9 @@
objtool-y += arch/$(SRCARCH)/
objtool-y += builtin-check.o
+objtool-y += builtin-undwarf.o
objtool-y += check.o
+objtool-y += undwarf_gen.o
+objtool-y += undwarf_dump.o
objtool-y += elf.o
objtool-y += special.o
objtool-y += objtool.o
diff --git a/tools/objtool/Documentation/stack-validation.txt b/tools/objtool/Documentation/stack-validation.txt
index 17c1195..14c0ded 100644
--- a/tools/objtool/Documentation/stack-validation.txt
+++ b/tools/objtool/Documentation/stack-validation.txt
@@ -11,9 +11,6 @@ analyzes every .o file and ensures the validity of its stack metadata.
It enforces a set of rules on asm code and C inline assembly code so
that stack traces can be reliable.
-Currently it only checks frame pointer usage, but there are plans to add
-CFI validation for C files and CFI generation for asm files.
-
For each function, it recursively follows all possible code paths and
validates the correct frame pointer state at each instruction.
@@ -23,6 +20,10 @@ alternative execution paths to a given instruction (or set of
instructions). Similarly, it knows how to follow switch statements, for
which gcc sometimes uses jump tables.
+(Objtool also has an 'undwarf generate' subcommand which generates
+debuginfo for the undwarf unwinder. See Documentation/x86/undwarf.txt
+in the kernel tree for more details.)
+
Why do we need stack metadata validation?
-----------------------------------------
@@ -93,37 +94,14 @@ a) More reliable stack traces for frame pointer enabled kernels
or at the very end of the function after the stack frame has been
destroyed. This is an inherent limitation of frame pointers.
-b) 100% reliable stack traces for DWARF enabled kernels
-
- (NOTE: This is not yet implemented)
-
- As an alternative to frame pointers, DWARF Call Frame Information
- (CFI) metadata can be used to walk the stack. Unlike frame pointers,
- CFI metadata is out of band. So it doesn't affect runtime
- performance and it can be reliable even when interrupts or exceptions
- are involved.
-
- For C code, gcc automatically generates DWARF CFI metadata. But for
- asm code, generating CFI is a tedious manual approach which requires
- manually placed .cfi assembler macros to be scattered throughout the
- code. It's clumsy and very easy to get wrong, and it makes the real
- code harder to read.
-
- Stacktool will improve this situation in several ways. For code
- which already has CFI annotations, it will validate them. For code
- which doesn't have CFI annotations, it will generate them. So an
- architecture can opt to strip out all the manual .cfi annotations
- from their asm code and have objtool generate them instead.
-
- We might also add a runtime stack validation debug option where we
- periodically walk the stack from schedule() and/or an NMI to ensure
- that the stack metadata is sane and that we reach the bottom of the
- stack.
-
- So the benefit of objtool here will be that external tooling should
- always show perfect stack traces. And the same will be true for
- kernel warning/oops traces if the architecture has a runtime DWARF
- unwinder.
+b) Out-of-band debuginfo generation (undwarf)
+
+ As an alternative to frame pointers, undwarf metadata can be used to
+ walk the stack. Unlike frame pointers, undwarf is out of band. So
+ it doesn't affect runtime performance and it can be reliable even
+ when interrupts or exceptions are involved.
+
+ For more details, see undwarf.txt.
c) Higher live patching compatibility rate
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 365c34e..eedf089 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -52,5 +52,5 @@ int cmd_check(int argc, const char **argv)
objname = argv[0];
- return check(objname, nofp);
+ return check(objname, nofp, false);
}
diff --git a/tools/objtool/builtin-undwarf.c b/tools/objtool/builtin-undwarf.c
new file mode 100644
index 0000000..900b1e5
--- /dev/null
+++ b/tools/objtool/builtin-undwarf.c
@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2017 Josh Poimboeuf <jpoimboe@redhat.com>
+ *
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * objtool undwarf:
+ *
+ * This command analyzes a .o file and adds an .undwarf section to it, which is
+ * used by the in-kernel "undwarf" unwinder.
+ *
+ * This command is a superset of "objtool check".
+ */
+
+#include <string.h>
+#include <subcmd/parse-options.h>
+#include "builtin.h"
+#include "check.h"
+
+
+static const char *undwarf_usage[] = {
+ "objtool undwarf generate [<options>] file.o",
+ "objtool undwarf dump file.o",
+ NULL,
+};
+
+extern const struct option check_options[];
+extern bool nofp;
+
+int cmd_undwarf(int argc, const char **argv)
+{
+ const char *objname;
+
+ argc--; argv++;
+ if (!strncmp(argv[0], "gen", 3)) {
+ argc = parse_options(argc, argv, check_options, undwarf_usage, 0);
+ if (argc != 1)
+ usage_with_options(undwarf_usage, check_options);
+
+ objname = argv[0];
+
+ return check(objname, nofp, true);
+
+ }
+
+ if (!strcmp(argv[0], "dump")) {
+ if (argc != 2)
+ usage_with_options(undwarf_usage, check_options);
+
+ objname = argv[1];
+
+ return undwarf_dump(objname);
+ }
+
+ usage_with_options(undwarf_usage, check_options);
+
+ return 0;
+}
diff --git a/tools/objtool/builtin.h b/tools/objtool/builtin.h
index 34d2ba7..0b9722f 100644
--- a/tools/objtool/builtin.h
+++ b/tools/objtool/builtin.h
@@ -18,5 +18,6 @@
#define _BUILTIN_H
extern int cmd_check(int argc, const char **argv);
+extern int cmd_undwarf(int argc, const char **argv);
#endif /* _BUILTIN_H */
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 2f80aa51..f76ac4c 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -36,8 +36,8 @@ 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)
+struct instruction *find_insn(struct objtool_file *file,
+ struct section *sec, unsigned long offset)
{
struct instruction *insn;
@@ -253,6 +253,11 @@ static int decode_instructions(struct objtool_file *file)
if (!(sec->sh.sh_flags & SHF_EXECINSTR))
continue;
+ if (strcmp(sec->name, ".altinstr_replacement") &&
+ strcmp(sec->name, ".altinstr_aux") &&
+ strncmp(sec->name, ".discard.", 9))
+ sec->text = true;
+
for (offset = 0; offset < sec->len; offset += insn->len) {
insn = malloc(sizeof(*insn));
if (!insn) {
@@ -941,6 +946,30 @@ 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)
+{
+ struct cfi_reg *cfa = &state->cfa;
+ struct stack_op *op = &insn->stack_op;
+
+ if (cfa->base != CFI_SP)
+ return 0;
+
+ /* push */
+ if (op->dest.type == OP_DEST_PUSH)
+ cfa->offset += 8;
+
+ /* pop */
+ if (op->src.type == OP_SRC_POP)
+ cfa->offset -= 8;
+
+ /* add immediate to sp */
+ if (op->dest.type == OP_DEST_REG && op->src.type == OP_SRC_ADD &&
+ op->dest.reg == CFI_SP && op->src.reg == CFI_SP)
+ cfa->offset -= op->src.offset;
+
+ return 0;
+}
+
static void save_reg(struct insn_state *state, unsigned char reg, int base,
int offset)
{
@@ -1026,6 +1055,10 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
return 0;
}
+ if (state->type == UNDWARF_TYPE_REGS ||
+ state->type == UNDWARF_TYPE_REGS_IRET)
+ return update_insn_state_regs(insn, state);
+
switch (op->dest.type) {
case OP_DEST_REG:
@@ -1317,6 +1350,10 @@ static bool insn_state_match(struct instruction *insn, struct insn_state *state)
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 (state1->drap != state2->drap ||
(state1->drap && state1->drap_reg != state2->drap_reg)) {
WARN_FUNC("stack state mismatch: drap1=%d(%d) drap2=%d(%d)",
@@ -1606,7 +1643,7 @@ static void cleanup(struct objtool_file *file)
elf_close(file->elf);
}
-int check(const char *_objname, bool _nofp)
+int check(const char *_objname, bool _nofp, bool undwarf)
{
struct objtool_file file;
int ret, warnings = 0;
@@ -1614,7 +1651,7 @@ int check(const char *_objname, bool _nofp)
objname = _objname;
nofp = _nofp;
- file.elf = elf_open(objname);
+ file.elf = elf_open(objname, undwarf ? O_RDWR : O_RDONLY);
if (!file.elf)
return 1;
@@ -1647,6 +1684,20 @@ int check(const char *_objname, bool _nofp)
warnings += ret;
}
+ if (undwarf) {
+ ret = create_undwarf(&file);
+ if (ret < 0)
+ goto out;
+
+ ret = create_undwarf_sections(&file);
+ if (ret < 0)
+ goto out;
+
+ ret = elf_write(file.elf);
+ if (ret < 0)
+ goto out;
+ }
+
out:
cleanup(&file);
diff --git a/tools/objtool/check.h b/tools/objtool/check.h
index da85f5b..2fe0810 100644
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -22,12 +22,14 @@
#include "elf.h"
#include "cfi.h"
#include "arch.h"
+#include "undwarf.h"
#include <linux/hashtable.h>
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;
int drap_reg;
@@ -48,6 +50,7 @@ struct instruction {
struct symbol *func;
struct stack_op stack_op;
struct insn_state state;
+ struct undwarf undwarf;
};
struct objtool_file {
@@ -58,9 +61,19 @@ struct objtool_file {
bool ignore_unreachables, c_file;
};
-int check(const char *objname, bool nofp);
+int check(const char *objname, bool nofp, bool undwarf);
+
+struct instruction *find_insn(struct objtool_file *file,
+ struct section *sec, unsigned long offset);
#define for_each_insn(file, insn) \
list_for_each_entry(insn, &file->insn_list, list)
+#define sec_for_each_insn(file, sec, insn) \
+ for (insn = find_insn(file, sec, 0); \
+ insn && &insn->list != &file->insn_list && \
+ insn->sec == sec; \
+ insn = list_next_entry(insn, list))
+
+
#endif /* _CHECK_H */
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 1a7e8aa..6e9f980 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -30,16 +30,6 @@
#include "elf.h"
#include "warn.h"
-/*
- * Fallback for systems without this "read, mmaping if possible" cmd.
- */
-#ifndef ELF_C_READ_MMAP
-#define ELF_C_READ_MMAP ELF_C_READ
-#endif
-
-#define WARN_ELF(format, ...) \
- WARN(format ": %s", ##__VA_ARGS__, elf_errmsg(-1))
-
struct section *find_section_by_name(struct elf *elf, const char *name)
{
struct section *sec;
@@ -349,9 +339,10 @@ static int read_relas(struct elf *elf)
return 0;
}
-struct elf *elf_open(const char *name)
+struct elf *elf_open(const char *name, int flags)
{
struct elf *elf;
+ Elf_Cmd cmd;
elf_version(EV_CURRENT);
@@ -364,13 +355,20 @@ struct elf *elf_open(const char *name)
INIT_LIST_HEAD(&elf->sections);
- elf->fd = open(name, O_RDONLY);
+ elf->fd = open(name, flags);
if (elf->fd == -1) {
perror("open");
goto err;
}
- elf->elf = elf_begin(elf->fd, ELF_C_READ_MMAP, NULL);
+ if ((flags & O_ACCMODE) == O_RDONLY)
+ cmd = ELF_C_READ_MMAP;
+ else if ((flags & O_ACCMODE) == O_RDWR)
+ cmd = ELF_C_RDWR;
+ else /* O_WRONLY */
+ cmd = ELF_C_WRITE;
+
+ elf->elf = elf_begin(elf->fd, cmd, NULL);
if (!elf->elf) {
WARN_ELF("elf_begin");
goto err;
@@ -397,6 +395,194 @@ struct elf *elf_open(const char *name)
return NULL;
}
+struct section *elf_create_section(struct elf *elf, const char *name,
+ size_t entsize, int nr)
+{
+ struct section *sec, *shstrtab;
+ size_t size = entsize * nr;
+ struct Elf_Scn *s;
+ Elf_Data *data;
+
+ sec = malloc(sizeof(*sec));
+ if (!sec) {
+ perror("malloc");
+ return NULL;
+ }
+ memset(sec, 0, sizeof(*sec));
+
+ INIT_LIST_HEAD(&sec->symbol_list);
+ INIT_LIST_HEAD(&sec->rela_list);
+ hash_init(sec->rela_hash);
+ hash_init(sec->symbol_hash);
+
+ list_add_tail(&sec->list, &elf->sections);
+
+ s = elf_newscn(elf->elf);
+ if (!s) {
+ WARN_ELF("elf_newscn");
+ return NULL;
+ }
+
+ sec->name = strdup(name);
+ if (!sec->name) {
+ perror("strdup");
+ return NULL;
+ }
+
+ sec->idx = elf_ndxscn(s);
+ sec->len = size;
+ sec->changed = true;
+
+ sec->data = elf_newdata(s);
+ if (!sec->data) {
+ WARN_ELF("elf_newdata");
+ return NULL;
+ }
+
+ sec->data->d_size = size;
+ sec->data->d_align = 1;
+
+ if (size) {
+ sec->data->d_buf = malloc(size);
+ if (!sec->data->d_buf) {
+ perror("malloc");
+ return NULL;
+ }
+ memset(sec->data->d_buf, 0, size);
+ }
+
+ if (!gelf_getshdr(s, &sec->sh)) {
+ WARN_ELF("gelf_getshdr");
+ return NULL;
+ }
+
+ sec->sh.sh_size = size;
+ sec->sh.sh_entsize = entsize;
+ sec->sh.sh_type = SHT_PROGBITS;
+ sec->sh.sh_addralign = 1;
+ sec->sh.sh_flags = SHF_ALLOC;
+
+
+ /* Add section name to .shstrtab */
+ shstrtab = find_section_by_name(elf, ".shstrtab");
+ if (!shstrtab) {
+ WARN("can't find .shstrtab section");
+ return NULL;
+ }
+
+ s = elf_getscn(elf->elf, shstrtab->idx);
+ if (!s) {
+ WARN_ELF("elf_getscn");
+ return NULL;
+ }
+
+ data = elf_newdata(s);
+ if (!data) {
+ WARN_ELF("elf_newdata");
+ return NULL;
+ }
+
+ data->d_buf = sec->name;
+ data->d_size = strlen(name) + 1;
+ data->d_align = 1;
+
+ sec->sh.sh_name = shstrtab->len;
+
+ shstrtab->len += strlen(name) + 1;
+ shstrtab->changed = true;
+
+ return sec;
+}
+
+struct section *elf_create_rela_section(struct elf *elf, struct section *base)
+{
+ char *relaname;
+ struct section *sec;
+
+ relaname = malloc(strlen(base->name) + strlen(".rela") + 1);
+ if (!relaname) {
+ perror("malloc");
+ return NULL;
+ }
+ strcpy(relaname, ".rela");
+ strcat(relaname, base->name);
+
+ sec = elf_create_section(elf, relaname, sizeof(GElf_Rela), 0);
+ if (!sec)
+ return NULL;
+
+ base->rela = sec;
+ sec->base = base;
+
+ sec->sh.sh_type = SHT_RELA;
+ sec->sh.sh_addralign = 8;
+ sec->sh.sh_link = find_section_by_name(elf, ".symtab")->idx;
+ sec->sh.sh_info = base->idx;
+ sec->sh.sh_flags = SHF_INFO_LINK;
+
+ return sec;
+}
+
+int elf_rebuild_rela_section(struct section *sec)
+{
+ struct rela *rela;
+ int nr, idx = 0, size;
+ GElf_Rela *relas;
+
+ nr = 0;
+ list_for_each_entry(rela, &sec->rela_list, list)
+ nr++;
+
+ size = nr * sizeof(*relas);
+ relas = malloc(size);
+ if (!relas) {
+ perror("malloc");
+ return -1;
+ }
+
+ sec->data->d_buf = relas;
+ sec->data->d_size = size;
+
+ sec->sh.sh_size = size;
+
+ idx = 0;
+ list_for_each_entry(rela, &sec->rela_list, list) {
+ relas[idx].r_offset = rela->offset;
+ relas[idx].r_addend = rela->addend;
+ relas[idx].r_info = GELF_R_INFO(rela->sym->idx, rela->type);
+ idx++;
+ }
+
+ return 0;
+}
+
+int elf_write(struct elf *elf)
+{
+ struct section *sec;
+ Elf_Scn *s;
+
+ list_for_each_entry(sec, &elf->sections, list) {
+ if (sec->changed) {
+ s = elf_getscn(elf->elf, sec->idx);
+ if (!s) {
+ WARN_ELF("elf_getscn");
+ return -1;
+ }
+ if (!gelf_update_shdr (s, &sec->sh)) {
+ WARN_ELF("gelf_update_shdr");
+ return -1;
+ }
+ }
+ }
+
+ if (elf_update(elf->elf, ELF_C_WRITE) < 0) {
+ WARN_ELF("elf_update");
+ return -1;
+ }
+
+ return 0;
+}
+
void elf_close(struct elf *elf)
{
struct section *sec, *tmpsec;
diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
index 343968b..d86e2ff1 100644
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -28,6 +28,13 @@
# define elf_getshdrstrndx elf_getshstrndx
#endif
+/*
+ * Fallback for systems without this "read, mmaping if possible" cmd.
+ */
+#ifndef ELF_C_READ_MMAP
+#define ELF_C_READ_MMAP ELF_C_READ
+#endif
+
struct section {
struct list_head list;
GElf_Shdr sh;
@@ -41,6 +48,7 @@ struct section {
char *name;
int idx;
unsigned int len;
+ bool changed, text;
};
struct symbol {
@@ -75,7 +83,7 @@ struct elf {
};
-struct elf *elf_open(const char *name);
+struct elf *elf_open(const char *name, int flags);
struct section *find_section_by_name(struct elf *elf, const char *name);
struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset);
struct symbol *find_symbol_containing(struct section *sec, unsigned long offset);
@@ -83,6 +91,11 @@ struct rela *find_rela_by_dest(struct section *sec, unsigned long offset);
struct rela *find_rela_by_dest_range(struct section *sec, unsigned long offset,
unsigned int len);
struct symbol *find_containing_func(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);
#define for_each_sec(file, sec) \
diff --git a/tools/objtool/objtool.c b/tools/objtool/objtool.c
index ecc5b1b..b2051d1 100644
--- a/tools/objtool/objtool.c
+++ b/tools/objtool/objtool.c
@@ -42,10 +42,11 @@ struct cmd_struct {
};
static const char objtool_usage_string[] =
- "objtool [OPTIONS] COMMAND [ARGS]";
+ "objtool COMMAND [ARGS]";
static struct cmd_struct objtool_cmds[] = {
{"check", cmd_check, "Perform stack metadata validation on an object file" },
+ {"undwarf", cmd_undwarf, "Generate in-place undwarf metadata for an object file" },
};
bool help;
diff --git a/tools/objtool/undwarf-types.h b/tools/objtool/undwarf-types.h
new file mode 100644
index 0000000..ef92a1d
--- /dev/null
+++ b/tools/objtool/undwarf-types.h
@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2017 Josh Poimboeuf <jpoimboe@redhat.com>
+ *
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _UNDWARF_TYPES_H
+#define _UNDWARF_TYPES_H
+
+/*
+ * The UNDWARF_REG_* registers are base registers which are used to find other
+ * registers on the stack.
+ *
+ * The CFA (call frame address) is the value of the stack pointer on the
+ * previous frame, i.e. the caller's SP before it called the callee.
+ *
+ * The CFA is usually based on SP, unless a frame pointer has been saved, in
+ * which case it's based on BP.
+ *
+ * BP is usually either based on CFA or is undefined (meaning its value didn't
+ * change for the current frame).
+ *
+ * So the CFA base is usually either SP or BP, and the FP base is usually either
+ * CFA or undefined. The rest of the base registers are needed for special
+ * cases like entry code and gcc aligned stacks.
+ */
+#define UNDWARF_REG_UNDEFINED 0
+#define UNDWARF_REG_CFA 1
+#define UNDWARF_REG_DX 2
+#define UNDWARF_REG_DI 3
+#define UNDWARF_REG_BP 4
+#define UNDWARF_REG_SP 5
+#define UNDWARF_REG_R10 6
+#define UNDWARF_REG_R13 7
+#define UNDWARF_REG_BP_INDIRECT 8
+#define UNDWARF_REG_SP_INDIRECT 9
+#define UNDWARF_REG_MAX 15
+
+/*
+ * UNDWARF_TYPE_CFA: Indicates that cfa_reg+cfa_offset points to the caller's
+ * stack pointer (aka the CFA in DWARF terms). Used for all callable
+ * functions, i.e. all C code and all callable asm functions.
+ *
+ * UNDWARF_TYPE_REGS: Used in entry code to indicate that cfa_reg+cfa_offset
+ * points to a fully populated pt_regs from a syscall, interrupt, or exception.
+ *
+ * UNDWARF_TYPE_REGS_IRET: Used in entry code to indicate that
+ * cfa_reg+cfa_offset points to the iret return frame.
+ */
+#define UNDWARF_TYPE_CFA 0
+#define UNDWARF_TYPE_REGS 1
+#define UNDWARF_TYPE_REGS_IRET 2
+
+/*
+ * This struct contains a simplified version of the DWARF Call Frame
+ * Information standard. It contains only the necessary parts of the real
+ * DWARF, simplified for ease of access by the in-kernel unwinder. It tells
+ * the unwinder how to find the previous SP and BP (and sometimes entry regs)
+ * on the stack for a given code address (IP). Each instance of the struct
+ * corresponds to one or more code locations.
+ */
+struct undwarf {
+ short cfa_offset;
+ short bp_offset;
+ unsigned cfa_reg:4;
+ unsigned bp_reg:4;
+ unsigned type:2;
+};
+
+#endif /* _UNDWARF_TYPES_H */
diff --git a/tools/objtool/builtin.h b/tools/objtool/undwarf.h
similarity index 67%
copy from tools/objtool/builtin.h
copy to tools/objtool/undwarf.h
index 34d2ba7..c9f5116 100644
--- a/tools/objtool/builtin.h
+++ b/tools/objtool/undwarf.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2015 Josh Poimboeuf <jpoimboe@redhat.com>
+ * Copyright (C) 2017 Josh Poimboeuf <jpoimboe@redhat.com>
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@@ -14,9 +14,17 @@
* You should have received a copy of the GNU General Public License
* along with this program; if not, see <http://www.gnu.org/licenses/>.
*/
-#ifndef _BUILTIN_H
-#define _BUILTIN_H
-extern int cmd_check(int argc, const char **argv);
+#ifndef _UNDWARF_H
+#define _UNDWARF_H
-#endif /* _BUILTIN_H */
+#include "undwarf-types.h"
+
+struct objtool_file;
+
+int create_undwarf(struct objtool_file *file);
+int create_undwarf_sections(struct objtool_file *file);
+
+int undwarf_dump(const char *objname);
+
+#endif /* _UNDWARF_H */
diff --git a/tools/objtool/undwarf_dump.c b/tools/objtool/undwarf_dump.c
new file mode 100644
index 0000000..7bab393
--- /dev/null
+++ b/tools/objtool/undwarf_dump.c
@@ -0,0 +1,212 @@
+/*
+ * Copyright (C) 2017 Josh Poimboeuf <jpoimboe@redhat.com>
+ *
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+#include <unistd.h>
+#include "undwarf.h"
+#include "warn.h"
+
+static const char *reg_name(unsigned int reg)
+{
+ switch (reg) {
+ case UNDWARF_REG_CFA:
+ return "cfa";
+ case UNDWARF_REG_DX:
+ return "dx";
+ case UNDWARF_REG_DI:
+ return "di";
+ case UNDWARF_REG_BP:
+ return "bp";
+ case UNDWARF_REG_SP:
+ return "sp";
+ case UNDWARF_REG_R10:
+ return "r10";
+ case UNDWARF_REG_R13:
+ return "r13";
+ case UNDWARF_REG_BP_INDIRECT:
+ return "bp(ind)";
+ case UNDWARF_REG_SP_INDIRECT:
+ return "sp(ind)";
+ default:
+ return "?";
+ }
+}
+
+static const char *undwarf_type_name(unsigned int type)
+{
+ switch (type) {
+ case UNDWARF_TYPE_CFA:
+ return "cfa";
+ case UNDWARF_TYPE_REGS:
+ return "regs";
+ case UNDWARF_TYPE_REGS_IRET:
+ return "iret";
+ default:
+ return "?";
+ }
+}
+
+static void print_reg(unsigned int reg, int offset)
+{
+ if (reg == UNDWARF_REG_BP_INDIRECT)
+ printf("(bp%+d)", offset);
+ else if (reg == UNDWARF_REG_SP_INDIRECT)
+ printf("(sp%+d)", offset);
+ else if (reg == UNDWARF_REG_UNDEFINED)
+ printf("(und)");
+ else
+ printf("%s%+d", reg_name(reg), offset);
+}
+
+int undwarf_dump(const char *_objname)
+{
+ int fd, nr_entries, i, *undwarf_ip = NULL, undwarf_size = 0;
+ struct undwarf *undwarf = NULL;
+ char *name;
+ unsigned long nr_sections, undwarf_ip_addr = 0;
+ size_t shstrtab_idx;
+ Elf *elf;
+ Elf_Scn *scn;
+ GElf_Shdr sh;
+ GElf_Rela rela;
+ GElf_Sym sym;
+ Elf_Data *data, *symtab = NULL, *rela_undwarf_ip = NULL;
+
+
+ objname = _objname;
+
+ elf_version(EV_CURRENT);
+
+ fd = open(objname, O_RDONLY);
+ if (fd == -1) {
+ perror("open");
+ return -1;
+ }
+
+ elf = elf_begin(fd, ELF_C_READ_MMAP, NULL);
+ if (!elf) {
+ WARN_ELF("elf_begin");
+ return -1;
+ }
+
+ if (elf_getshdrnum(elf, &nr_sections)) {
+ WARN_ELF("elf_getshdrnum");
+ return -1;
+ }
+
+ if (elf_getshdrstrndx(elf, &shstrtab_idx)) {
+ WARN_ELF("elf_getshdrstrndx");
+ return -1;
+ }
+
+ for (i = 0; i < nr_sections; i++) {
+ scn = elf_getscn(elf, i);
+ if (!scn) {
+ WARN_ELF("elf_getscn");
+ return -1;
+ }
+
+ if (!gelf_getshdr(scn, &sh)) {
+ WARN_ELF("gelf_getshdr");
+ return -1;
+ }
+
+ name = elf_strptr(elf, shstrtab_idx, sh.sh_name);
+ if (!name) {
+ WARN_ELF("elf_strptr");
+ return -1;
+ }
+
+ data = elf_getdata(scn, NULL);
+ if (!data) {
+ WARN_ELF("elf_getdata");
+ return -1;
+ }
+
+ if (!strcmp(name, ".symtab")) {
+ symtab = data;
+ } else if (!strcmp(name, ".undwarf")) {
+ undwarf = data->d_buf;
+ undwarf_size = sh.sh_size;
+ } else if (!strcmp(name, ".undwarf_ip")) {
+ undwarf_ip = data->d_buf;
+ undwarf_ip_addr = sh.sh_addr;
+ } else if (!strcmp(name, ".rela.undwarf_ip")) {
+ rela_undwarf_ip = data;
+ }
+ }
+
+ if (!symtab || !undwarf || !undwarf_ip)
+ return 0;
+
+ if (undwarf_size % sizeof(*undwarf) != 0) {
+ WARN("bad .undwarf section size");
+ return -1;
+ }
+
+ nr_entries = undwarf_size / sizeof(*undwarf);
+ for (i = 0; i < nr_entries; i++) {
+ if (rela_undwarf_ip) {
+ if (!gelf_getrela(rela_undwarf_ip, i, &rela)) {
+ WARN_ELF("gelf_getrela");
+ return -1;
+ }
+
+ if (!gelf_getsym(symtab, GELF_R_SYM(rela.r_info), &sym)) {
+ WARN_ELF("gelf_getsym");
+ return -1;
+ }
+
+ scn = elf_getscn(elf, sym.st_shndx);
+ if (!scn) {
+ WARN_ELF("elf_getscn");
+ return -1;
+ }
+
+ if (!gelf_getshdr(scn, &sh)) {
+ WARN_ELF("gelf_getshdr");
+ return -1;
+ }
+
+ name = elf_strptr(elf, shstrtab_idx, sh.sh_name);
+ if (!name || !*name) {
+ WARN_ELF("elf_strptr");
+ return -1;
+ }
+
+ printf("%s+%lx:", name, rela.r_addend);
+
+ } else {
+ printf("%lx:", undwarf_ip_addr + (i * sizeof(int)) + undwarf_ip[i]);
+ }
+
+
+ printf(" cfa:");
+
+ print_reg(undwarf[i].cfa_reg, undwarf[i].cfa_offset);
+
+ printf(" bp:");
+
+ print_reg(undwarf[i].bp_reg, undwarf[i].bp_offset);
+
+ printf(" type:%s\n", undwarf_type_name(undwarf[i].type));
+ }
+
+ elf_end(elf);
+ close(fd);
+
+ return 0;
+}
diff --git a/tools/objtool/undwarf_gen.c b/tools/objtool/undwarf_gen.c
new file mode 100644
index 0000000..03021d8
--- /dev/null
+++ b/tools/objtool/undwarf_gen.c
@@ -0,0 +1,215 @@
+/*
+ * Copyright (C) 2017 Josh Poimboeuf <jpoimboe@redhat.com>
+ *
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+#include <stdlib.h>
+#include <string.h>
+
+#include "undwarf.h"
+#include "check.h"
+#include "warn.h"
+
+int create_undwarf(struct objtool_file *file)
+{
+ struct instruction *insn;
+
+ for_each_insn(file, insn) {
+ struct undwarf *undwarf = &insn->undwarf;
+ struct cfi_reg *cfa = &insn->state.cfa;
+ struct cfi_reg *bp = &insn->state.regs[CFI_BP];
+
+ if (cfa->base == CFI_UNDEFINED) {
+ undwarf->cfa_reg = UNDWARF_REG_UNDEFINED;
+ continue;
+ }
+
+ switch (cfa->base) {
+ case CFI_SP:
+ undwarf->cfa_reg = UNDWARF_REG_SP;
+ break;
+ case CFI_SP_INDIRECT:
+ undwarf->cfa_reg = UNDWARF_REG_SP_INDIRECT;
+ break;
+ case CFI_BP:
+ undwarf->cfa_reg = UNDWARF_REG_BP;
+ break;
+ case CFI_BP_INDIRECT:
+ undwarf->cfa_reg = UNDWARF_REG_BP_INDIRECT;
+ break;
+ case CFI_R10:
+ undwarf->cfa_reg = UNDWARF_REG_R10;
+ break;
+ case CFI_R13:
+ undwarf->cfa_reg = UNDWARF_REG_R13;
+ break;
+ case CFI_DI:
+ undwarf->cfa_reg = UNDWARF_REG_DI;
+ break;
+ case CFI_DX:
+ undwarf->cfa_reg = UNDWARF_REG_DX;
+ break;
+ default:
+ WARN_FUNC("unknown CFA base reg %d",
+ insn->sec, insn->offset, cfa->base);
+ return -1;
+ }
+
+ switch(bp->base) {
+ case CFI_UNDEFINED:
+ undwarf->bp_reg = UNDWARF_REG_UNDEFINED;
+ break;
+ case CFI_CFA:
+ undwarf->bp_reg = UNDWARF_REG_CFA;
+ break;
+ case CFI_BP:
+ undwarf->bp_reg = UNDWARF_REG_BP;
+ break;
+ default:
+ WARN_FUNC("unknown BP base reg %d",
+ insn->sec, insn->offset, bp->base);
+ return -1;
+ }
+
+ undwarf->cfa_offset = cfa->offset;
+ undwarf->bp_offset = bp->offset;
+ undwarf->type = insn->state.type;
+ }
+
+ return 0;
+}
+
+static int create_undwarf_entry(struct section *u_sec, struct section *ip_relasec,
+ unsigned int idx, struct section *insn_sec,
+ unsigned long insn_off, struct undwarf *u)
+{
+ struct undwarf *undwarf;
+ struct rela *rela;
+
+ /* populate undwarf */
+ undwarf = (struct undwarf *)u_sec->data->d_buf + idx;
+ memcpy(undwarf, u, sizeof(*undwarf));
+
+ /* populate rela for ip */
+ rela = malloc(sizeof(*rela));
+ if (!rela) {
+ perror("malloc");
+ return -1;
+ }
+ memset(rela, 0, sizeof(*rela));
+
+ rela->sym = insn_sec->sym;
+ rela->addend = insn_off;
+ rela->type = R_X86_64_PC32;
+ rela->offset = idx * sizeof(int);
+
+ list_add_tail(&rela->list, &ip_relasec->rela_list);
+ hash_add(ip_relasec->rela_hash, &rela->hash, rela->offset);
+
+ return 0;
+}
+
+int create_undwarf_sections(struct objtool_file *file)
+{
+ struct instruction *insn, *prev_insn;
+ struct section *sec, *u_sec, *ip_relasec;
+ unsigned int idx;
+
+ struct undwarf empty = {
+ .cfa_reg = UNDWARF_REG_UNDEFINED,
+ .bp_reg = UNDWARF_REG_UNDEFINED,
+ .type = UNDWARF_TYPE_CFA,
+ };
+
+ sec = find_section_by_name(file->elf, ".undwarf");
+ if (sec) {
+ WARN("file already has .undwarf section, skipping");
+ return -1;
+ }
+
+ /* count the number of needed undwarves */
+ idx = 0;
+ for_each_sec(file, sec) {
+ if (!sec->text)
+ continue;
+
+ prev_insn = NULL;
+ sec_for_each_insn(file, sec, insn) {
+ if (!prev_insn ||
+ memcmp(&insn->undwarf, &prev_insn->undwarf,
+ sizeof(struct undwarf))) {
+ idx++;
+ }
+ prev_insn = insn;
+ }
+
+ /* section terminator */
+ if (prev_insn)
+ idx++;
+ }
+ if (!idx)
+ return -1;
+
+
+ /* create .undwarf_ip and .rela.undwarf_ip sections */
+ sec = elf_create_section(file->elf, ".undwarf_ip", sizeof(int), idx);
+
+ ip_relasec = elf_create_rela_section(file->elf, sec);
+ if (!ip_relasec)
+ return -1;
+
+ /* create .undwarf section */
+ u_sec = elf_create_section(file->elf, ".undwarf",
+ sizeof(struct undwarf), idx);
+
+ /* populate sections */
+ idx = 0;
+ for_each_sec(file, sec) {
+ if (!sec->text)
+ continue;
+
+ prev_insn = NULL;
+ sec_for_each_insn(file, sec, insn) {
+ if (!prev_insn || memcmp(&insn->undwarf,
+ &prev_insn->undwarf,
+ sizeof(struct undwarf))) {
+
+ if (create_undwarf_entry(u_sec, ip_relasec, idx,
+ insn->sec, insn->offset,
+ &insn->undwarf))
+ return -1;
+
+ idx++;
+ }
+ prev_insn = insn;
+ }
+
+ /* section terminator */
+ if (prev_insn) {
+ if (create_undwarf_entry(u_sec, ip_relasec, idx,
+ prev_insn->sec,
+ prev_insn->offset + prev_insn->len,
+ &empty))
+ return -1;
+
+ idx++;
+ }
+ }
+
+ if (elf_rebuild_rela_section(ip_relasec))
+ return -1;
+
+ return 0;
+}
--
2.7.5
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-06-29 09:20 +0200 |
| Subject | Re: [PATCH v2 4/8] objtool: add undwarf debuginfo generation |
| Message-ID | <tXBU7-4BD-9@gated-at.bofh.it> |
| In reply to | #1676801 |
* Josh Poimboeuf <jpoimboe@redhat.com> wrote: > Now that objtool knows the states of all registers on the stack for each > instruction, it's straightforward to generate debuginfo for an unwinder > to use. > > Instead of generating DWARF, generate a new format called undwarf, which > is more suitable for an in-kernel unwinder. See > tools/objtool/Documentation/undwarf.txt for a more detailed description > of this new debuginfo format and why it's preferable to DWARF. > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > --- > tools/objtool/Build | 3 + > tools/objtool/Documentation/stack-validation.txt | 46 ++--- > tools/objtool/builtin-check.c | 2 +- > tools/objtool/builtin-undwarf.c | 70 ++++++++ > tools/objtool/builtin.h | 1 + > tools/objtool/check.c | 59 ++++++- > tools/objtool/check.h | 15 +- > tools/objtool/elf.c | 212 ++++++++++++++++++++-- > tools/objtool/elf.h | 15 +- > tools/objtool/objtool.c | 3 +- > tools/objtool/undwarf-types.h | 81 +++++++++ Just a very quick stylistic suggestion: please name the header 'undwarf_types.h' (note the underscore versus hyphen), which is the common naming pattern used in the kernel. Thanks, Ingo
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-06-29 15:50 +0200 |
| Subject | Re: [PATCH v2 4/8] objtool: add undwarf debuginfo generation |
| Message-ID | <tXHZw-8dW-13@gated-at.bofh.it> |
| In reply to | #1677458 |
On Thu, Jun 29, 2017 at 09:14:14AM +0200, Ingo Molnar wrote: > > * Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > Now that objtool knows the states of all registers on the stack for each > > instruction, it's straightforward to generate debuginfo for an unwinder > > to use. > > > > Instead of generating DWARF, generate a new format called undwarf, which > > is more suitable for an in-kernel unwinder. See > > tools/objtool/Documentation/undwarf.txt for a more detailed description > > of this new debuginfo format and why it's preferable to DWARF. > > > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > > --- > > tools/objtool/Build | 3 + > > tools/objtool/Documentation/stack-validation.txt | 46 ++--- > > tools/objtool/builtin-check.c | 2 +- > > tools/objtool/builtin-undwarf.c | 70 ++++++++ > > tools/objtool/builtin.h | 1 + > > tools/objtool/check.c | 59 ++++++- > > tools/objtool/check.h | 15 +- > > tools/objtool/elf.c | 212 ++++++++++++++++++++-- > > tools/objtool/elf.h | 15 +- > > tools/objtool/objtool.c | 3 +- > > tools/objtool/undwarf-types.h | 81 +++++++++ > > Just a very quick stylistic suggestion: please name the header 'undwarf_types.h' > (note the underscore versus hyphen), which is the common naming pattern used in > the kernel. Ok, will rename it. -- Josh
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-06-29 09:30 +0200 |
| Subject | Re: [PATCH v2 4/8] objtool: add undwarf debuginfo generation |
| Message-ID | <tXC3M-4EX-13@gated-at.bofh.it> |
| In reply to | #1676801 |
* Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> +#ifndef _UNDWARF_TYPES_H
> +#define _UNDWARF_TYPES_H
> +
> +/*
> + * The UNDWARF_REG_* registers are base registers which are used to find other
> + * registers on the stack.
> + *
> + * The CFA (call frame address) is the value of the stack pointer on the
> + * previous frame, i.e. the caller's SP before it called the callee.
> + *
> + * The CFA is usually based on SP, unless a frame pointer has been saved, in
> + * which case it's based on BP.
> + *
> + * BP is usually either based on CFA or is undefined (meaning its value didn't
> + * change for the current frame).
> + *
> + * So the CFA base is usually either SP or BP, and the FP base is usually either
> + * CFA or undefined. The rest of the base registers are needed for special
> + * cases like entry code and gcc aligned stacks.
> + */
> +#define UNDWARF_REG_UNDEFINED 0
> +#define UNDWARF_REG_CFA 1
> +#define UNDWARF_REG_DX 2
> +#define UNDWARF_REG_DI 3
> +#define UNDWARF_REG_BP 4
> +#define UNDWARF_REG_SP 5
> +#define UNDWARF_REG_R10 6
> +#define UNDWARF_REG_R13 7
> +#define UNDWARF_REG_BP_INDIRECT 8
> +#define UNDWARF_REG_SP_INDIRECT 9
> +#define UNDWARF_REG_MAX 15
> +
> +/*
> + * UNDWARF_TYPE_CFA: Indicates that cfa_reg+cfa_offset points to the caller's
> + * stack pointer (aka the CFA in DWARF terms). Used for all callable
> + * functions, i.e. all C code and all callable asm functions.
> + *
> + * UNDWARF_TYPE_REGS: Used in entry code to indicate that cfa_reg+cfa_offset
> + * points to a fully populated pt_regs from a syscall, interrupt, or exception.
> + *
> + * UNDWARF_TYPE_REGS_IRET: Used in entry code to indicate that
> + * cfa_reg+cfa_offset points to the iret return frame.
> + */
> +#define UNDWARF_TYPE_CFA 0
> +#define UNDWARF_TYPE_REGS 1
> +#define UNDWARF_TYPE_REGS_IRET 2
> +
> +/*
> + * This struct contains a simplified version of the DWARF Call Frame
> + * Information standard. It contains only the necessary parts of the real
> + * DWARF, simplified for ease of access by the in-kernel unwinder. It tells
> + * the unwinder how to find the previous SP and BP (and sometimes entry regs)
> + * on the stack for a given code address (IP). Each instance of the struct
> + * corresponds to one or more code locations.
> + */
> +struct undwarf {
> + short cfa_offset;
> + short bp_offset;
> + unsigned cfa_reg:4;
> + unsigned bp_reg:4;
> + unsigned type:2;
> +};
I never know straight away what 'CFA' stands for - could we please use natural
names, i.e. something like:
struct undwarf {
u16 sp_offset;
u16 bp_offset;
unsigned sp_reg:4;
unsigned bp_reg:4;
unsigned type:2;
};
...
struct unwind_hint {
u32 ip;
u16 sp_offset;
u8 sp_reg;
u8 type;
};
?
Also note the slightly cleaner vertical alignment, plus the conversion to more
stable data types: I believe various bits of tooling (perf and so) will eventually
learn about undwarf, so having a well defined cross-arch data structure is
probably of advantage.
Since we are not bound by DWARF anymore, we might as well use readable names and
such?
Plus, shouldn't we use __packed for 'struct undwarf' to minimize the structure's
size (to 6 bytes AFAICS?) - or is optimal packing of the main undwarf array
already guaranteed on every platform with this layout?
Thanks,
Ingo
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-06-29 16:10 +0200 |
| Subject | Re: [PATCH v2 4/8] objtool: add undwarf debuginfo generation |
| Message-ID | <tXIiS-80-31@gated-at.bofh.it> |
| In reply to | #1677478 |
On Thu, Jun 29, 2017 at 09:25:12AM +0200, Ingo Molnar wrote:
> > +/*
> > + * This struct contains a simplified version of the DWARF Call Frame
> > + * Information standard. It contains only the necessary parts of the real
> > + * DWARF, simplified for ease of access by the in-kernel unwinder. It tells
> > + * the unwinder how to find the previous SP and BP (and sometimes entry regs)
> > + * on the stack for a given code address (IP). Each instance of the struct
> > + * corresponds to one or more code locations.
> > + */
> > +struct undwarf {
> > + short cfa_offset;
> > + short bp_offset;
> > + unsigned cfa_reg:4;
> > + unsigned bp_reg:4;
> > + unsigned type:2;
> > +};
>
> I never know straight away what 'CFA' stands for - could we please use natural
> names, i.e. something like:
>
> struct undwarf {
> u16 sp_offset;
> u16 bp_offset;
> unsigned sp_reg:4;
> unsigned bp_reg:4;
> unsigned type:2;
> };
>
> ...
>
> struct unwind_hint {
> u32 ip;
> u16 sp_offset;
> u8 sp_reg;
> u8 type;
> };
>
> ?
>
> Also note the slightly cleaner vertical alignment, plus the conversion to more
> stable data types: I believe various bits of tooling (perf and so) will eventually
> learn about undwarf, so having a well defined cross-arch data structure is
> probably of advantage.
I agree with all your suggestions.
(Though if we want to make it truly cross-arch, 'bp' should be 'fp', for
frame pointer. But there were some objections to that, so I'll leave it
'bp' for now.)
> Since we are not bound by DWARF anymore, we might as well use readable names and
> such?
>
> Plus, shouldn't we use __packed for 'struct undwarf' to minimize the structure's
> size (to 6 bytes AFAICS?) - or is optimal packing of the main undwarf array
> already guaranteed on every platform with this layout?
Ah yes, it should definitely be packed (assuming that doesn't affect
performance negatively).
--
Josh
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-06-29 16:50 +0200 |
| Subject | Re: [PATCH v2 4/8] objtool: add undwarf debuginfo generation |
| Message-ID | <tXIVA-mS-9@gated-at.bofh.it> |
| In reply to | #1677785 |
* Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > Plus, shouldn't we use __packed for 'struct undwarf' to minimize the > > structure's size (to 6 bytes AFAICS?) - or is optimal packing of the main > > undwarf array already guaranteed on every platform with this layout? > > Ah yes, it should definitely be packed (assuming that doesn't affect performance > negatively). So if I count that correctly that should shave another ~1MB off a typical ~4MB table size? Thanks, Ingo
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-06-29 17:10 +0200 |
| Subject | Re: [PATCH v2 4/8] objtool: add undwarf debuginfo generation |
| Message-ID | <tXJeV-IJ-1@gated-at.bofh.it> |
| In reply to | #1677830 |
On Thu, Jun 29, 2017 at 04:46:18PM +0200, Ingo Molnar wrote: > > * Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > > Plus, shouldn't we use __packed for 'struct undwarf' to minimize the > > > structure's size (to 6 bytes AFAICS?) - or is optimal packing of the main > > > undwarf array already guaranteed on every platform with this layout? > > > > Ah yes, it should definitely be packed (assuming that doesn't affect performance > > negatively). > > So if I count that correctly that should shave another ~1MB off a typical ~4MB > table size? Here's what my Fedora kernel looks like *before* the packed change: $ eu-readelf -S vmlinux |grep undwarf [15] .undwarf_ip PROGBITS ffffffff81f776d0 011776d0 0012d9d0 0 A 0 0 1 [16] .undwarf PROGBITS ffffffff820a50a0 012a50a0 0025b3a0 0 A 0 0 1 The total undwarf data size is ~3.5MB. There are 308852 entries of two parallel arrays: * .undwarf (8 bytes/entry) = 2470816 bytes * .undwarf_ip (4 bytes/entry) = 1235408 bytes If we pack undwarf, reducing the size of the .undwarf entries by two bytes, it will save 308852 * 2 = 617704. So the savings will be ~600k, and the typical size will be reduced to ~3MB. -- Josh
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-06-28 17:20 +0200 |
| Subject | [PATCH v2 7/8] x86/asm: add unwind hint annotations to sync_core() |
| Message-ID | <tXmV5-8uj-35@gated-at.bofh.it> |
| In reply to | #1676800 |
This enables the undwarf unwinder to grok the iret in the middle of a C function. Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- arch/x86/include/asm/processor.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index f3b1b27..465e5e2 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -22,6 +22,7 @@ struct vm86; #include <asm/nops.h> #include <asm/special_insns.h> #include <asm/fpu/types.h> +#include <asm/undwarf.h> #include <linux/personality.h> #include <linux/cache.h> @@ -684,6 +685,7 @@ 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" @@ -693,6 +695,7 @@ static inline void sync_core(void) "pushq %q0\n\t" "pushq $1f\n\t" "iretq\n\t" + UNWIND_HINT_RESTORE "1:" : "=&r" (tmp), "+r" (__sp) : : "cc", "memory"); #endif -- 2.7.5
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-06-28 17:20 +0200 |
| Subject | [PATCH v2 2/8] objtool, x86: add several functions and files to the objtool whitelist |
| Message-ID | <tXmV4-8uj-21@gated-at.bofh.it> |
| In reply to | #1676800 |
In preparation for an objtool rewrite which will have broader checks,
whitelist functions and files which cause problems because they do
unusual things with the stack.
These whitelists serve as a TODO list for which functions and files
don't yet have undwarf unwinder coverage. Eventually most of the
whitelists can be removed in favor of manual CFI hint annotations or
objtool improvements.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
arch/x86/crypto/Makefile | 2 ++
arch/x86/crypto/sha1-mb/Makefile | 2 ++
arch/x86/crypto/sha256-mb/Makefile | 2 ++
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/acpi/Makefile | 2 ++
arch/x86/kernel/kprobes/opt.c | 9 ++++++++-
arch/x86/kernel/reboot.c | 2 ++
arch/x86/kvm/svm.c | 2 ++
arch/x86/kvm/vmx.c | 3 +++
arch/x86/lib/msr-reg.S | 8 ++++----
arch/x86/net/Makefile | 2 ++
arch/x86/platform/efi/Makefile | 1 +
arch/x86/power/Makefile | 2 ++
arch/x86/xen/Makefile | 3 +++
kernel/kexec_core.c | 4 +++-
15 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/arch/x86/crypto/Makefile b/arch/x86/crypto/Makefile
index 34b3fa2..9e32d40 100644
--- a/arch/x86/crypto/Makefile
+++ b/arch/x86/crypto/Makefile
@@ -2,6 +2,8 @@
# Arch-specific CryptoAPI modules.
#
+OBJECT_FILES_NON_STANDARD := y
+
avx_supported := $(call as-instr,vpxor %xmm0$(comma)%xmm0$(comma)%xmm0,yes,no)
avx2_supported := $(call as-instr,vpgatherdd %ymm0$(comma)(%eax$(comma)%ymm1\
$(comma)4)$(comma)%ymm2,yes,no)
diff --git a/arch/x86/crypto/sha1-mb/Makefile b/arch/x86/crypto/sha1-mb/Makefile
index 2f87563..2e14acc 100644
--- a/arch/x86/crypto/sha1-mb/Makefile
+++ b/arch/x86/crypto/sha1-mb/Makefile
@@ -2,6 +2,8 @@
# Arch-specific CryptoAPI modules.
#
+OBJECT_FILES_NON_STANDARD := y
+
avx2_supported := $(call as-instr,vpgatherdd %ymm0$(comma)(%eax$(comma)%ymm1\
$(comma)4)$(comma)%ymm2,yes,no)
ifeq ($(avx2_supported),yes)
diff --git a/arch/x86/crypto/sha256-mb/Makefile b/arch/x86/crypto/sha256-mb/Makefile
index 41089e7..45b4fca 100644
--- a/arch/x86/crypto/sha256-mb/Makefile
+++ b/arch/x86/crypto/sha256-mb/Makefile
@@ -2,6 +2,8 @@
# Arch-specific CryptoAPI modules.
#
+OBJECT_FILES_NON_STANDARD := y
+
avx2_supported := $(call as-instr,vpgatherdd %ymm0$(comma)(%eax$(comma)%ymm1\
$(comma)4)$(comma)%ymm2,yes,no)
ifeq ($(avx2_supported),yes)
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 4b99423..3c7c419 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -29,6 +29,7 @@ OBJECT_FILES_NON_STANDARD_head_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_relocate_kernel_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_ftrace_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_test_nx.o := y
+OBJECT_FILES_NON_STANDARD_paravirt_patch_$(BITS).o := y
# If instrumentation of this dir is enabled, boot hangs during first second.
# Probably could be more selective here, but note that files related to irqs,
diff --git a/arch/x86/kernel/acpi/Makefile b/arch/x86/kernel/acpi/Makefile
index 26b78d8..85a9e17 100644
--- a/arch/x86/kernel/acpi/Makefile
+++ b/arch/x86/kernel/acpi/Makefile
@@ -1,3 +1,5 @@
+OBJECT_FILES_NON_STANDARD_wakeup_$(BITS).o := y
+
obj-$(CONFIG_ACPI) += boot.o
obj-$(CONFIG_ACPI_SLEEP) += sleep.o wakeup_$(BITS).o
obj-$(CONFIG_ACPI_APEI) += apei.o
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 901c640..69ea0bc 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -28,6 +28,7 @@
#include <linux/kdebug.h>
#include <linux/kallsyms.h>
#include <linux/ftrace.h>
+#include <linux/frame.h>
#include <asm/text-patching.h>
#include <asm/cacheflush.h>
@@ -94,6 +95,7 @@ static void synthesize_set_arg1(kprobe_opcode_t *addr, unsigned long val)
}
asm (
+ "optprobe_template_func:\n"
".global optprobe_template_entry\n"
"optprobe_template_entry:\n"
#ifdef CONFIG_X86_64
@@ -131,7 +133,12 @@ asm (
" popf\n"
#endif
".global optprobe_template_end\n"
- "optprobe_template_end:\n");
+ "optprobe_template_end:\n"
+ ".type optprobe_template_func, @function\n"
+ ".size optprobe_template_func, .-optprobe_template_func\n");
+
+void optprobe_template_func(void);
+STACK_FRAME_NON_STANDARD(optprobe_template_func);
#define TMPL_MOVE_IDX \
((long)&optprobe_template_val - (long)&optprobe_template_entry)
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 2544700..67393fc 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -9,6 +9,7 @@
#include <linux/sched.h>
#include <linux/tboot.h>
#include <linux/delay.h>
+#include <linux/frame.h>
#include <acpi/reboot.h>
#include <asm/io.h>
#include <asm/apic.h>
@@ -123,6 +124,7 @@ void __noreturn machine_real_restart(unsigned int type)
#ifdef CONFIG_APM_MODULE
EXPORT_SYMBOL(machine_real_restart);
#endif
+STACK_FRAME_NON_STANDARD(machine_real_restart);
/*
* Some Apple MacBook and MacBookPro's needs reboot=p to be able to reboot
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index ba9891a..33460fc 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -36,6 +36,7 @@
#include <linux/slab.h>
#include <linux/amd-iommu.h>
#include <linux/hashtable.h>
+#include <linux/frame.h>
#include <asm/apic.h>
#include <asm/perf_event.h>
@@ -4906,6 +4907,7 @@ static void svm_vcpu_run(struct kvm_vcpu *vcpu)
mark_all_clean(svm->vmcb);
}
+STACK_FRAME_NON_STANDARD(svm_vcpu_run);
static void svm_set_cr3(struct kvm_vcpu *vcpu, unsigned long root)
{
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 7dd53fb..6dcc487 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -33,6 +33,7 @@
#include <linux/slab.h>
#include <linux/tboot.h>
#include <linux/hrtimer.h>
+#include <linux/frame.h>
#include "kvm_cache_regs.h"
#include "x86.h"
@@ -8661,6 +8662,7 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
);
}
}
+STACK_FRAME_NON_STANDARD(vmx_handle_external_intr);
static bool vmx_has_high_real_mode_segbase(void)
{
@@ -9043,6 +9045,7 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu)
vmx_recover_nmi_blocking(vmx);
vmx_complete_interrupts(vmx);
}
+STACK_FRAME_NON_STANDARD(vmx_vcpu_run);
static void vmx_switch_vmcs(struct kvm_vcpu *vcpu, struct loaded_vmcs *vmcs)
{
diff --git a/arch/x86/lib/msr-reg.S b/arch/x86/lib/msr-reg.S
index c815564..10ffa7e 100644
--- a/arch/x86/lib/msr-reg.S
+++ b/arch/x86/lib/msr-reg.S
@@ -13,14 +13,14 @@
.macro op_safe_regs op
ENTRY(\op\()_safe_regs)
pushq %rbx
- pushq %rbp
+ pushq %r12
movq %rdi, %r10 /* Save pointer */
xorl %r11d, %r11d /* Return value */
movl (%rdi), %eax
movl 4(%rdi), %ecx
movl 8(%rdi), %edx
movl 12(%rdi), %ebx
- movl 20(%rdi), %ebp
+ movl 20(%rdi), %r12d
movl 24(%rdi), %esi
movl 28(%rdi), %edi
1: \op
@@ -29,10 +29,10 @@ ENTRY(\op\()_safe_regs)
movl %ecx, 4(%r10)
movl %edx, 8(%r10)
movl %ebx, 12(%r10)
- movl %ebp, 20(%r10)
+ movl %r12d, 20(%r10)
movl %esi, 24(%r10)
movl %edi, 28(%r10)
- popq %rbp
+ popq %r12
popq %rbx
ret
3:
diff --git a/arch/x86/net/Makefile b/arch/x86/net/Makefile
index 90568c3..fefb4b6 100644
--- a/arch/x86/net/Makefile
+++ b/arch/x86/net/Makefile
@@ -1,4 +1,6 @@
#
# Arch-specific network modules
#
+OBJECT_FILES_NON_STANDARD_bpf_jit.o += y
+
obj-$(CONFIG_BPF_JIT) += bpf_jit.o bpf_jit_comp.o
diff --git a/arch/x86/platform/efi/Makefile b/arch/x86/platform/efi/Makefile
index f1d83b3..2f56e1e 100644
--- a/arch/x86/platform/efi/Makefile
+++ b/arch/x86/platform/efi/Makefile
@@ -1,4 +1,5 @@
OBJECT_FILES_NON_STANDARD_efi_thunk_$(BITS).o := y
+OBJECT_FILES_NON_STANDARD_efi_stub_$(BITS).o := y
obj-$(CONFIG_EFI) += quirks.o efi.o efi_$(BITS).o efi_stub_$(BITS).o
obj-$(CONFIG_EARLY_PRINTK_EFI) += early_printk.o
diff --git a/arch/x86/power/Makefile b/arch/x86/power/Makefile
index a6a198c..0504187 100644
--- a/arch/x86/power/Makefile
+++ b/arch/x86/power/Makefile
@@ -1,3 +1,5 @@
+OBJECT_FILES_NON_STANDARD_hibernate_asm_$(BITS).o := y
+
# __restore_processor_state() restores %gs after S3 resume and so should not
# itself be stack-protected
nostackp := $(call cc-option, -fno-stack-protector)
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index fffb0a1..bced7a3 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -1,3 +1,6 @@
+OBJECT_FILES_NON_STANDARD_xen-asm_$(BITS).o := y
+OBJECT_FILES_NON_STANDARD_xen-pvh.o := y
+
ifdef CONFIG_FUNCTION_TRACER
# Do not profile debug and lowlevel utilities
CFLAGS_REMOVE_spinlock.o = -pg
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index ae1a3ba..154ffb4 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -38,6 +38,7 @@
#include <linux/syscore_ops.h>
#include <linux/compiler.h>
#include <linux/hugetlb.h>
+#include <linux/frame.h>
#include <asm/page.h>
#include <asm/sections.h>
@@ -874,7 +875,7 @@ int kexec_load_disabled;
* only when panic_cpu holds the current CPU number; this is the only CPU
* which processes crash_kexec routines.
*/
-void __crash_kexec(struct pt_regs *regs)
+void __noclone __crash_kexec(struct pt_regs *regs)
{
/* Take the kexec_mutex here to prevent sys_kexec_load
* running on one cpu from replacing the crash kernel
@@ -896,6 +897,7 @@ void __crash_kexec(struct pt_regs *regs)
mutex_unlock(&kexec_mutex);
}
}
+STACK_FRAME_NON_STANDARD(__crash_kexec);
void crash_kexec(struct pt_regs *regs)
{
--
2.7.5
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Josh Poimboeuf <tipbot@zytor.com> |
|---|---|
| Date | 2017-06-30 15:20 +0200 |
| Subject | [tip:core/objtool] objtool, x86: Add several functions and files to the objtool whitelist |
| Message-ID | <tY402-5YH-7@gated-at.bofh.it> |
| In reply to | #1676809 |
Commit-ID: c207aee48037abca71c669cbec407b9891965c34
Gitweb: http://git.kernel.org/tip/c207aee48037abca71c669cbec407b9891965c34
Author: Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Wed, 28 Jun 2017 10:11:06 -0500
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 30 Jun 2017 10:19:19 +0200
objtool, x86: Add several functions and files to the objtool whitelist
In preparation for an objtool rewrite which will have broader checks,
whitelist functions and files which cause problems because they do
unusual things with the stack.
These whitelists serve as a TODO list for which functions and files
don't yet have undwarf unwinder coverage. Eventually most of the
whitelists can be removed in favor of manual CFI hint annotations or
objtool improvements.
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/7f934a5d707a574bda33ea282e9478e627fb1829.1498659915.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/crypto/Makefile | 2 ++
arch/x86/crypto/sha1-mb/Makefile | 2 ++
arch/x86/crypto/sha256-mb/Makefile | 2 ++
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/acpi/Makefile | 2 ++
arch/x86/kernel/kprobes/opt.c | 9 ++++++++-
arch/x86/kernel/reboot.c | 2 ++
arch/x86/kvm/svm.c | 2 ++
arch/x86/kvm/vmx.c | 3 +++
arch/x86/lib/msr-reg.S | 8 ++++----
arch/x86/net/Makefile | 2 ++
arch/x86/platform/efi/Makefile | 1 +
arch/x86/power/Makefile | 2 ++
arch/x86/xen/Makefile | 3 +++
kernel/kexec_core.c | 4 +++-
15 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/arch/x86/crypto/Makefile b/arch/x86/crypto/Makefile
index 34b3fa2..9e32d40 100644
--- a/arch/x86/crypto/Makefile
+++ b/arch/x86/crypto/Makefile
@@ -2,6 +2,8 @@
# Arch-specific CryptoAPI modules.
#
+OBJECT_FILES_NON_STANDARD := y
+
avx_supported := $(call as-instr,vpxor %xmm0$(comma)%xmm0$(comma)%xmm0,yes,no)
avx2_supported := $(call as-instr,vpgatherdd %ymm0$(comma)(%eax$(comma)%ymm1\
$(comma)4)$(comma)%ymm2,yes,no)
diff --git a/arch/x86/crypto/sha1-mb/Makefile b/arch/x86/crypto/sha1-mb/Makefile
index 2f87563..2e14acc 100644
--- a/arch/x86/crypto/sha1-mb/Makefile
+++ b/arch/x86/crypto/sha1-mb/Makefile
@@ -2,6 +2,8 @@
# Arch-specific CryptoAPI modules.
#
+OBJECT_FILES_NON_STANDARD := y
+
avx2_supported := $(call as-instr,vpgatherdd %ymm0$(comma)(%eax$(comma)%ymm1\
$(comma)4)$(comma)%ymm2,yes,no)
ifeq ($(avx2_supported),yes)
diff --git a/arch/x86/crypto/sha256-mb/Makefile b/arch/x86/crypto/sha256-mb/Makefile
index 41089e7..45b4fca 100644
--- a/arch/x86/crypto/sha256-mb/Makefile
+++ b/arch/x86/crypto/sha256-mb/Makefile
@@ -2,6 +2,8 @@
# Arch-specific CryptoAPI modules.
#
+OBJECT_FILES_NON_STANDARD := y
+
avx2_supported := $(call as-instr,vpgatherdd %ymm0$(comma)(%eax$(comma)%ymm1\
$(comma)4)$(comma)%ymm2,yes,no)
ifeq ($(avx2_supported),yes)
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 4b99423..3c7c419 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -29,6 +29,7 @@ OBJECT_FILES_NON_STANDARD_head_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_relocate_kernel_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_ftrace_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_test_nx.o := y
+OBJECT_FILES_NON_STANDARD_paravirt_patch_$(BITS).o := y
# If instrumentation of this dir is enabled, boot hangs during first second.
# Probably could be more selective here, but note that files related to irqs,
diff --git a/arch/x86/kernel/acpi/Makefile b/arch/x86/kernel/acpi/Makefile
index 26b78d8..85a9e17 100644
--- a/arch/x86/kernel/acpi/Makefile
+++ b/arch/x86/kernel/acpi/Makefile
@@ -1,3 +1,5 @@
+OBJECT_FILES_NON_STANDARD_wakeup_$(BITS).o := y
+
obj-$(CONFIG_ACPI) += boot.o
obj-$(CONFIG_ACPI_SLEEP) += sleep.o wakeup_$(BITS).o
obj-$(CONFIG_ACPI_APEI) += apei.o
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 901c640..69ea0bc 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -28,6 +28,7 @@
#include <linux/kdebug.h>
#include <linux/kallsyms.h>
#include <linux/ftrace.h>
+#include <linux/frame.h>
#include <asm/text-patching.h>
#include <asm/cacheflush.h>
@@ -94,6 +95,7 @@ static void synthesize_set_arg1(kprobe_opcode_t *addr, unsigned long val)
}
asm (
+ "optprobe_template_func:\n"
".global optprobe_template_entry\n"
"optprobe_template_entry:\n"
#ifdef CONFIG_X86_64
@@ -131,7 +133,12 @@ asm (
" popf\n"
#endif
".global optprobe_template_end\n"
- "optprobe_template_end:\n");
+ "optprobe_template_end:\n"
+ ".type optprobe_template_func, @function\n"
+ ".size optprobe_template_func, .-optprobe_template_func\n");
+
+void optprobe_template_func(void);
+STACK_FRAME_NON_STANDARD(optprobe_template_func);
#define TMPL_MOVE_IDX \
((long)&optprobe_template_val - (long)&optprobe_template_entry)
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 2544700..67393fc 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -9,6 +9,7 @@
#include <linux/sched.h>
#include <linux/tboot.h>
#include <linux/delay.h>
+#include <linux/frame.h>
#include <acpi/reboot.h>
#include <asm/io.h>
#include <asm/apic.h>
@@ -123,6 +124,7 @@ void __noreturn machine_real_restart(unsigned int type)
#ifdef CONFIG_APM_MODULE
EXPORT_SYMBOL(machine_real_restart);
#endif
+STACK_FRAME_NON_STANDARD(machine_real_restart);
/*
* Some Apple MacBook and MacBookPro's needs reboot=p to be able to reboot
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index ba9891a..33460fc 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -36,6 +36,7 @@
#include <linux/slab.h>
#include <linux/amd-iommu.h>
#include <linux/hashtable.h>
+#include <linux/frame.h>
#include <asm/apic.h>
#include <asm/perf_event.h>
@@ -4906,6 +4907,7 @@ static void svm_vcpu_run(struct kvm_vcpu *vcpu)
mark_all_clean(svm->vmcb);
}
+STACK_FRAME_NON_STANDARD(svm_vcpu_run);
static void svm_set_cr3(struct kvm_vcpu *vcpu, unsigned long root)
{
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ca5d2b9..1b469b6 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -33,6 +33,7 @@
#include <linux/slab.h>
#include <linux/tboot.h>
#include <linux/hrtimer.h>
+#include <linux/frame.h>
#include "kvm_cache_regs.h"
#include "x86.h"
@@ -8652,6 +8653,7 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
);
}
}
+STACK_FRAME_NON_STANDARD(vmx_handle_external_intr);
static bool vmx_has_high_real_mode_segbase(void)
{
@@ -9028,6 +9030,7 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu)
vmx_recover_nmi_blocking(vmx);
vmx_complete_interrupts(vmx);
}
+STACK_FRAME_NON_STANDARD(vmx_vcpu_run);
static void vmx_switch_vmcs(struct kvm_vcpu *vcpu, struct loaded_vmcs *vmcs)
{
diff --git a/arch/x86/lib/msr-reg.S b/arch/x86/lib/msr-reg.S
index c815564..10ffa7e 100644
--- a/arch/x86/lib/msr-reg.S
+++ b/arch/x86/lib/msr-reg.S
@@ -13,14 +13,14 @@
.macro op_safe_regs op
ENTRY(\op\()_safe_regs)
pushq %rbx
- pushq %rbp
+ pushq %r12
movq %rdi, %r10 /* Save pointer */
xorl %r11d, %r11d /* Return value */
movl (%rdi), %eax
movl 4(%rdi), %ecx
movl 8(%rdi), %edx
movl 12(%rdi), %ebx
- movl 20(%rdi), %ebp
+ movl 20(%rdi), %r12d
movl 24(%rdi), %esi
movl 28(%rdi), %edi
1: \op
@@ -29,10 +29,10 @@ ENTRY(\op\()_safe_regs)
movl %ecx, 4(%r10)
movl %edx, 8(%r10)
movl %ebx, 12(%r10)
- movl %ebp, 20(%r10)
+ movl %r12d, 20(%r10)
movl %esi, 24(%r10)
movl %edi, 28(%r10)
- popq %rbp
+ popq %r12
popq %rbx
ret
3:
diff --git a/arch/x86/net/Makefile b/arch/x86/net/Makefile
index 90568c3..fefb4b6 100644
--- a/arch/x86/net/Makefile
+++ b/arch/x86/net/Makefile
@@ -1,4 +1,6 @@
#
# Arch-specific network modules
#
+OBJECT_FILES_NON_STANDARD_bpf_jit.o += y
+
obj-$(CONFIG_BPF_JIT) += bpf_jit.o bpf_jit_comp.o
diff --git a/arch/x86/platform/efi/Makefile b/arch/x86/platform/efi/Makefile
index f1d83b3..2f56e1e 100644
--- a/arch/x86/platform/efi/Makefile
+++ b/arch/x86/platform/efi/Makefile
@@ -1,4 +1,5 @@
OBJECT_FILES_NON_STANDARD_efi_thunk_$(BITS).o := y
+OBJECT_FILES_NON_STANDARD_efi_stub_$(BITS).o := y
obj-$(CONFIG_EFI) += quirks.o efi.o efi_$(BITS).o efi_stub_$(BITS).o
obj-$(CONFIG_EARLY_PRINTK_EFI) += early_printk.o
diff --git a/arch/x86/power/Makefile b/arch/x86/power/Makefile
index a6a198c..0504187 100644
--- a/arch/x86/power/Makefile
+++ b/arch/x86/power/Makefile
@@ -1,3 +1,5 @@
+OBJECT_FILES_NON_STANDARD_hibernate_asm_$(BITS).o := y
+
# __restore_processor_state() restores %gs after S3 resume and so should not
# itself be stack-protected
nostackp := $(call cc-option, -fno-stack-protector)
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index fffb0a1..bced7a3 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -1,3 +1,6 @@
+OBJECT_FILES_NON_STANDARD_xen-asm_$(BITS).o := y
+OBJECT_FILES_NON_STANDARD_xen-pvh.o := y
+
ifdef CONFIG_FUNCTION_TRACER
# Do not profile debug and lowlevel utilities
CFLAGS_REMOVE_spinlock.o = -pg
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index ae1a3ba..154ffb4 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -38,6 +38,7 @@
#include <linux/syscore_ops.h>
#include <linux/compiler.h>
#include <linux/hugetlb.h>
+#include <linux/frame.h>
#include <asm/page.h>
#include <asm/sections.h>
@@ -874,7 +875,7 @@ int kexec_load_disabled;
* only when panic_cpu holds the current CPU number; this is the only CPU
* which processes crash_kexec routines.
*/
-void __crash_kexec(struct pt_regs *regs)
+void __noclone __crash_kexec(struct pt_regs *regs)
{
/* Take the kexec_mutex here to prevent sys_kexec_load
* running on one cpu from replacing the crash kernel
@@ -896,6 +897,7 @@ void __crash_kexec(struct pt_regs *regs)
mutex_unlock(&kexec_mutex);
}
}
+STACK_FRAME_NON_STANDARD(__crash_kexec);
void crash_kexec(struct pt_regs *regs)
{
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-06-28 17:20 +0200 |
| Subject | [PATCH v2 6/8] x86/entry: add unwind hint annotations |
| Message-ID | <tXmV4-8uj-23@gated-at.bofh.it> |
| In reply to | #1676800 |
Add unwind hint annotations to entry_64.S. This will enable the undwarf
unwinder to unwind through any location in the entry code including
syscalls, interrupts, and exceptions.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
arch/x86/entry/Makefile | 1 -
arch/x86/entry/calling.h | 6 +++++
arch/x86/entry/entry_64.S | 56 ++++++++++++++++++++++++++++++++++++++++++-----
3 files changed, 56 insertions(+), 7 deletions(-)
diff --git a/arch/x86/entry/Makefile b/arch/x86/entry/Makefile
index 9976fce..af28a8a 100644
--- a/arch/x86/entry/Makefile
+++ b/arch/x86/entry/Makefile
@@ -2,7 +2,6 @@
# Makefile for the x86 low level entry code
#
-OBJECT_FILES_NON_STANDARD_entry_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_entry_64_compat.o := y
CFLAGS_syscall_64.o += $(call cc-option,-Wno-override-init,)
diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index 05ed3d3..4050b73 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -1,4 +1,6 @@
#include <linux/jump_label.h>
+#include <asm/undwarf.h>
+
/*
@@ -112,6 +114,7 @@ For 32-bit we have the following conventions - kernel is built with
movq %rdx, 12*8+\offset(%rsp)
movq %rsi, 13*8+\offset(%rsp)
movq %rdi, 14*8+\offset(%rsp)
+ UNWIND_HINT_REGS offset=\offset extra=0
.endm
.macro SAVE_C_REGS offset=0
SAVE_C_REGS_HELPER \offset, 1, 1, 1, 1
@@ -136,6 +139,7 @@ For 32-bit we have the following conventions - kernel is built with
movq %r12, 3*8+\offset(%rsp)
movq %rbp, 4*8+\offset(%rsp)
movq %rbx, 5*8+\offset(%rsp)
+ UNWIND_HINT_REGS offset=\offset
.endm
.macro RESTORE_EXTRA_REGS offset=0
@@ -145,6 +149,7 @@ For 32-bit we have the following conventions - kernel is built with
movq 3*8+\offset(%rsp), %r12
movq 4*8+\offset(%rsp), %rbp
movq 5*8+\offset(%rsp), %rbx
+ UNWIND_HINT_REGS offset=\offset extra=0
.endm
.macro RESTORE_C_REGS_HELPER rstor_rax=1, rstor_rcx=1, rstor_r11=1, rstor_r8910=1, rstor_rdx=1
@@ -167,6 +172,7 @@ For 32-bit we have the following conventions - kernel is built with
.endif
movq 13*8(%rsp), %rsi
movq 14*8(%rsp), %rdi
+ UNWIND_HINT_IRET_REGS offset=16*8
.endm
.macro RESTORE_C_REGS
RESTORE_C_REGS_HELPER 1,1,1,1,1
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index a9a8027..9075a6c 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -36,6 +36,7 @@
#include <asm/smap.h>
#include <asm/pgtable_types.h>
#include <asm/export.h>
+#include <asm/frame.h>
#include <linux/err.h>
.code64
@@ -43,9 +44,10 @@
#ifdef CONFIG_PARAVIRT
ENTRY(native_usergs_sysret64)
+ UNWIND_HINT_EMPTY
swapgs
sysretq
-ENDPROC(native_usergs_sysret64)
+END(native_usergs_sysret64)
#endif /* CONFIG_PARAVIRT */
.macro TRACE_IRQS_IRETQ
@@ -134,6 +136,7 @@ ENDPROC(native_usergs_sysret64)
*/
ENTRY(entry_SYSCALL_64)
+ UNWIND_HINT_EMPTY
/*
* Interrupts are off on entry.
* We do not frame this tiny irq-off block with TRACE_IRQS_OFF/ON,
@@ -169,6 +172,7 @@ GLOBAL(entry_SYSCALL_64_after_swapgs)
pushq %r10 /* pt_regs->r10 */
pushq %r11 /* pt_regs->r11 */
sub $(6*8), %rsp /* pt_regs->bp, bx, r12-15 not saved */
+ UNWIND_HINT_REGS extra=0
/*
* If we need to do entry work or if we guess we'll need to do
@@ -223,6 +227,7 @@ entry_SYSCALL_64_fastpath:
movq EFLAGS(%rsp), %r11
RESTORE_C_REGS_EXCEPT_RCX_R11
movq RSP(%rsp), %rsp
+ UNWIND_HINT_EMPTY
USERGS_SYSRET64
1:
@@ -316,6 +321,7 @@ syscall_return_via_sysret:
/* rcx and r11 are already restored (see code above) */
RESTORE_C_REGS_EXCEPT_RCX_R11
movq RSP(%rsp), %rsp
+ UNWIND_HINT_EMPTY
USERGS_SYSRET64
opportunistic_sysret_failed:
@@ -343,6 +349,7 @@ ENTRY(stub_ptregs_64)
DISABLE_INTERRUPTS(CLBR_ANY)
TRACE_IRQS_OFF
popq %rax
+ UNWIND_HINT_REGS extra=0
jmp entry_SYSCALL64_slow_path
1:
@@ -351,6 +358,7 @@ END(stub_ptregs_64)
.macro ptregs_stub func
ENTRY(ptregs_\func)
+ UNWIND_HINT_FUNC
leaq \func(%rip), %rax
jmp stub_ptregs_64
END(ptregs_\func)
@@ -367,6 +375,7 @@ END(ptregs_\func)
* %rsi: next task
*/
ENTRY(__switch_to_asm)
+ UNWIND_HINT_FUNC
/*
* Save callee-saved registers
* This must match the order in inactive_task_frame
@@ -406,6 +415,7 @@ END(__switch_to_asm)
* r12: kernel thread arg
*/
ENTRY(ret_from_fork)
+ UNWIND_HINT_EMPTY
movq %rax, %rdi
call schedule_tail /* rdi: 'prev' task parameter */
@@ -413,6 +423,7 @@ ENTRY(ret_from_fork)
jnz 1f /* kernel threads are uncommon */
2:
+ UNWIND_HINT_REGS
movq %rsp, %rdi
call syscall_return_slowpath /* returns with IRQs disabled */
TRACE_IRQS_ON /* user mode is traced as IRQS on */
@@ -440,10 +451,11 @@ END(ret_from_fork)
ENTRY(irq_entries_start)
vector=FIRST_EXTERNAL_VECTOR
.rept (FIRST_SYSTEM_VECTOR - FIRST_EXTERNAL_VECTOR)
+ UNWIND_HINT_IRET_REGS
pushq $(~vector+0x80) /* Note: always in signed byte range */
- vector=vector+1
jmp common_interrupt
.align 8
+ vector=vector+1
.endr
END(irq_entries_start)
@@ -495,7 +507,9 @@ END(irq_entries_start)
movq %rsp, %rdi
incl PER_CPU_VAR(irq_count)
cmovzq PER_CPU_VAR(irq_stack_ptr), %rsp
+ UNWIND_HINT_REGS base=rdi
pushq %rdi
+ UNWIND_HINT_REGS indirect=1
/* We entered an interrupt context - irqs are off: */
TRACE_IRQS_OFF
@@ -519,6 +533,7 @@ ret_from_intr:
/* Restore saved previous stack */
popq %rsp
+ UNWIND_HINT_REGS
testb $3, CS(%rsp)
jz retint_kernel
@@ -561,6 +576,7 @@ restore_c_regs_and_iret:
INTERRUPT_RETURN
ENTRY(native_iret)
+ UNWIND_HINT_IRET_REGS
/*
* Are we returning to a stack segment from the LDT? Note: in
* 64-bit mode SS:RSP on the exception stack is always valid.
@@ -633,6 +649,7 @@ native_irq_return_ldt:
orq PER_CPU_VAR(espfix_stack), %rax
SWAPGS
movq %rax, %rsp
+ UNWIND_HINT_IRET_REGS offset=8
/*
* At this point, we cannot write to the stack any more, but we can
@@ -654,6 +671,7 @@ END(common_interrupt)
*/
.macro apicinterrupt3 num sym do_sym
ENTRY(\sym)
+ UNWIND_HINT_IRET_REGS
ASM_CLAC
pushq $~(\num)
.Lcommon_\sym:
@@ -739,6 +757,8 @@ apicinterrupt IRQ_WORK_VECTOR irq_work_interrupt smp_irq_work_interrupt
.macro idtentry sym do_sym has_error_code:req paranoid=0 shift_ist=-1
ENTRY(\sym)
+ UNWIND_HINT_IRET_REGS offset=8
+
/* Sanity check */
.if \shift_ist != -1 && \paranoid == 0
.error "using shift_ist requires paranoid=1"
@@ -762,6 +782,7 @@ ENTRY(\sym)
.else
call error_entry
.endif
+ UNWIND_HINT_REGS
/* returned flag: ebx=0: need swapgs on exit, ebx=1: don't need it */
.if \paranoid
@@ -859,6 +880,7 @@ idtentry simd_coprocessor_error do_simd_coprocessor_error has_error_code=0
* edi: new selector
*/
ENTRY(native_load_gs_index)
+ FRAME_BEGIN
pushfq
DISABLE_INTERRUPTS(CLBR_ANY & ~CLBR_RDI)
SWAPGS
@@ -867,8 +889,9 @@ ENTRY(native_load_gs_index)
2: ALTERNATIVE "", "mfence", X86_BUG_SWAPGS_FENCE
SWAPGS
popfq
+ FRAME_END
ret
-END(native_load_gs_index)
+ENDPROC(native_load_gs_index)
EXPORT_SYMBOL(native_load_gs_index)
_ASM_EXTABLE(.Lgs_change, bad_gs)
@@ -898,7 +921,7 @@ ENTRY(do_softirq_own_stack)
leaveq
decl PER_CPU_VAR(irq_count)
ret
-END(do_softirq_own_stack)
+ENDPROC(do_softirq_own_stack)
#ifdef CONFIG_XEN
idtentry xen_hypervisor_callback xen_do_hypervisor_callback has_error_code=0
@@ -922,13 +945,18 @@ ENTRY(xen_do_hypervisor_callback) /* do_hypervisor_callback(struct *pt_regs) */
* Since we don't modify %rdi, evtchn_do_upall(struct *pt_regs) will
* see the correct pointer to the pt_regs
*/
+ UNWIND_HINT_FUNC
movq %rdi, %rsp /* we don't return, adjust the stack frame */
+ UNWIND_HINT_REGS
11: incl PER_CPU_VAR(irq_count)
movq %rsp, %rbp
cmovzq PER_CPU_VAR(irq_stack_ptr), %rsp
+ UNWIND_HINT_REGS base=rbp
pushq %rbp /* frame pointer backlink */
+ UNWIND_HINT_REGS indirect=1
call xen_evtchn_do_upcall
popq %rsp
+ UNWIND_HINT_REGS
decl PER_CPU_VAR(irq_count)
#ifndef CONFIG_PREEMPT
call xen_maybe_preempt_hcall
@@ -950,6 +978,7 @@ END(xen_do_hypervisor_callback)
* with its current contents: any discrepancy means we in category 1.
*/
ENTRY(xen_failsafe_callback)
+ UNWIND_HINT_EMPTY
movl %ds, %ecx
cmpw %cx, 0x10(%rsp)
jne 1f
@@ -969,11 +998,13 @@ ENTRY(xen_failsafe_callback)
pushq $0 /* RIP */
pushq %r11
pushq %rcx
+ UNWIND_HINT_IRET_REGS offset=8
jmp general_protection
1: /* Segment mismatch => Category 1 (Bad segment). Retry the IRET. */
movq (%rsp), %rcx
movq 8(%rsp), %r11
addq $0x30, %rsp
+ UNWIND_HINT_IRET_REGS
pushq $-1 /* orig_ax = -1 => not a system call */
ALLOC_PT_GPREGS_ON_STACK
SAVE_C_REGS
@@ -1019,6 +1050,7 @@ idtentry machine_check has_error_code=0 paranoid=1 do_sym=*machine_check_vec
* Return: ebx=0: need swapgs on exit, ebx=1: otherwise
*/
ENTRY(paranoid_entry)
+ UNWIND_HINT_FUNC
cld
SAVE_C_REGS 8
SAVE_EXTRA_REGS 8
@@ -1046,6 +1078,7 @@ END(paranoid_entry)
* On entry, ebx is "no swapgs" flag (1: don't need swapgs, 0: need it)
*/
ENTRY(paranoid_exit)
+ UNWIND_HINT_REGS
DISABLE_INTERRUPTS(CLBR_ANY)
TRACE_IRQS_OFF_DEBUG
testl %ebx, %ebx /* swapgs needed? */
@@ -1067,6 +1100,7 @@ END(paranoid_exit)
* Return: EBX=0: came from user mode; EBX=1: otherwise
*/
ENTRY(error_entry)
+ UNWIND_HINT_FUNC
cld
SAVE_C_REGS 8
SAVE_EXTRA_REGS 8
@@ -1151,6 +1185,7 @@ END(error_entry)
* 0: user gsbase is loaded, we need SWAPGS and standard preparation for return to usermode
*/
ENTRY(error_exit)
+ UNWIND_HINT_REGS
DISABLE_INTERRUPTS(CLBR_ANY)
TRACE_IRQS_OFF
testl %ebx, %ebx
@@ -1160,6 +1195,7 @@ END(error_exit)
/* Runs on exception stack */
ENTRY(nmi)
+ UNWIND_HINT_IRET_REGS
/*
* Fix up the exception frame if we're on Xen.
* PARAVIRT_ADJUST_EXCEPTION_FRAME is guaranteed to push at most
@@ -1231,11 +1267,13 @@ ENTRY(nmi)
cld
movq %rsp, %rdx
movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp
+ UNWIND_HINT_IRET_REGS base=rdx offset=8
pushq 5*8(%rdx) /* pt_regs->ss */
pushq 4*8(%rdx) /* pt_regs->rsp */
pushq 3*8(%rdx) /* pt_regs->flags */
pushq 2*8(%rdx) /* pt_regs->cs */
pushq 1*8(%rdx) /* pt_regs->rip */
+ UNWIND_HINT_IRET_REGS
pushq $-1 /* pt_regs->orig_ax */
pushq %rdi /* pt_regs->di */
pushq %rsi /* pt_regs->si */
@@ -1252,6 +1290,7 @@ ENTRY(nmi)
pushq %r13 /* pt_regs->r13 */
pushq %r14 /* pt_regs->r14 */
pushq %r15 /* pt_regs->r15 */
+ UNWIND_HINT_REGS
ENCODE_FRAME_POINTER
/*
@@ -1406,6 +1445,7 @@ first_nmi:
.rept 5
pushq 11*8(%rsp)
.endr
+ UNWIND_HINT_IRET_REGS
/* Everything up to here is safe from nested NMIs */
@@ -1421,6 +1461,7 @@ first_nmi:
pushq $__KERNEL_CS /* CS */
pushq $1f /* RIP */
INTERRUPT_RETURN /* continues at repeat_nmi below */
+ UNWIND_HINT_IRET_REGS
1:
#endif
@@ -1470,6 +1511,7 @@ end_repeat_nmi:
* exceptions might do.
*/
call paranoid_entry
+ UNWIND_HINT_REGS
/* paranoidentry do_nmi, 0; without TRACE_IRQS_OFF */
movq %rsp, %rdi
@@ -1507,17 +1549,19 @@ nmi_restore:
END(nmi)
ENTRY(ignore_sysret)
+ UNWIND_HINT_EMPTY
mov $-ENOSYS, %eax
sysret
END(ignore_sysret)
ENTRY(rewind_stack_do_exit)
+ UNWIND_HINT_FUNC
/* Prevent any naive code from trying to unwind to our caller. */
xorl %ebp, %ebp
movq PER_CPU_VAR(cpu_current_top_of_stack), %rax
- leaq -TOP_OF_KERNEL_STACK_PADDING-PTREGS_SIZE(%rax), %rsp
+ leaq -PTREGS_SIZE(%rax), %rsp
+ UNWIND_HINT_FUNC cfa_offset=PTREGS_SIZE
call do_exit
-1: jmp 1b
END(rewind_stack_do_exit)
--
2.7.5
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-06-29 20:00 +0200 |
| Subject | Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations |
| Message-ID | <tXLTs-2de-23@gated-at.bofh.it> |
| In reply to | #1676810 |
There's a bug here that will need a small change to the entry code. Mike Galbraith reported: WARNING: can't dereference registers at ffffc900089d7e08 for ip ffffffff81740bbb After some looking I found that it's caused by the following code snippet in the 'interrupt' macro in entry_64.S: /* * Save previous stack pointer, optionally switch to interrupt stack. * irq_count is used to check if a CPU is already on an interrupt stack * or not. While this is essentially redundant with preempt_count it is * a little cheaper to use a separate counter in the PDA (short of * moving irq_enter into assembly, which would be too much work) */ movq %rsp, %rdi incl PER_CPU_VAR(irq_count) cmovzq PER_CPU_VAR(irq_stack_ptr), %rsp UNWIND_HINT_REGS base=rdi pushq %rdi UNWIND_HINT_REGS indirect=1 The problem is that it's changing the stack pointer *before* writing the previous stack pointer (push %rdi). So when unwinding from an NMI which hit between the rsp write and the rdi push, the unwinder tries to access the regs on the previous stack (by reading rdi), but the previous stack pointer isn't there yet, so the access is considered out of bounds. -- Josh
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@kernel.org> |
|---|---|
| Date | 2017-06-29 21:00 +0200 |
| Subject | Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations |
| Message-ID | <tXMPw-2Ms-23@gated-at.bofh.it> |
| In reply to | #1678009 |
On Thu, Jun 29, 2017 at 10:53 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > There's a bug here that will need a small change to the entry code. > > Mike Galbraith reported: > > WARNING: can't dereference registers at ffffc900089d7e08 for ip ffffffff81740bbb > > After some looking I found that it's caused by the following code > snippet in the 'interrupt' macro in entry_64.S: > > /* > * Save previous stack pointer, optionally switch to interrupt stack. > * irq_count is used to check if a CPU is already on an interrupt stack > * or not. While this is essentially redundant with preempt_count it is > * a little cheaper to use a separate counter in the PDA (short of > * moving irq_enter into assembly, which would be too much work) > */ > movq %rsp, %rdi > incl PER_CPU_VAR(irq_count) > cmovzq PER_CPU_VAR(irq_stack_ptr), %rsp > UNWIND_HINT_REGS base=rdi > pushq %rdi > UNWIND_HINT_REGS indirect=1 > > The problem is that it's changing the stack pointer *before* writing the > previous stack pointer (push %rdi). So when unwinding from an NMI which > hit between the rsp write and the rdi push, the unwinder tries to access > the regs on the previous stack (by reading rdi), but the previous stack > pointer isn't there yet, so the access is considered out of bounds. Ugh, that code. Does this problem go away with this patch applied: https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/entry_ist&id=2231ec7e0bcc1a2bc94a17081511ab54cc6badd1 If so, want to update the patch for new kernels (shouldn't conflict with anything except your unwind hints)? --Andy
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-06-29 21:10 +0200 |
| Subject | Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations |
| Message-ID | <tXMZd-354-49@gated-at.bofh.it> |
| In reply to | #1678071 |
On Thu, Jun 29, 2017 at 11:50:18AM -0700, Andy Lutomirski wrote: > On Thu, Jun 29, 2017 at 10:53 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > There's a bug here that will need a small change to the entry code. > > > > Mike Galbraith reported: > > > > WARNING: can't dereference registers at ffffc900089d7e08 for ip ffffffff81740bbb > > > > After some looking I found that it's caused by the following code > > snippet in the 'interrupt' macro in entry_64.S: > > > > /* > > * Save previous stack pointer, optionally switch to interrupt stack. > > * irq_count is used to check if a CPU is already on an interrupt stack > > * or not. While this is essentially redundant with preempt_count it is > > * a little cheaper to use a separate counter in the PDA (short of > > * moving irq_enter into assembly, which would be too much work) > > */ > > movq %rsp, %rdi > > incl PER_CPU_VAR(irq_count) > > cmovzq PER_CPU_VAR(irq_stack_ptr), %rsp > > UNWIND_HINT_REGS base=rdi > > pushq %rdi > > UNWIND_HINT_REGS indirect=1 > > > > The problem is that it's changing the stack pointer *before* writing the > > previous stack pointer (push %rdi). So when unwinding from an NMI which > > hit between the rsp write and the rdi push, the unwinder tries to access > > the regs on the previous stack (by reading rdi), but the previous stack > > pointer isn't there yet, so the access is considered out of bounds. > > Ugh, that code. Does this problem go away with this patch applied: > > https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/entry_ist&id=2231ec7e0bcc1a2bc94a17081511ab54cc6badd1 > > If so, want to update the patch for new kernels (shouldn't conflict > with anything except your unwind hints)? I don't think that patch will fix it, because it still updates rsp *before* writing the old rsp on the new stack. So there's still a window where the "previous stack" pointer is missing. -- Josh
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@kernel.org> |
|---|---|
| Date | 2017-06-29 23:20 +0200 |
| Subject | Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations |
| Message-ID | <tXP0Z-4yl-5@gated-at.bofh.it> |
| In reply to | #1678093 |
On Thu, Jun 29, 2017 at 12:05 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > On Thu, Jun 29, 2017 at 11:50:18AM -0700, Andy Lutomirski wrote: >> On Thu, Jun 29, 2017 at 10:53 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: >> > There's a bug here that will need a small change to the entry code. >> > >> > Mike Galbraith reported: >> > >> > WARNING: can't dereference registers at ffffc900089d7e08 for ip ffffffff81740bbb >> > >> > After some looking I found that it's caused by the following code >> > snippet in the 'interrupt' macro in entry_64.S: >> > >> > /* >> > * Save previous stack pointer, optionally switch to interrupt stack. >> > * irq_count is used to check if a CPU is already on an interrupt stack >> > * or not. While this is essentially redundant with preempt_count it is >> > * a little cheaper to use a separate counter in the PDA (short of >> > * moving irq_enter into assembly, which would be too much work) >> > */ >> > movq %rsp, %rdi >> > incl PER_CPU_VAR(irq_count) >> > cmovzq PER_CPU_VAR(irq_stack_ptr), %rsp >> > UNWIND_HINT_REGS base=rdi >> > pushq %rdi >> > UNWIND_HINT_REGS indirect=1 >> > >> > The problem is that it's changing the stack pointer *before* writing the >> > previous stack pointer (push %rdi). So when unwinding from an NMI which >> > hit between the rsp write and the rdi push, the unwinder tries to access >> > the regs on the previous stack (by reading rdi), but the previous stack >> > pointer isn't there yet, so the access is considered out of bounds. >> >> Ugh, that code. Does this problem go away with this patch applied: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/entry_ist&id=2231ec7e0bcc1a2bc94a17081511ab54cc6badd1 >> >> If so, want to update the patch for new kernels (shouldn't conflict >> with anything except your unwind hints)? > > I don't think that patch will fix it, because it still updates rsp > *before* writing the old rsp on the new stack. So there's still a > window where the "previous stack" pointer is missing. But it's in a register. Is undwarf not able to grok that? I have no fundamental problem with pushing it to the new stack first, but the actual asm is nastier because we don't have an addressing mode that's *(*(gs:blahblahblah)) = reg. At least my patch makes all the copied of this code identical so the problem can be solved only once.
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-06-29 23:50 +0200 |
| Subject | Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations |
| Message-ID | <tXPu3-4Pd-27@gated-at.bofh.it> |
| In reply to | #1678204 |
On Thu, Jun 29, 2017 at 02:09:54PM -0700, Andy Lutomirski wrote: > On Thu, Jun 29, 2017 at 12:05 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > On Thu, Jun 29, 2017 at 11:50:18AM -0700, Andy Lutomirski wrote: > >> On Thu, Jun 29, 2017 at 10:53 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > >> > There's a bug here that will need a small change to the entry code. > >> > > >> > Mike Galbraith reported: > >> > > >> > WARNING: can't dereference registers at ffffc900089d7e08 for ip ffffffff81740bbb > >> > > >> > After some looking I found that it's caused by the following code > >> > snippet in the 'interrupt' macro in entry_64.S: > >> > > >> > /* > >> > * Save previous stack pointer, optionally switch to interrupt stack. > >> > * irq_count is used to check if a CPU is already on an interrupt stack > >> > * or not. While this is essentially redundant with preempt_count it is > >> > * a little cheaper to use a separate counter in the PDA (short of > >> > * moving irq_enter into assembly, which would be too much work) > >> > */ > >> > movq %rsp, %rdi > >> > incl PER_CPU_VAR(irq_count) > >> > cmovzq PER_CPU_VAR(irq_stack_ptr), %rsp > >> > UNWIND_HINT_REGS base=rdi > >> > pushq %rdi > >> > UNWIND_HINT_REGS indirect=1 > >> > > >> > The problem is that it's changing the stack pointer *before* writing the > >> > previous stack pointer (push %rdi). So when unwinding from an NMI which > >> > hit between the rsp write and the rdi push, the unwinder tries to access > >> > the regs on the previous stack (by reading rdi), but the previous stack > >> > pointer isn't there yet, so the access is considered out of bounds. > >> > >> Ugh, that code. Does this problem go away with this patch applied: > >> > >> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/entry_ist&id=2231ec7e0bcc1a2bc94a17081511ab54cc6badd1 > >> > >> If so, want to update the patch for new kernels (shouldn't conflict > >> with anything except your unwind hints)? > > > > I don't think that patch will fix it, because it still updates rsp > > *before* writing the old rsp on the new stack. So there's still a > > window where the "previous stack" pointer is missing. > > But it's in a register. Is undwarf not able to grok that? Sorry, I didn't explain it very well. Undwarf can find the regs pointer in rdi, it just doesn't trust its value. See the stack_info.next_sp field, which is set in in_irq_stack(): /* * The next stack pointer is the first thing pushed by the entry code * after switching to the irq stack. */ info->next_sp = (unsigned long *)*(end - 1); It's a safety mechanism. The unwinder needs the last word of the irq stack page to point to the previous stack. That way it can double check that the stack pointer it calculates is within the bounds of either the current stack or the previous stack. In the above code, the previous stack pointer (or next stack pointer, depending on your perspective) hasn't been set up before it switches stacks. So the unwinder reads an uninitialized value into info->next_sp, and compares that with the regs pointer, and then stops the unwind because it thinks it went off into the weeds. -- Josh
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2017-06-30 01:00 +0200 |
| Subject | Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations |
| Message-ID | <tXQzM-5ww-39@gated-at.bofh.it> |
| In reply to | #1678228 |
--Andy > On Jun 29, 2017, at 2:41 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > >> On Thu, Jun 29, 2017 at 02:09:54PM -0700, Andy Lutomirski wrote: >>> On Thu, Jun 29, 2017 at 12:05 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: >>>> On Thu, Jun 29, 2017 at 11:50:18AM -0700, Andy Lutomirski wrote: >>>>> On Thu, Jun 29, 2017 at 10:53 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: >>>>> There's a bug here that will need a small change to the entry code. >>>>> >>>>> Mike Galbraith reported: >>>>> >>>>> WARNING: can't dereference registers at ffffc900089d7e08 for ip ffffffff81740bbb >>>>> >>>>> After some looking I found that it's caused by the following code >>>>> snippet in the 'interrupt' macro in entry_64.S: >>>>> >>>>> /* >>>>> * Save previous stack pointer, optionally switch to interrupt stack. >>>>> * irq_count is used to check if a CPU is already on an interrupt stack >>>>> * or not. While this is essentially redundant with preempt_count it is >>>>> * a little cheaper to use a separate counter in the PDA (short of >>>>> * moving irq_enter into assembly, which would be too much work) >>>>> */ >>>>> movq %rsp, %rdi >>>>> incl PER_CPU_VAR(irq_count) >>>>> cmovzq PER_CPU_VAR(irq_stack_ptr), %rsp >>>>> UNWIND_HINT_REGS base=rdi >>>>> pushq %rdi >>>>> UNWIND_HINT_REGS indirect=1 >>>>> >>>>> The problem is that it's changing the stack pointer *before* writing the >>>>> previous stack pointer (push %rdi). So when unwinding from an NMI which >>>>> hit between the rsp write and the rdi push, the unwinder tries to access >>>>> the regs on the previous stack (by reading rdi), but the previous stack >>>>> pointer isn't there yet, so the access is considered out of bounds. >>>> >>>> Ugh, that code. Does this problem go away with this patch applied: >>>> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/entry_ist&id=2231ec7e0bcc1a2bc94a17081511ab54cc6badd1 >>>> >>>> If so, want to update the patch for new kernels (shouldn't conflict >>>> with anything except your unwind hints)? >>> >>> I don't think that patch will fix it, because it still updates rsp >>> *before* writing the old rsp on the new stack. So there's still a >>> window where the "previous stack" pointer is missing. >> >> But it's in a register. Is undwarf not able to grok that? > > Sorry, I didn't explain it very well. Undwarf can find the regs pointer > in rdi, it just doesn't trust its value. > > See the stack_info.next_sp field, which is set in in_irq_stack(): > > /* > * The next stack pointer is the first thing pushed by the entry code > * after switching to the irq stack. > */ > info->next_sp = (unsigned long *)*(end - 1); > > It's a safety mechanism. The unwinder needs the last word of the irq > stack page to point to the previous stack. That way it can double check > that the stack pointer it calculates is within the bounds of either the > current stack or the previous stack. > > In the above code, the previous stack pointer (or next stack pointer, > depending on your perspective) hasn't been set up before it switches > stacks. So the unwinder reads an uninitialized value into > info->next_sp, and compares that with the regs pointer, and then stops > the unwind because it thinks it went off into the weeds. > That should be manageable, though, I think. With my patch applied (and maybe even without it), the only exception to that rule is if regs->sp points just above the top of the IRQ stack and the next instruction is push reg. In that case, the reg is exactly as trustworthy as the normal rule.* Can you teach the unwinding code that this is okay? * If an NMI hits right there, then it relies on unwinding out of the NMI correctly. But the usual checks that the target stack is a valid stack should prevent us from going off into the weeds regardless. > -- > Josh
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-06-30 04:20 +0200 |
| Subject | Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations |
| Message-ID | <tXTHj-7JM-7@gated-at.bofh.it> |
| In reply to | #1678268 |
On Thu, Jun 29, 2017 at 03:59:04PM -0700, Andy Lutomirski wrote: > > On Jun 29, 2017, at 2:41 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > >> On Thu, Jun 29, 2017 at 02:09:54PM -0700, Andy Lutomirski wrote: > >>> On Thu, Jun 29, 2017 at 12:05 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > >>>> On Thu, Jun 29, 2017 at 11:50:18AM -0700, Andy Lutomirski wrote: > >>>>> On Thu, Jun 29, 2017 at 10:53 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > >>>>> There's a bug here that will need a small change to the entry code. > >>>>> > >>>>> Mike Galbraith reported: > >>>>> > >>>>> WARNING: can't dereference registers at ffffc900089d7e08 for ip ffffffff81740bbb > >>>>> > >>>>> After some looking I found that it's caused by the following code > >>>>> snippet in the 'interrupt' macro in entry_64.S: > >>>>> > >>>>> /* > >>>>> * Save previous stack pointer, optionally switch to interrupt stack. > >>>>> * irq_count is used to check if a CPU is already on an interrupt stack > >>>>> * or not. While this is essentially redundant with preempt_count it is > >>>>> * a little cheaper to use a separate counter in the PDA (short of > >>>>> * moving irq_enter into assembly, which would be too much work) > >>>>> */ > >>>>> movq %rsp, %rdi > >>>>> incl PER_CPU_VAR(irq_count) > >>>>> cmovzq PER_CPU_VAR(irq_stack_ptr), %rsp > >>>>> UNWIND_HINT_REGS base=rdi > >>>>> pushq %rdi > >>>>> UNWIND_HINT_REGS indirect=1 > >>>>> > >>>>> The problem is that it's changing the stack pointer *before* writing the > >>>>> previous stack pointer (push %rdi). So when unwinding from an NMI which > >>>>> hit between the rsp write and the rdi push, the unwinder tries to access > >>>>> the regs on the previous stack (by reading rdi), but the previous stack > >>>>> pointer isn't there yet, so the access is considered out of bounds. > >>>> > >>>> Ugh, that code. Does this problem go away with this patch applied: > >>>> > >>>> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/entry_ist&id=2231ec7e0bcc1a2bc94a17081511ab54cc6badd1 > >>>> > >>>> If so, want to update the patch for new kernels (shouldn't conflict > >>>> with anything except your unwind hints)? > >>> > >>> I don't think that patch will fix it, because it still updates rsp > >>> *before* writing the old rsp on the new stack. So there's still a > >>> window where the "previous stack" pointer is missing. > >> > >> But it's in a register. Is undwarf not able to grok that? > > > > Sorry, I didn't explain it very well. Undwarf can find the regs pointer > > in rdi, it just doesn't trust its value. > > > > See the stack_info.next_sp field, which is set in in_irq_stack(): > > > > /* > > * The next stack pointer is the first thing pushed by the entry code > > * after switching to the irq stack. > > */ > > info->next_sp = (unsigned long *)*(end - 1); > > > > It's a safety mechanism. The unwinder needs the last word of the irq > > stack page to point to the previous stack. That way it can double check > > that the stack pointer it calculates is within the bounds of either the > > current stack or the previous stack. > > > > In the above code, the previous stack pointer (or next stack pointer, > > depending on your perspective) hasn't been set up before it switches > > stacks. So the unwinder reads an uninitialized value into > > info->next_sp, and compares that with the regs pointer, and then stops > > the unwind because it thinks it went off into the weeds. > > > > That should be manageable, though, I think. With my patch applied > (and maybe even without it), the only exception to that rule is if > regs->sp points just above the top of the IRQ stack and the next > instruction is push reg. In that case, the reg is exactly as > trustworthy as the normal rule.* Can you teach the unwinding code > that this is okay? > > * If an NMI hits right there, then it relies on unwinding out of the > NMI correctly. But the usual checks that the target stack is a valid > stack should prevent us from going off into the weeds regardless. But that would remove a safeguard against the undwarf data being corrupt. Sure, it would only affect the rare case where the stack pointer is at the top of the IRQ stack, but still... Also, the frame pointer and guess unwinders have the same issue, and this solution wouldn't work for them. And, worst of all, the oops stack dumping code in show_trace_log_lvl() also has this issue . It relies on those previous stack pointers. And it's separated from the unwinder logic by design, so it can't ask the unwinder where the next stack is. -- Josh
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@kernel.org> |
|---|---|
| Date | 2017-06-30 07:10 +0200 |
| Subject | Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations |
| Message-ID | <tXWlQ-15N-25@gated-at.bofh.it> |
| In reply to | #1678404 |
On Thu, Jun 29, 2017 at 7:12 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Thu, Jun 29, 2017 at 03:59:04PM -0700, Andy Lutomirski wrote:
>> >
>> > Sorry, I didn't explain it very well. Undwarf can find the regs pointer
>> > in rdi, it just doesn't trust its value.
>> >
>> > See the stack_info.next_sp field, which is set in in_irq_stack():
>> >
>> > /*
>> > * The next stack pointer is the first thing pushed by the entry code
>> > * after switching to the irq stack.
>> > */
>> > info->next_sp = (unsigned long *)*(end - 1);
>> >
>> > It's a safety mechanism. The unwinder needs the last word of the irq
>> > stack page to point to the previous stack. That way it can double check
>> > that the stack pointer it calculates is within the bounds of either the
>> > current stack or the previous stack.
>> >
>> > In the above code, the previous stack pointer (or next stack pointer,
>> > depending on your perspective) hasn't been set up before it switches
>> > stacks. So the unwinder reads an uninitialized value into
>> > info->next_sp, and compares that with the regs pointer, and then stops
>> > the unwind because it thinks it went off into the weeds.
>> >
>>
>> That should be manageable, though, I think. With my patch applied
>> (and maybe even without it), the only exception to that rule is if
>> regs->sp points just above the top of the IRQ stack and the next
>> instruction is push reg. In that case, the reg is exactly as
>> trustworthy as the normal rule.* Can you teach the unwinding code
>> that this is okay?
>>
>> * If an NMI hits right there, then it relies on unwinding out of the
>> NMI correctly. But the usual checks that the target stack is a valid
>> stack should prevent us from going off into the weeds regardless.
>
> But that would remove a safeguard against the undwarf data being
> corrupt. Sure, it would only affect the rare case where the stack
> pointer is at the top of the IRQ stack, but still...
>
> Also, the frame pointer and guess unwinders have the same issue, and
> this solution wouldn't work for them.
>
> And, worst of all, the oops stack dumping code in show_trace_log_lvl()
> also has this issue . It relies on those previous stack pointers. And
> it's separated from the unwinder logic by design, so it can't ask the
> unwinder where the next stack is.
Ugh.
I feel like we had this debate before, and I thought it was rather
silly that the unwinder cared. After all, we already have separate
safety mechanisms to make sure that the unwinder never wanders off of
the valid stacks and that it never touches any given stack more than
once. But it is indeed useful for the oops unwinder, so c'est la vie.
That being said, I bet we could get away with this (sorry for immense
whitespace damage):
.macro ENTER_IRQ_STACK old_rsp scratch_reg
DEBUG_ENTRY_ASSERT_IRQS_OFF
movq %rsp, \old_rsp
incl PER_CPU_VAR(irq_count)
jnz .Lrecurse_irq_stack_\@
/*
* Right now, we just incremented irq_count to zero, so we've
* claimed the IRQ stack but we haven't switched to it yet.
* Anything that can interrupt us here without using IST
* must be *extremely* careful to limit its stack usage.
*
* We write old_rsp to the IRQ stack before switching to
* %rsp for the benefit of the OOPS unwinder.
*/
movq PER_CPU_VAR(irq_stack_ptr), \scratch_reg
movq \old_rsp, -8(\scratch_reg)
leaq -8(\old_rsp), %rsp
jmp .Lout_\@
.Lrecurse_irq_stack_\@:
pushq \old_rsp
.Lout_\@:
.endm
After all, it looks like all the users have a scratch reg available.
Hmm. There's another option that might be considerably nicer, though:
put the IRQ stack at a known (at link time) position *in percpu
space*. (Presumably it already is -- I haven't checked.) Then we do:
.macro ENTER_IRQ_STACK old_rsp
DEBUG_ENTRY_ASSERT_IRQS_OFF
movq %rsp, \old_rsp
incl PER_CPU_VAR(irq_count)
/*
* Right now, if we just incremented irq_count to zero, we've
* claimed the IRQ stack but we haven't switched to it yet.
* Anything that can interrupt us here without using IST
* must be *extremely* careful to limit its stack usage.
*/
jnz .Lpush_old_rsp_\@
movq \old_rsp, PER_CPU_VAR(top_word_in_irq_stack)
movq PER_CPU_VAR(irq_stack_ptr), %rsp
.Lpush_old_rsp_\@:
pushq \old_rsp
.endm
This pushes the old pointer twice, but that's easy enough to fix if we
really cared. I think I like this variant better. What do you think?
--Andy
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web