LinuxLists.cc - [PATCH v4 0/3] Compile-time stack frame pointer validation

[permalink] [raw]

Subject: [PATCH v4 1/3] x86, stackvalidate: Compile-time stack frame pointer validation

Frame pointer based stack traces aren't always reliable. One big reason
is that most asm functions don't set up the frame pointer.

Fix that by enforcing that all asm functions honor CONFIG_FRAME_POINTER.
This is done with a new stackvalidate host tool which is automatically
run for every compiled .S file and which validates that every asm
function does the proper frame pointer setup.

Also, to make sure somebody didn't forget to annotate their callable asm code
as a function, flag an error for any return instructions which are hiding
outside of a function. In almost all cases, return instructions are part of
callable functions and should be annotated as such so that we can validate
their frame pointer usage. A whitelist mechanism exists for those few return
instructions which are not actually in callable code.

It currently only supports x86_64. It *almost* supports x86_32, but the
stackvalidate code doesn't yet know how to deal with 32-bit REL
relocations for the return whitelists. I tried to make the code generic
so that support for other architectures can be plugged in pretty easily.

As a first step, CONFIG_STACK_VALIDATION is disabled by default, and all
reported non-compliances result in warnings. Right now I'm seeing 200+
warnings. Once we get them all cleaned up, we can change the default to
CONFIG_STACK_VALIDATION=y and change the warnings to build errors so the
asm code can stay clean.

Signed-off-by: Josh Poimboeuf <[email protected]>
Acked-by: Michal Marek <[email protected]>
---
MAINTAINERS | 6 +
arch/Kconfig | 3 +
arch/x86/Kconfig | 1 +
arch/x86/Makefile | 6 +-
lib/Kconfig.debug | 11 ++
scripts/Makefile | 1 +
scripts/Makefile.build | 22 ++-
scripts/stackvalidate/Makefile | 17 ++
scripts/stackvalidate/arch-x86.c | 134 +++++++++++++
scripts/stackvalidate/arch.h | 10 +
scripts/stackvalidate/elf.c | 352 ++++++++++++++++++++++++++++++++++
scripts/stackvalidate/elf.h | 56 ++++++
scripts/stackvalidate/list.h | 217 +++++++++++++++++++++
scripts/stackvalidate/stackvalidate.c | 226 ++++++++++++++++++++++
14 files changed, 1059 insertions(+), 3 deletions(-)
create mode 100644 scripts/stackvalidate/Makefile
create mode 100644 scripts/stackvalidate/arch-x86.c
create mode 100644 scripts/stackvalidate/arch.h
create mode 100644 scripts/stackvalidate/elf.c
create mode 100644 scripts/stackvalidate/elf.h
create mode 100644 scripts/stackvalidate/list.h
create mode 100644 scripts/stackvalidate/stackvalidate.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 78ea7b6..6d700bf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9451,6 +9451,12 @@ L: [email protected]
S: Supported
F: Documentation/stable_kernel_rules.txt

+STACK VALIDATION
+M: Josh Poimboeuf <[email protected]>
+S: Supported
+F: scripts/stackvalidate/
+F: arch/x86/include/asm/func.h
+
STAGING SUBSYSTEM
M: Greg Kroah-Hartman <[email protected]>
T: git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
diff --git a/arch/Kconfig b/arch/Kconfig
index bec6666..a5c3f50 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -506,6 +506,9 @@ config HAVE_COPY_THREAD_TLS
normal C parameter passing, rather than extracting the syscall
argument from pt_regs.

+config HAVE_STACK_VALIDATION
+ bool
+
#
# ABI hall of shame
#
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c92fdcc..d60a2378a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -146,6 +146,7 @@ config X86
select ACPI_LEGACY_TABLES_LOOKUP if ACPI
select X86_FEATURE_NAMES if PROC_FS
select SRCU
+ select HAVE_STACK_VALIDATION if FRAME_POINTER && X86_64

config INSTRUCTION_DECODER
def_bool y
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 57996ee..c5598a0 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -180,9 +180,13 @@ KBUILD_CFLAGS += $(call cc-option,-mno-avx,)
KBUILD_CFLAGS += $(mflags-y)
KBUILD_AFLAGS += $(mflags-y)

-archscripts: scripts_basic
+archscripts: scripts_basic $(objtree)/arch/x86/lib/inat-tables.c
$(Q)$(MAKE) $(build)=arch/x86/tools relocs

+# this file is needed early by scripts/stackvalidate
+$(objtree)/arch/x86/lib/inat-tables.c:
+ $(Q)$(MAKE) $(build)=arch/x86/lib $@
+
###
# Syscall table generation

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index eb3997b..7bfaf80 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -332,6 +332,17 @@ config FRAME_POINTER
larger and slower, but it gives very useful debugging information
in case of kernel bugs. (precise oopses/stacktraces/warnings)

+
+config STACK_VALIDATION
+ bool "Enable kernel stack validation"
+ depends on HAVE_STACK_VALIDATION
+ default n
+ help
+ Add compile-time validations which help make kernel stack traces more
+ reliable. This includes checks to ensure that assembly functions
+ save, update and restore the frame pointer or the back chain pointer.
+
+
config DEBUG_FORCE_WEAK_PER_CPU
bool "Force weak per-cpu definitions"
depends on DEBUG_KERNEL
diff --git a/scripts/Makefile b/scripts/Makefile
index 2016a64..c882a91 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -37,6 +37,7 @@ subdir-y += mod
subdir-$(CONFIG_SECURITY_SELINUX) += selinux
subdir-$(CONFIG_DTC) += dtc
subdir-$(CONFIG_GDB_SCRIPTS) += gdb
+subdir-$(CONFIG_STACK_VALIDATION) += stackvalidate

# Let clean descend into subdirs
subdir- += basic kconfig package
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 01df30a..3b05833 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -253,6 +253,24 @@ define rule_cc_o_c
mv -f $(dot-target).tmp $(dot-target).cmd
endef

+ifdef CONFIG_STACK_VALIDATION
+stackvalidate = $(objtree)/scripts/stackvalidate/stackvalidate
+cmd_stackvalidate = \
+ case $(@) in \
+ arch/x86/purgatory/*) ;; \
+ *) $(stackvalidate) "$(@)"; \
+ esac;
+endif
+
+define rule_as_o_S
+ $(call echo-cmd,as_o_S) $(cmd_as_o_S); \
+ $(cmd_stackvalidate) \
+ scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,as_o_S)' > \
+ $(dot-target).tmp; \
+ rm -f $(depfile); \
+ mv -f $(dot-target).tmp $(dot-target).cmd
+endef
+
# Built-in and composite module parts
$(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
$(call cmd,force_checksrc)
@@ -290,8 +308,8 @@ $(obj)/%.s: $(src)/%.S FORCE
quiet_cmd_as_o_S = AS $(quiet_modtag) $@
cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $<

-$(obj)/%.o: $(src)/%.S FORCE
- $(call if_changed_dep,as_o_S)
+$(obj)/%.o: $(src)/%.S $(stackvalidate) FORCE
+ $(call if_changed_rule,as_o_S)

targets += $(real-objs-y) $(real-objs-m) $(lib-y)
targets += $(extra-y) $(MAKECMDGOALS) $(always)
diff --git a/scripts/stackvalidate/Makefile b/scripts/stackvalidate/Makefile
new file mode 100644
index 0000000..6027ec4
--- /dev/null
+++ b/scripts/stackvalidate/Makefile
@@ -0,0 +1,17 @@
+hostprogs-y := stackvalidate
+always := $(hostprogs-y)
+
+stackvalidate-objs := stackvalidate.o elf.o
+
+HOSTCFLAGS += -Werror
+HOSTLOADLIBES_stackvalidate := -lelf
+
+ifdef CONFIG_X86
+
+stackvalidate-objs += arch-x86.o
+
+HOSTCFLAGS_arch-x86.o := -I$(objtree)/arch/x86/lib/ -I$(srctree)/arch/x86/include/ -I$(srctree)/arch/x86/lib/
+
+$(obj)/arch-x86.o: $(srctree)/arch/x86/lib/insn.c $(srctree)/arch/x86/lib/inat.c $(srctree)/arch/x86/include/asm/inat_types.h $(srctree)/arch/x86/include/asm/inat.h $(srctree)/arch/x86/include/asm/insn.h $(objtree)/arch/x86/lib/inat-tables.c
+
+endif
diff --git a/scripts/stackvalidate/arch-x86.c b/scripts/stackvalidate/arch-x86.c
new file mode 100644
index 0000000..fbc0756
--- /dev/null
+++ b/scripts/stackvalidate/arch-x86.c
@@ -0,0 +1,134 @@
+#include <stdio.h>
+
+#define unlikely(cond) (cond)
+#include <asm/insn.h>
+#include <inat.c>
+#include <insn.c>
+
+#include "elf.h"
+#include "arch.h"
+
+static int is_x86_64(struct elf *elf)
+{
+ switch (elf->ehdr.e_machine) {
+ case EM_X86_64:
+ return 1;
+ case EM_386:
+ return 0;
+ default:
+ WARN("unexpected ELF machine type %d", elf->ehdr.e_machine);
+ return -1;
+ }
+}
+
+/*
+ * arch_validate_function() - Ensures the given asm function saves, sets up,
+ * and restores the frame pointer.
+ *
+ * The frame pointer prologue/epilogue should look something like:
+ *
+ * push %rbp
+ * mov %rsp, %rbp
+ * [ function body ]
+ * pop %rbp
+ * ret
+ *
+ * Return value:
+ * -1: bad instruction
+ * 1: missing frame pointer logic
+ * 0: validation succeeded
+ */
+int arch_validate_function(struct elf *elf, struct symbol *func)
+{
+ struct insn insn;
+ unsigned long addr, length;
+ int push, mov, pop, ret, x86_64;
+
+ push = mov = pop = ret = 0;
+
+ x86_64 = is_x86_64(elf);
+ if (x86_64 == -1)
+ return -1;
+
+ for (addr = func->start; addr < func->end; addr += length) {
+ insn_init(&insn, (void *)addr, func->end - addr, x86_64);
+ insn_get_length(&insn);
+ length = insn.length;
+ insn_get_opcode(&insn);
+ if (!length || !insn.opcode.got) {
+ WARN("%s+0x%lx: bad instruction", func->name,
+ addr - func->start);
+ return -1;
+ }
+
+ switch (insn.opcode.bytes[0]) {
+ case 0x55:
+ if (!insn.rex_prefix.nbytes)
+ /* push bp */
+ push++;
+ break;
+ case 0x5d:
+ if (!insn.rex_prefix.nbytes)
+ /* pop bp */
+ pop++;
+ break;
+ case 0xc9: /* leave */
+ pop++;
+ break;
+ case 0x89:
+ insn_get_modrm(&insn);
+ if (insn.modrm.bytes[0] == 0xe5)
+ /* mov sp, bp */
+ mov++;
+ break;
+ case 0xc3: /* ret */
+ ret++;
+ break;
+ }
+ }
+
+ if (push != 1 || mov != 1 || !pop || !ret || pop != ret) {
+ WARN("%s() is missing frame pointer logic. Please use FUNC_ENTER.",
+ func->name);
+ return 1;
+ }
+
+ return 0;
+}
+
+/*
+ * arch_is_return_insn() - Determines whether the instruction at the given
+ * address is a return instruction. Also returns the instruction length in
+ * *len.
+ *
+ * Return value:
+ * -1: bad instruction
+ * 0: no, it's not a return instruction
+ * 1: yes, it's a return instruction
+ */
+int arch_is_return_insn(struct elf *elf, unsigned long addr,
+ unsigned int maxlen, unsigned int *len)
+{
+ struct insn insn;
+ int x86_64;
+
+ x86_64 = is_x86_64(elf);
+ if (x86_64 == -1)
+ return -1;
+
+ insn_init(&insn, (void *)addr, maxlen, x86_64);
+ insn_get_length(&insn);
+ insn_get_opcode(&insn);
+ if (!insn.opcode.got)
+ return -1;
+
+ *len = insn.length;
+
+ switch (insn.opcode.bytes[0]) {
+ case 0xc2: case 0xc3: /* ret near */
+ case 0xca: case 0xcb: /* ret far */
+ return 1;
+ }
+
+ return 0;
+}
diff --git a/scripts/stackvalidate/arch.h b/scripts/stackvalidate/arch.h
new file mode 100644
index 0000000..3b91b1c
--- /dev/null
+++ b/scripts/stackvalidate/arch.h
@@ -0,0 +1,10 @@
+#ifndef _ARCH_H_
+#define _ARCH_H_
+
+#include "elf.h"
+
+int arch_validate_function(struct elf *elf, struct symbol *func);
+int arch_is_return_insn(struct elf *elf, unsigned long addr,
+ unsigned int maxlen, unsigned int *len);
+
+#endif /* _ARCH_H_ */
diff --git a/scripts/stackvalidate/elf.c b/scripts/stackvalidate/elf.c
new file mode 100644
index 0000000..a1419a5
--- /dev/null
+++ b/scripts/stackvalidate/elf.c
@@ -0,0 +1,352 @@
+/*
+ * elf.c - ELF access library
+ *
+ * Adapted from kpatch (https://github.com/dynup/kpatch):
+ * Copyright (C) 2013-2015 Josh Poimboeuf <[email protected]>
+ * Copyright (C) 2014 Seth Jennings <[email protected]>
+ *
+ * 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 <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "elf.h"
+
+struct section *elf_find_section_by_name(struct elf *elf, const char *name)
+{
+ struct section *sec;
+
+ list_for_each_entry(sec, &elf->sections, list)
+ if (!strcmp(sec->name, name))
+ return sec;
+
+ return NULL;
+}
+
+static struct section *elf_find_section_by_index(struct elf *elf,
+ unsigned int index)
+{
+ struct section *sec;
+
+ list_for_each_entry(sec, &elf->sections, list)
+ if (sec->index == index)
+ return sec;
+
+ return NULL;
+}
+
+static struct symbol *elf_find_symbol_by_index(struct elf *elf,
+ unsigned int index)
+{
+ struct section *sec;
+ struct symbol *sym;
+
+ list_for_each_entry(sec, &elf->sections, list)
+ list_for_each_entry(sym, &sec->symbols, list)
+ if (sym->index == index)
+ return sym;
+
+ return NULL;
+}
+
+static int elf_read_sections(struct elf *elf)
+{
+ Elf_Scn *s = NULL;
+ struct section *sec;
+ size_t shstrndx, sections_nr;
+ int i;
+
+ if (elf_getshdrnum(elf->elf, &sections_nr)) {
+ perror("elf_getshdrnum");
+ return -1;
+ }
+
+ if (elf_getshdrstrndx(elf->elf, &shstrndx)) {
+ perror("elf_getshdrstrndx");
+ return -1;
+ }
+
+ for (i = 0; i < sections_nr; i++) {
+ sec = malloc(sizeof(*sec));
+ if (!sec) {
+ perror("malloc");
+ return -1;
+ }
+ memset(sec, 0, sizeof(*sec));
+
+ INIT_LIST_HEAD(&sec->symbols);
+ INIT_LIST_HEAD(&sec->relas);
+
+ list_add_tail(&sec->list, &elf->sections);
+
+ s = elf_getscn(elf->elf, i);
+ if (!s) {
+ perror("elf_getscn");
+ return -1;
+ }
+
+ sec->index = elf_ndxscn(s);
+
+ if (!gelf_getshdr(s, &sec->sh)) {
+ perror("gelf_getshdr");
+ return -1;
+ }
+
+ sec->name = elf_strptr(elf->elf, shstrndx, sec->sh.sh_name);
+ if (!sec->name) {
+ perror("elf_strptr");
+ return -1;
+ }
+
+ sec->data = elf_getdata(s, NULL);
+ if (!sec->data) {
+ perror("elf_getdata");
+ return -1;
+ }
+
+ if (sec->data->d_off != 0 ||
+ sec->data->d_size != sec->sh.sh_size) {
+ WARN("unexpected data attributes for %s", sec->name);
+ return -1;
+ }
+
+ sec->start = (unsigned long)sec->data->d_buf;
+ sec->end = sec->start + sec->data->d_size;
+ }
+
+ /* sanity check, one more call to elf_nextscn() should return NULL */
+ if (elf_nextscn(elf->elf, s)) {
+ WARN("section entry mismatch");
+ return -1;
+ }
+
+ return 0;
+}
+
+static int elf_read_symbols(struct elf *elf)
+{
+ struct section *symtab;
+ struct symbol *sym;
+ struct list_head *entry, *tmp;
+ int symbols_nr, i;
+
+ symtab = elf_find_section_by_name(elf, ".symtab");
+ if (!symtab) {
+ WARN("missing symbol table");
+ return -1;
+ }
+
+ symbols_nr = symtab->sh.sh_size / symtab->sh.sh_entsize;
+
+ for (i = 0; i < symbols_nr; i++) {
+ sym = malloc(sizeof(*sym));
+ if (!sym) {
+ perror("malloc");
+ return -1;
+ }
+ memset(sym, 0, sizeof(*sym));
+
+ sym->index = i;
+
+ if (!gelf_getsym(symtab->data, i, &sym->sym)) {
+ perror("gelf_getsym");
+ goto err;
+ }
+
+ sym->name = elf_strptr(elf->elf, symtab->sh.sh_link,
+ sym->sym.st_name);
+ if (!sym->name) {
+ perror("elf_strptr");
+ goto err;
+ }
+
+ sym->type = GELF_ST_TYPE(sym->sym.st_info);
+ sym->bind = GELF_ST_BIND(sym->sym.st_info);
+
+ if (sym->sym.st_shndx > SHN_UNDEF &&
+ sym->sym.st_shndx < SHN_LORESERVE) {
+ sym->sec = elf_find_section_by_index(elf,
+ sym->sym.st_shndx);
+ if (!sym->sec) {
+ WARN("couldn't find section for symbol %s",
+ sym->name);
+ goto err;
+ }
+ if (sym->type == STT_SECTION)
+ sym->name = sym->sec->name;
+ } else
+ sym->sec = elf_find_section_by_index(elf, 0);
+
+ sym->start = sym->sec->start + sym->sym.st_value;
+ sym->end = sym->start + sym->sym.st_size;
+
+ /* sorted insert into a per-section list */
+ entry = &sym->sec->symbols;
+ list_for_each_prev(tmp, &sym->sec->symbols) {
+ struct symbol *s;
+
+ s = list_entry(tmp, struct symbol, list);
+
+ if (sym->start > s->start) {
+ entry = tmp;
+ break;
+ }
+
+ if (sym->start == s->start && sym->end >= s->end) {
+ entry = tmp;
+ break;
+ }
+ }
+ list_add(&sym->list, entry);
+ }
+
+ return 0;
+
+err:
+ free(sym);
+ return -1;
+}
+
+static int elf_read_relas(struct elf *elf)
+{
+ struct section *sec;
+ struct rela *rela;
+ int i;
+ unsigned int symndx;
+
+ list_for_each_entry(sec, &elf->sections, list) {
+ if (sec->sh.sh_type != SHT_RELA)
+ continue;
+
+ sec->base = elf_find_section_by_name(elf, sec->name + 5);
+ if (!sec->base) {
+ WARN("can't find base section for rela section %s",
+ sec->name);
+ return -1;
+ }
+
+ sec->base->rela = sec;
+
+ for (i = 0; i < sec->sh.sh_size / sec->sh.sh_entsize; i++) {
+ rela = malloc(sizeof(*rela));
+ if (!rela) {
+ perror("malloc");
+ return -1;
+ }
+ memset(rela, 0, sizeof(*rela));
+
+ list_add_tail(&rela->list, &sec->relas);
+
+ if (!gelf_getrela(sec->data, i, &rela->rela)) {
+ perror("gelf_getrela");
+ return -1;
+ }
+
+ rela->type = GELF_R_TYPE(rela->rela.r_info);
+ rela->addend = rela->rela.r_addend;
+ rela->offset = rela->rela.r_offset;
+ symndx = GELF_R_SYM(rela->rela.r_info);
+ rela->sym = elf_find_symbol_by_index(elf, symndx);
+ if (!rela->sym) {
+ WARN("can't find rela entry symbol %d for %s",
+ symndx, sec->name);
+ return -1;
+ }
+ }
+ }
+
+ return 0;
+}
+
+struct elf *elf_open(const char *name)
+{
+ struct elf *elf;
+
+ elf_version(EV_CURRENT);
+
+ elf = malloc(sizeof(*elf));
+ if (!elf) {
+ perror("malloc");
+ return NULL;
+ }
+ memset(elf, 0, sizeof(*elf));
+
+ INIT_LIST_HEAD(&elf->sections);
+
+ elf->name = strdup(name);
+ if (!elf->name) {
+ perror("strdup");
+ goto err;
+ }
+
+ elf->fd = open(name, O_RDONLY);
+ if (elf->fd == -1) {
+ perror("open");
+ goto err;
+ }
+
+ elf->elf = elf_begin(elf->fd, ELF_C_READ_MMAP, NULL);
+ if (!elf->elf) {
+ perror("elf_begin");
+ goto err;
+ }
+
+ if (!gelf_getehdr(elf->elf, &elf->ehdr)) {
+ perror("gelf_getehdr");
+ goto err;
+ }
+
+ if (elf_read_sections(elf))
+ goto err;
+
+ if (elf_read_symbols(elf))
+ goto err;
+
+ if (elf_read_relas(elf))
+ goto err;
+
+ return elf;
+
+err:
+ elf_close(elf);
+ return NULL;
+}
+
+void elf_close(struct elf *elf)
+{
+ struct section *sec, *tmpsec;
+ struct symbol *sym, *tmpsym;
+
+ list_for_each_entry_safe(sec, tmpsec, &elf->sections, list) {
+ list_for_each_entry_safe(sym, tmpsym, &sec->symbols, list) {
+ list_del(&sym->list);
+ free(sym);
+ }
+ list_del(&sec->list);
+ free(sec);
+ }
+ if (elf->name)
+ free(elf->name);
+ if (elf->fd > 0)
+ close(elf->fd);
+ if (elf->elf)
+ elf_end(elf->elf);
+ free(elf);
+}
diff --git a/scripts/stackvalidate/elf.h b/scripts/stackvalidate/elf.h
new file mode 100644
index 0000000..db5d5fa
--- /dev/null
+++ b/scripts/stackvalidate/elf.h
@@ -0,0 +1,56 @@
+#ifndef _ELF_H_
+#define _ELF_H_
+
+#include <gelf.h>
+#include "list.h"
+
+#define WARN(format, ...) \
+ fprintf(stderr, \
+ "%s: " format "\n", \
+ elf->name, ##__VA_ARGS__)
+
+struct section {
+ struct list_head list;
+ GElf_Shdr sh;
+ struct list_head symbols;
+ struct list_head relas;
+ struct section *base, *rela;
+ Elf_Data *data;
+ char *name;
+ int index;
+ unsigned long start, end;
+};
+
+struct symbol {
+ struct list_head list;
+ GElf_Sym sym;
+ struct section *sec;
+ char *name;
+ int index;
+ unsigned char bind, type;
+ unsigned long start, end;
+};
+
+struct rela {
+ struct list_head list;
+ GElf_Rela rela;
+ struct symbol *sym;
+ unsigned int type;
+ int offset;
+ int addend;
+};
+
+struct elf {
+ Elf *elf;
+ GElf_Ehdr ehdr;
+ int fd;
+ char *name;
+ struct list_head sections;
+};
+
+
+struct elf *elf_open(const char *name);
+struct section *elf_find_section_by_name(struct elf *elf, const char *name);
+void elf_close(struct elf *elf);
+
+#endif /* _ELF_H_ */
diff --git a/scripts/stackvalidate/list.h b/scripts/stackvalidate/list.h
new file mode 100644
index 0000000..25716b5
--- /dev/null
+++ b/scripts/stackvalidate/list.h
@@ -0,0 +1,217 @@
+#ifndef _LIST_H
+#define _LIST_H
+
+#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
+
+#define container_of(ptr, type, member) ({ \
+ const typeof(((type *)0)->member) *__mptr = (ptr); \
+ (type *)((char *)__mptr - offsetof(type, member)); })
+
+#define LIST_POISON1 ((void *) 0x00100100)
+#define LIST_POISON2 ((void *) 0x00200200)
+
+struct list_head {
+ struct list_head *next, *prev;
+};
+
+#define LIST_HEAD_INIT(name) { &(name), &(name) }
+
+#define LIST_HEAD(name) \
+ struct list_head name = LIST_HEAD_INIT(name)
+
+static inline void INIT_LIST_HEAD(struct list_head *list)
+{
+ list->next = list;
+ list->prev = list;
+}
+
+static inline void __list_add(struct list_head *new,
+ struct list_head *prev,
+ struct list_head *next)
+{
+ next->prev = new;
+ new->next = next;
+ new->prev = prev;
+ prev->next = new;
+}
+
+static inline void list_add(struct list_head *new, struct list_head *head)
+{
+ __list_add(new, head, head->next);
+}
+
+static inline void list_add_tail(struct list_head *new, struct list_head *head)
+{
+ __list_add(new, head->prev, head);
+}
+
+static inline void __list_del(struct list_head *prev, struct list_head *next)
+{
+ next->prev = prev;
+ prev->next = next;
+}
+
+static inline void __list_del_entry(struct list_head *entry)
+{
+ __list_del(entry->prev, entry->next);
+}
+
+static inline void list_del(struct list_head *entry)
+{
+ __list_del(entry->prev, entry->next);
+ entry->next = LIST_POISON1;
+ entry->prev = LIST_POISON2;
+}
+
+static inline void list_replace(struct list_head *old,
+ struct list_head *new)
+{
+ new->next = old->next;
+ new->next->prev = new;
+ new->prev = old->prev;
+ new->prev->next = new;
+}
+
+static inline void list_replace_init(struct list_head *old,
+ struct list_head *new)
+{
+ list_replace(old, new);
+ INIT_LIST_HEAD(old);
+}
+
+static inline void list_del_init(struct list_head *entry)
+{
+ __list_del_entry(entry);
+ INIT_LIST_HEAD(entry);
+}
+
+static inline void list_move(struct list_head *list, struct list_head *head)
+{
+ __list_del_entry(list);
+ list_add(list, head);
+}
+
+static inline void list_move_tail(struct list_head *list,
+ struct list_head *head)
+{
+ __list_del_entry(list);
+ list_add_tail(list, head);
+}
+
+static inline int list_is_last(const struct list_head *list,
+ const struct list_head *head)
+{
+ return list->next == head;
+}
+
+static inline int list_empty(const struct list_head *head)
+{
+ return head->next == head;
+}
+
+static inline int list_empty_careful(const struct list_head *head)
+{
+ struct list_head *next = head->next;
+
+ return (next == head) && (next == head->prev);
+}
+
+static inline void list_rotate_left(struct list_head *head)
+{
+ struct list_head *first;
+
+ if (!list_empty(head)) {
+ first = head->next;
+ list_move_tail(first, head);
+ }
+}
+
+static inline int list_is_singular(const struct list_head *head)
+{
+ return !list_empty(head) && (head->next == head->prev);
+}
+
+#define list_entry(ptr, type, member) \
+ container_of(ptr, type, member)
+
+#define list_first_entry(ptr, type, member) \
+ list_entry((ptr)->next, type, member)
+
+#define list_last_entry(ptr, type, member) \
+ list_entry((ptr)->prev, type, member)
+
+#define list_first_entry_or_null(ptr, type, member) \
+ (!list_empty(ptr) ? list_first_entry(ptr, type, member) : NULL)
+
+#define list_next_entry(pos, member) \
+ list_entry((pos)->member.next, typeof(*(pos)), member)
+
+#define list_prev_entry(pos, member) \
+ list_entry((pos)->member.prev, typeof(*(pos)), member)
+
+#define list_for_each(pos, head) \
+ for (pos = (head)->next; pos != (head); pos = pos->next)
+
+#define list_for_each_prev(pos, head) \
+ for (pos = (head)->prev; pos != (head); pos = pos->prev)
+
+#define list_for_each_safe(pos, n, head) \
+ for (pos = (head)->next, n = pos->next; pos != (head); \
+ pos = n, n = pos->next)
+
+#define list_for_each_prev_safe(pos, n, head) \
+ for (pos = (head)->prev, n = pos->prev; \
+ pos != (head); \
+ pos = n, n = pos->prev)
+
+#define list_for_each_entry(pos, head, member) \
+ for (pos = list_first_entry(head, typeof(*pos), member); \
+ &pos->member != (head); \
+ pos = list_next_entry(pos, member))
+
+#define list_for_each_entry_reverse(pos, head, member) \
+ for (pos = list_last_entry(head, typeof(*pos), member); \
+ &pos->member != (head); \
+ pos = list_prev_entry(pos, member))
+
+#define list_prepare_entry(pos, head, member) \
+ ((pos) ? : list_entry(head, typeof(*pos), member))
+
+#define list_for_each_entry_continue(pos, head, member) \
+ for (pos = list_next_entry(pos, member); \
+ &pos->member != (head); \
+ pos = list_next_entry(pos, member))
+
+#define list_for_each_entry_continue_reverse(pos, head, member) \
+ for (pos = list_prev_entry(pos, member); \
+ &pos->member != (head); \
+ pos = list_prev_entry(pos, member))
+
+#define list_for_each_entry_from(pos, head, member) \
+ for (; &pos->member != (head); \
+ pos = list_next_entry(pos, member))
+
+#define list_for_each_entry_safe(pos, n, head, member) \
+ for (pos = list_first_entry(head, typeof(*pos), member), \
+ n = list_next_entry(pos, member); \
+ &pos->member != (head); \
+ pos = n, n = list_next_entry(n, member))
+
+#define list_for_each_entry_safe_continue(pos, n, head, member) \
+ for (pos = list_next_entry(pos, member), \
+ n = list_next_entry(pos, member); \
+ &pos->member != (head); \
+ pos = n, n = list_next_entry(n, member))
+
+#define list_for_each_entry_safe_from(pos, n, head, member) \
+ for (n = list_next_entry(pos, member); \
+ &pos->member != (head); \
+ pos = n, n = list_next_entry(n, member))
+
+#define list_for_each_entry_safe_reverse(pos, n, head, member) \
+ for (pos = list_last_entry(head, typeof(*pos), member), \
+ n = list_prev_entry(pos, member); \
+ &pos->member != (head); \
+ pos = n, n = list_prev_entry(n, member))
+
+#endif /* _LIST_H */
diff --git a/scripts/stackvalidate/stackvalidate.c b/scripts/stackvalidate/stackvalidate.c
new file mode 100644
index 0000000..07f1110
--- /dev/null
+++ b/scripts/stackvalidate/stackvalidate.c
@@ -0,0 +1,226 @@
+/*
+ * stackvalidate.c
+ *
+ * Copyright (C) 2015 Josh Poimboeuf <[email protected]>
+ *
+ * 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/>.
+ */
+
+/*
+ * This tool automatically runs for every compiled .S file and validates that
+ * every asm function does the proper frame pointer setup.
+ *
+ * Also, to make sure somebody didn't forget to annotate their callable asm
+ * code as a function (e.g. via the FUNC_ENTER/FUNC_RETURN macros), it flags an
+ * error for any return instructions which are hiding outside of a function.
+ * In almost all cases, return instructions are part of callable functions and
+ * should be annotated as such so that we can validate their frame pointer
+ * usage.
+ *
+ * Whitelist mechanisms exist (RET_NOVALIDATE and FILE_NOVALIDATE) for those
+ * few return instructions which are not actually in callable code.
+ */
+
+#include <argp.h>
+#include <stdbool.h>
+
+#include "elf.h"
+#include "arch.h"
+
+int warnings;
+
+struct args {
+ char *args[1];
+};
+static const char args_doc[] = "file.o";
+static struct argp_option options[] = {
+ {0},
+};
+static error_t parse_opt(int key, char *arg, struct argp_state *state)
+{
+ /* Get the input argument from argp_parse, which we
+ know is a pointer to our args structure. */
+ struct args *args = state->input;
+
+ switch (key) {
+ case ARGP_KEY_ARG:
+ if (state->arg_num >= 1)
+ /* Too many arguments. */
+ argp_usage(state);
+ args->args[state->arg_num] = arg;
+ break;
+ case ARGP_KEY_END:
+ if (state->arg_num < 1)
+ /* Not enough arguments. */
+ argp_usage(state);
+ break;
+ default:
+ return ARGP_ERR_UNKNOWN;
+ }
+ return 0;
+}
+static struct argp argp = { options, parse_opt, args_doc, 0 };
+
+/*
+ * Check for the RET_NOVALIDATE macro.
+ */
+static bool is_ret_whitelisted(struct elf *elf, struct section *sec,
+ unsigned long offset)
+{
+ struct section *wlsec;
+ struct rela *rela;
+
+ wlsec = elf_find_section_by_name(elf,
+ ".rela__stackvalidate_whitelist_ret");
+ if (!wlsec)
+ return false;
+
+ list_for_each_entry(rela, &wlsec->relas, list)
+ if (rela->sym->type == STT_SECTION &&
+ rela->sym->index == sec->index && rela->addend == offset)
+ return true;
+
+ return false;
+}
+
+/*
+ * Check for the FILE_NOVALIDATE macro.
+ */
+static bool is_file_whitelisted(struct elf *elf)
+{
+ if (elf_find_section_by_name(elf, "__stackvalidate_whitelist_file"))
+ return true;
+
+ return false;
+}
+
+/*
+ * For the given collection of instructions which are outside an STT_FUNC
+ * function, ensure there are no (whitelisted) return instructions.
+ */
+static int validate_nonfunction(struct elf *elf, struct section *sec,
+ unsigned long start, unsigned long end)
+{
+ unsigned long addr;
+ unsigned int len;
+ int ret, warnings = 0;
+
+ for (addr = start; addr < end; addr += len) {
+ ret = arch_is_return_insn(elf, addr, end - addr, &len);
+ if (ret == -1)
+ return -1;
+
+ if (ret && !is_ret_whitelisted(elf, sec, addr - sec->start)) {
+ WARN("return instruction outside of a function at %s+0x%lx. Please use FUNC_ENTER.",
+ sec->name, addr - sec->start);
+ warnings++;
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * For the given section, ensure that:
+ *
+ * 1) all STT_FUNC functions do the proper frame pointer setup; and
+ * 2) any other instructions outside of STT_FUNC aren't return instructions
+ * (unless they're annotated with the RET_NOVALIDATE macro).
+ */
+static int validate_section(struct elf *elf, struct section *sec)
+{
+ struct symbol *func, *last_func;
+ struct symbol null_func = {};
+ int ret, warnings = 0;
+
+ if (!(sec->sh.sh_flags & SHF_EXECINSTR))
+ return 0;
+
+ if (list_empty(&sec->symbols)) {
+ WARN("%s: no symbols", sec->name);
+ return -1;
+ }
+
+ last_func = &null_func;
+ last_func->start = last_func->end = sec->start;
+ list_for_each_entry(func, &sec->symbols, list) {
+ if (func->type != STT_FUNC)
+ continue;
+
+ if (func->start != last_func->start &&
+ func->end != last_func->end &&
+ func->start < last_func->end) {
+ WARN("overlapping functions %s and %s",
+ last_func->name, func->name);
+ warnings++;
+ }
+
+ if (func->start > last_func->end) {
+ ret = validate_nonfunction(elf, sec, last_func->end,
+ func->start);
+ if (ret < 0)
+ return -1;
+
+ warnings += ret;
+ }
+
+ ret = arch_validate_function(elf, func);
+ if (ret < 0)
+ return -1;
+
+ warnings += ret;
+
+ last_func = func;
+ }
+
+ if (last_func->end < sec->end) {
+ ret = validate_nonfunction(elf, sec, last_func->end, sec->end);
+ if (ret < 0)
+ return -1;
+
+ warnings += ret;
+ }
+
+ return warnings;
+}
+
+int main(int argc, char *argv[])
+{
+ struct args args;
+ struct elf *elf;
+ struct section *sec;
+ int ret, warnings = 0;
+
+ argp_parse(&argp, argc, argv, 0, 0, &args);
+
+ elf = elf_open(args.args[0]);
+ if (!elf) {
+ fprintf(stderr, "error reading elf file %s\n", args.args[0]);
+ return 1;
+ }
+
+ if (is_file_whitelisted(elf))
+ return 0;
+
+ list_for_each_entry(sec, &elf->sections, list) {
+ ret = validate_section(elf, sec);
+ if (ret < 0)
+ return 1;
+
+ warnings += ret;
+ }
+
+ /* ignore warnings for now until we get all the asm code cleaned up */
+ return 0;
+}
--
2.1.0

2015-05-18 16:35:35

[permalink] [raw]

Subject: [PATCH v4 2/3] x86: Make push/pop CFI macros arch-independent

The separate push{lq}_cfi and pop_{lq}_cfi macros aren't needed. Push
and pop only come in one size per architecture, so the trailing 'q' or
'l' characters are redundant, and awkward to use in arch-independent
code.

Replace the push/pop CFI macros with architecture-independent versions:
push_cfi, pop_cfi, etc.

This change is purely cosmetic, with no resulting object code changes.

Suggested-by: "H. Peter Anvin" <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
---
arch/x86/ia32/ia32entry.S | 60 ++++++------
arch/x86/include/asm/calling.h | 28 +++---
arch/x86/include/asm/dwarf2.h | 92 ++++++------------
arch/x86/include/asm/frame.h | 4 +-
arch/x86/kernel/entry_32.S | 214 ++++++++++++++++++++---------------------
arch/x86/kernel/entry_64.S | 96 +++++++++---------
arch/x86/lib/atomic64_386_32.S | 4 +-
arch/x86/lib/atomic64_cx8_32.S | 40 ++++----
arch/x86/lib/checksum_32.S | 42 ++++----
arch/x86/lib/cmpxchg16b_emu.S | 6 +-
arch/x86/lib/cmpxchg8b_emu.S | 6 +-
arch/x86/lib/msr-reg.S | 34 +++----
arch/x86/lib/rwsem.S | 40 ++++----
arch/x86/lib/thunk_32.S | 12 +--
arch/x86/lib/thunk_64.S | 36 +++----
15 files changed, 343 insertions(+), 371 deletions(-)

diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index 83e4ed2..7259bc9 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -124,19 +124,19 @@ ENTRY(ia32_sysenter_target)
CFI_REGISTER rip,r10

/* Construct struct pt_regs on stack */
- pushq_cfi $__USER32_DS /* pt_regs->ss */
- pushq_cfi %rbp /* pt_regs->sp */
+ push_cfi $__USER32_DS /* pt_regs->ss */
+ push_cfi %rbp /* pt_regs->sp */
CFI_REL_OFFSET rsp,0
- pushfq_cfi /* pt_regs->flags */
- pushq_cfi $__USER32_CS /* pt_regs->cs */
- pushq_cfi %r10 /* pt_regs->ip = thread_info->sysenter_return */
+ pushf_cfi /* pt_regs->flags */
+ push_cfi $__USER32_CS /* pt_regs->cs */
+ push_cfi %r10 /* pt_regs->ip = thread_info->sysenter_return */
CFI_REL_OFFSET rip,0
- pushq_cfi_reg rax /* pt_regs->orig_ax */
- pushq_cfi_reg rdi /* pt_regs->di */
- pushq_cfi_reg rsi /* pt_regs->si */
- pushq_cfi_reg rdx /* pt_regs->dx */
- pushq_cfi_reg rcx /* pt_regs->cx */
- pushq_cfi $-ENOSYS /* pt_regs->ax */
+ push_cfi_reg rax /* pt_regs->orig_ax */
+ push_cfi_reg rdi /* pt_regs->di */
+ push_cfi_reg rsi /* pt_regs->si */
+ push_cfi_reg rdx /* pt_regs->dx */
+ push_cfi_reg rcx /* pt_regs->cx */
+ push_cfi $-ENOSYS /* pt_regs->ax */
cld
sub $(10*8),%rsp /* pt_regs->r8-11,bp,bx,r12-15 not saved */
CFI_ADJUST_CFA_OFFSET 10*8
@@ -282,8 +282,8 @@ sysexit_audit:
#endif

sysenter_fix_flags:
- pushq_cfi $(X86_EFLAGS_IF|X86_EFLAGS_FIXED)
- popfq_cfi
+ push_cfi $(X86_EFLAGS_IF|X86_EFLAGS_FIXED)
+ popf_cfi
jmp sysenter_flags_fixed

sysenter_tracesys:
@@ -353,20 +353,20 @@ ENTRY(ia32_cstar_target)
movl %eax,%eax

/* Construct struct pt_regs on stack */
- pushq_cfi $__USER32_DS /* pt_regs->ss */
- pushq_cfi %r8 /* pt_regs->sp */
+ push_cfi $__USER32_DS /* pt_regs->ss */
+ push_cfi %r8 /* pt_regs->sp */
CFI_REL_OFFSET rsp,0
- pushq_cfi %r11 /* pt_regs->flags */
- pushq_cfi $__USER32_CS /* pt_regs->cs */
- pushq_cfi %rcx /* pt_regs->ip */
+ push_cfi %r11 /* pt_regs->flags */
+ push_cfi $__USER32_CS /* pt_regs->cs */
+ push_cfi %rcx /* pt_regs->ip */
CFI_REL_OFFSET rip,0
- pushq_cfi_reg rax /* pt_regs->orig_ax */
- pushq_cfi_reg rdi /* pt_regs->di */
- pushq_cfi_reg rsi /* pt_regs->si */
- pushq_cfi_reg rdx /* pt_regs->dx */
- pushq_cfi_reg rbp /* pt_regs->cx */
+ push_cfi_reg rax /* pt_regs->orig_ax */
+ push_cfi_reg rdi /* pt_regs->di */
+ push_cfi_reg rsi /* pt_regs->si */
+ push_cfi_reg rdx /* pt_regs->dx */
+ push_cfi_reg rbp /* pt_regs->cx */
movl %ebp,%ecx
- pushq_cfi $-ENOSYS /* pt_regs->ax */
+ push_cfi $-ENOSYS /* pt_regs->ax */
sub $(10*8),%rsp /* pt_regs->r8-11,bp,bx,r12-15 not saved */
CFI_ADJUST_CFA_OFFSET 10*8

@@ -506,12 +506,12 @@ ENTRY(ia32_syscall)
movl %eax,%eax

/* Construct struct pt_regs on stack (iret frame is already on stack) */
- pushq_cfi_reg rax /* pt_regs->orig_ax */
- pushq_cfi_reg rdi /* pt_regs->di */
- pushq_cfi_reg rsi /* pt_regs->si */
- pushq_cfi_reg rdx /* pt_regs->dx */
- pushq_cfi_reg rcx /* pt_regs->cx */
- pushq_cfi $-ENOSYS /* pt_regs->ax */
+ push_cfi_reg rax /* pt_regs->orig_ax */
+ push_cfi_reg rdi /* pt_regs->di */
+ push_cfi_reg rsi /* pt_regs->si */
+ push_cfi_reg rdx /* pt_regs->dx */
+ push_cfi_reg rcx /* pt_regs->cx */
+ push_cfi $-ENOSYS /* pt_regs->ax */
cld
sub $(10*8),%rsp /* pt_regs->r8-11,bp,bx,r12-15 not saved */
CFI_ADJUST_CFA_OFFSET 10*8
diff --git a/arch/x86/include/asm/calling.h b/arch/x86/include/asm/calling.h
index 1c8b50e..4abc60f 100644
--- a/arch/x86/include/asm/calling.h
+++ b/arch/x86/include/asm/calling.h
@@ -224,23 +224,23 @@ For 32-bit we have the following conventions - kernel is built with
*/

.macro SAVE_ALL
- pushl_cfi_reg eax
- pushl_cfi_reg ebp
- pushl_cfi_reg edi
- pushl_cfi_reg esi
- pushl_cfi_reg edx
- pushl_cfi_reg ecx
- pushl_cfi_reg ebx
+ push_cfi_reg eax
+ push_cfi_reg ebp
+ push_cfi_reg edi
+ push_cfi_reg esi
+ push_cfi_reg edx
+ push_cfi_reg ecx
+ push_cfi_reg ebx
.endm

.macro RESTORE_ALL
- popl_cfi_reg ebx
- popl_cfi_reg ecx
- popl_cfi_reg edx
- popl_cfi_reg esi
- popl_cfi_reg edi
- popl_cfi_reg ebp
- popl_cfi_reg eax
+ pop_cfi_reg ebx
+ pop_cfi_reg ecx
+ pop_cfi_reg edx
+ pop_cfi_reg esi
+ pop_cfi_reg edi
+ pop_cfi_reg ebp
+ pop_cfi_reg eax
.endm

#endif /* CONFIG_X86_64 */
diff --git a/arch/x86/include/asm/dwarf2.h b/arch/x86/include/asm/dwarf2.h
index de1cdaf..5af7e15 100644
--- a/arch/x86/include/asm/dwarf2.h
+++ b/arch/x86/include/asm/dwarf2.h
@@ -5,6 +5,8 @@
#warning "asm/dwarf2.h should be only included in pure assembly files"
#endif

+#include <asm/asm.h>
+
/*
* Macros for dwarf2 CFI unwind table entries.
* See "as.info" for details on these pseudo ops. Unfortunately
@@ -80,79 +82,39 @@
* what you're doing if you use them.
*/
#ifdef __ASSEMBLY__
-#ifdef CONFIG_X86_64
- .macro pushq_cfi reg
- pushq \reg
- CFI_ADJUST_CFA_OFFSET 8
- .endm
-
- .macro pushq_cfi_reg reg
- pushq %\reg
- CFI_ADJUST_CFA_OFFSET 8
- CFI_REL_OFFSET \reg, 0
- .endm

- .macro popq_cfi reg
- popq \reg
- CFI_ADJUST_CFA_OFFSET -8
- .endm
-
- .macro popq_cfi_reg reg
- popq %\reg
- CFI_ADJUST_CFA_OFFSET -8
- CFI_RESTORE \reg
- .endm
+#define STACK_WORD_SIZE __ASM_SEL(4,8)

- .macro pushfq_cfi
- pushfq
- CFI_ADJUST_CFA_OFFSET 8
+ .macro push_cfi reg
+ push \reg
+ CFI_ADJUST_CFA_OFFSET STACK_WORD_SIZE
.endm

- .macro popfq_cfi
- popfq
- CFI_ADJUST_CFA_OFFSET -8
- .endm
-
- .macro movq_cfi reg offset=0
- movq %\reg, \offset(%rsp)
- CFI_REL_OFFSET \reg, \offset
- .endm
-
- .macro movq_cfi_restore offset reg
- movq \offset(%rsp), %\reg
- CFI_RESTORE \reg
- .endm
-#else /*!CONFIG_X86_64*/
- .macro pushl_cfi reg
- pushl \reg
- CFI_ADJUST_CFA_OFFSET 4
- .endm
-
- .macro pushl_cfi_reg reg
- pushl %\reg
- CFI_ADJUST_CFA_OFFSET 4
+ .macro push_cfi_reg reg
+ push %\reg
+ CFI_ADJUST_CFA_OFFSET STACK_WORD_SIZE
CFI_REL_OFFSET \reg, 0
.endm

- .macro popl_cfi reg
- popl \reg
- CFI_ADJUST_CFA_OFFSET -4
+ .macro pop_cfi reg
+ pop \reg
+ CFI_ADJUST_CFA_OFFSET -STACK_WORD_SIZE
.endm

- .macro popl_cfi_reg reg
- popl %\reg
- CFI_ADJUST_CFA_OFFSET -4
+ .macro pop_cfi_reg reg
+ pop %\reg
+ CFI_ADJUST_CFA_OFFSET -STACK_WORD_SIZE
CFI_RESTORE \reg
.endm

- .macro pushfl_cfi
- pushfl
- CFI_ADJUST_CFA_OFFSET 4
+ .macro pushf_cfi
+ pushf
+ CFI_ADJUST_CFA_OFFSET STACK_WORD_SIZE
.endm

- .macro popfl_cfi
- popfl
- CFI_ADJUST_CFA_OFFSET -4
+ .macro popf_cfi
+ popf
+ CFI_ADJUST_CFA_OFFSET -STACK_WORD_SIZE
.endm

.macro movl_cfi reg offset=0
@@ -164,7 +126,17 @@
movl \offset(%esp), %\reg
CFI_RESTORE \reg
.endm
-#endif /*!CONFIG_X86_64*/
+
+ .macro movq_cfi reg offset=0
+ movq %\reg, \offset(%rsp)
+ CFI_REL_OFFSET \reg, \offset
+ .endm
+
+ .macro movq_cfi_restore offset reg
+ movq \offset(%rsp), %\reg
+ CFI_RESTORE \reg
+ .endm
+
#endif /*__ASSEMBLY__*/

#endif /* _ASM_X86_DWARF2_H */
diff --git a/arch/x86/include/asm/frame.h b/arch/x86/include/asm/frame.h
index 3b629f4..325e4e8 100644
--- a/arch/x86/include/asm/frame.h
+++ b/arch/x86/include/asm/frame.h
@@ -8,12 +8,12 @@
frame pointer later */
#ifdef CONFIG_FRAME_POINTER
.macro FRAME
- __ASM_SIZE(push,_cfi) %__ASM_REG(bp)
+ push_cfi %__ASM_REG(bp)
CFI_REL_OFFSET __ASM_REG(bp), 0
__ASM_SIZE(mov) %__ASM_REG(sp), %__ASM_REG(bp)
.endm
.macro ENDFRAME
- __ASM_SIZE(pop,_cfi) %__ASM_REG(bp)
+ pop_cfi %__ASM_REG(bp)
CFI_RESTORE __ASM_REG(bp)
.endm
#else
diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index 1c30976..7e88181 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -113,7 +113,7 @@

/* unfortunately push/pop can't be no-op */
.macro PUSH_GS
- pushl_cfi $0
+ push_cfi $0
.endm
.macro POP_GS pop=0
addl $(4 + \pop), %esp
@@ -137,12 +137,12 @@
#else /* CONFIG_X86_32_LAZY_GS */

.macro PUSH_GS
- pushl_cfi %gs
+ push_cfi %gs
/*CFI_REL_OFFSET gs, 0*/
.endm

.macro POP_GS pop=0
-98: popl_cfi %gs
+98: pop_cfi %gs
/*CFI_RESTORE gs*/
.if \pop <> 0
add $\pop, %esp
@@ -186,25 +186,25 @@
.macro SAVE_ALL
cld
PUSH_GS
- pushl_cfi %fs
+ push_cfi %fs
/*CFI_REL_OFFSET fs, 0;*/
- pushl_cfi %es
+ push_cfi %es
/*CFI_REL_OFFSET es, 0;*/
- pushl_cfi %ds
+ push_cfi %ds
/*CFI_REL_OFFSET ds, 0;*/
- pushl_cfi %eax
+ push_cfi %eax
CFI_REL_OFFSET eax, 0
- pushl_cfi %ebp
+ push_cfi %ebp
CFI_REL_OFFSET ebp, 0
- pushl_cfi %edi
+ push_cfi %edi
CFI_REL_OFFSET edi, 0
- pushl_cfi %esi
+ push_cfi %esi
CFI_REL_OFFSET esi, 0
- pushl_cfi %edx
+ push_cfi %edx
CFI_REL_OFFSET edx, 0
- pushl_cfi %ecx
+ push_cfi %ecx
CFI_REL_OFFSET ecx, 0
- pushl_cfi %ebx
+ push_cfi %ebx
CFI_REL_OFFSET ebx, 0
movl $(__USER_DS), %edx
movl %edx, %ds
@@ -215,29 +215,29 @@
.endm

.macro RESTORE_INT_REGS
- popl_cfi %ebx
+ pop_cfi %ebx
CFI_RESTORE ebx
- popl_cfi %ecx
+ pop_cfi %ecx
CFI_RESTORE ecx
- popl_cfi %edx
+ pop_cfi %edx
CFI_RESTORE edx
- popl_cfi %esi
+ pop_cfi %esi
CFI_RESTORE esi
- popl_cfi %edi
+ pop_cfi %edi
CFI_RESTORE edi
- popl_cfi %ebp
+ pop_cfi %ebp
CFI_RESTORE ebp
- popl_cfi %eax
+ pop_cfi %eax
CFI_RESTORE eax
.endm

.macro RESTORE_REGS pop=0
RESTORE_INT_REGS
-1: popl_cfi %ds
+1: pop_cfi %ds
/*CFI_RESTORE ds;*/
-2: popl_cfi %es
+2: pop_cfi %es
/*CFI_RESTORE es;*/
-3: popl_cfi %fs
+3: pop_cfi %fs
/*CFI_RESTORE fs;*/
POP_GS \pop
.pushsection .fixup, "ax"
@@ -289,24 +289,24 @@

ENTRY(ret_from_fork)
CFI_STARTPROC
- pushl_cfi %eax
+ push_cfi %eax
call schedule_tail
GET_THREAD_INFO(%ebp)
- popl_cfi %eax
- pushl_cfi $0x0202 # Reset kernel eflags
- popfl_cfi
+ pop_cfi %eax
+ push_cfi $0x0202 # Reset kernel eflags
+ popf_cfi
jmp syscall_exit
CFI_ENDPROC
END(ret_from_fork)

ENTRY(ret_from_kernel_thread)
CFI_STARTPROC
- pushl_cfi %eax
+ push_cfi %eax
call schedule_tail
GET_THREAD_INFO(%ebp)
- popl_cfi %eax
- pushl_cfi $0x0202 # Reset kernel eflags
- popfl_cfi
+ pop_cfi %eax
+ push_cfi $0x0202 # Reset kernel eflags
+ popf_cfi
movl PT_EBP(%esp),%eax
call *PT_EBX(%esp)
movl $0,PT_EAX(%esp)
@@ -385,13 +385,13 @@ sysenter_past_esp:
* enough kernel state to call TRACE_IRQS_OFF can be called - but
* we immediately enable interrupts at that point anyway.
*/
- pushl_cfi $__USER_DS
+ push_cfi $__USER_DS
/*CFI_REL_OFFSET ss, 0*/
- pushl_cfi %ebp
+ push_cfi %ebp
CFI_REL_OFFSET esp, 0
- pushfl_cfi
+ pushf_cfi
orl $X86_EFLAGS_IF, (%esp)
- pushl_cfi $__USER_CS
+ push_cfi $__USER_CS
/*CFI_REL_OFFSET cs, 0*/
/*
* Push current_thread_info()->sysenter_return to the stack.
@@ -401,10 +401,10 @@ sysenter_past_esp:
* TOP_OF_KERNEL_STACK_PADDING takes us to the top of the stack;
* and THREAD_SIZE takes us to the bottom.
*/
- pushl_cfi ((TI_sysenter_return) - THREAD_SIZE + TOP_OF_KERNEL_STACK_PADDING + 4*4)(%esp)
+ push_cfi ((TI_sysenter_return) - THREAD_SIZE + TOP_OF_KERNEL_STACK_PADDING + 4*4)(%esp)
CFI_REL_OFFSET eip, 0

- pushl_cfi %eax
+ push_cfi %eax
SAVE_ALL
ENABLE_INTERRUPTS(CLBR_NONE)

@@ -453,11 +453,11 @@ sysenter_audit:
/* movl PT_EAX(%esp), %eax already set, syscall number: 1st arg to audit */
movl PT_EBX(%esp), %edx /* ebx/a0: 2nd arg to audit */
/* movl PT_ECX(%esp), %ecx already set, a1: 3nd arg to audit */
- pushl_cfi PT_ESI(%esp) /* a3: 5th arg */
- pushl_cfi PT_EDX+4(%esp) /* a2: 4th arg */
+ push_cfi PT_ESI(%esp) /* a3: 5th arg */
+ push_cfi PT_EDX+4(%esp) /* a2: 4th arg */
call __audit_syscall_entry
- popl_cfi %ecx /* get that remapped edx off the stack */
- popl_cfi %ecx /* get that remapped esi off the stack */
+ pop_cfi %ecx /* get that remapped edx off the stack */
+ pop_cfi %ecx /* get that remapped esi off the stack */
movl PT_EAX(%esp),%eax /* reload syscall number */
jmp sysenter_do_call

@@ -493,7 +493,7 @@ ENDPROC(ia32_sysenter_target)
ENTRY(system_call)
RING0_INT_FRAME # can't unwind into user space anyway
ASM_CLAC
- pushl_cfi %eax # save orig_eax
+ push_cfi %eax # save orig_eax
SAVE_ALL
GET_THREAD_INFO(%ebp)
# system call tracing in operation / emulation
@@ -577,8 +577,8 @@ ldt_ss:
shr $16, %edx
mov %dl, GDT_ESPFIX_SS + 4 /* bits 16..23 */
mov %dh, GDT_ESPFIX_SS + 7 /* bits 24..31 */
- pushl_cfi $__ESPFIX_SS
- pushl_cfi %eax /* new kernel esp */
+ push_cfi $__ESPFIX_SS
+ push_cfi %eax /* new kernel esp */
/* Disable interrupts, but do not irqtrace this section: we
* will soon execute iret and the tracer was already set to
* the irqstate after the iret */
@@ -634,9 +634,9 @@ work_notifysig: # deal with pending signals and
#ifdef CONFIG_VM86
ALIGN
work_notifysig_v86:
- pushl_cfi %ecx # save ti_flags for do_notify_resume
+ push_cfi %ecx # save ti_flags for do_notify_resume
call save_v86_state # %eax contains pt_regs pointer
- popl_cfi %ecx
+ pop_cfi %ecx
movl %eax, %esp
jmp 1b
#endif
@@ -701,8 +701,8 @@ END(sysenter_badsys)
mov GDT_ESPFIX_SS + 7, %ah /* bits 24..31 */
shl $16, %eax
addl %esp, %eax /* the adjusted stack pointer */
- pushl_cfi $__KERNEL_DS
- pushl_cfi %eax
+ push_cfi $__KERNEL_DS
+ push_cfi %eax
lss (%esp), %esp /* switch to the normal stack segment */
CFI_ADJUST_CFA_OFFSET -8
#endif
@@ -731,7 +731,7 @@ ENTRY(irq_entries_start)
RING0_INT_FRAME
vector=FIRST_EXTERNAL_VECTOR
.rept (FIRST_SYSTEM_VECTOR - FIRST_EXTERNAL_VECTOR)
- pushl_cfi $(~vector+0x80) /* Note: always in signed byte range */
+ push_cfi $(~vector+0x80) /* Note: always in signed byte range */
vector=vector+1
jmp common_interrupt
CFI_ADJUST_CFA_OFFSET -4
@@ -759,7 +759,7 @@ ENDPROC(common_interrupt)
ENTRY(name) \
RING0_INT_FRAME; \
ASM_CLAC; \
- pushl_cfi $~(nr); \
+ push_cfi $~(nr); \
SAVE_ALL; \
TRACE_IRQS_OFF \
movl %esp,%eax; \
@@ -786,8 +786,8 @@ ENDPROC(name)
ENTRY(coprocessor_error)
RING0_INT_FRAME
ASM_CLAC
- pushl_cfi $0
- pushl_cfi $do_coprocessor_error
+ push_cfi $0
+ push_cfi $do_coprocessor_error
jmp error_code
CFI_ENDPROC
END(coprocessor_error)
@@ -795,14 +795,14 @@ END(coprocessor_error)
ENTRY(simd_coprocessor_error)
RING0_INT_FRAME
ASM_CLAC
- pushl_cfi $0
+ push_cfi $0
#ifdef CONFIG_X86_INVD_BUG
/* AMD 486 bug: invd from userspace calls exception 19 instead of #GP */
- ALTERNATIVE "pushl_cfi $do_general_protection", \
+ ALTERNATIVE "push_cfi $do_general_protection", \
"pushl $do_simd_coprocessor_error", \
X86_FEATURE_XMM
#else
- pushl_cfi $do_simd_coprocessor_error
+ push_cfi $do_simd_coprocessor_error
#endif
jmp error_code
CFI_ENDPROC
@@ -811,8 +811,8 @@ END(simd_coprocessor_error)
ENTRY(device_not_available)
RING0_INT_FRAME
ASM_CLAC
- pushl_cfi $-1 # mark this as an int
- pushl_cfi $do_device_not_available
+ push_cfi $-1 # mark this as an int
+ push_cfi $do_device_not_available
jmp error_code
CFI_ENDPROC
END(device_not_available)
@@ -832,8 +832,8 @@ END(native_irq_enable_sysexit)
ENTRY(overflow)
RING0_INT_FRAME
ASM_CLAC
- pushl_cfi $0
- pushl_cfi $do_overflow
+ push_cfi $0
+ push_cfi $do_overflow
jmp error_code
CFI_ENDPROC
END(overflow)
@@ -841,8 +841,8 @@ END(overflow)
ENTRY(bounds)
RING0_INT_FRAME
ASM_CLAC
- pushl_cfi $0
- pushl_cfi $do_bounds
+ push_cfi $0
+ push_cfi $do_bounds
jmp error_code
CFI_ENDPROC
END(bounds)
@@ -850,8 +850,8 @@ END(bounds)
ENTRY(invalid_op)
RING0_INT_FRAME
ASM_CLAC
- pushl_cfi $0
- pushl_cfi $do_invalid_op
+ push_cfi $0
+ push_cfi $do_invalid_op
jmp error_code
CFI_ENDPROC
END(invalid_op)
@@ -859,8 +859,8 @@ END(invalid_op)
ENTRY(coprocessor_segment_overrun)
RING0_INT_FRAME
ASM_CLAC
- pushl_cfi $0
- pushl_cfi $do_coprocessor_segment_overrun
+ push_cfi $0
+ push_cfi $do_coprocessor_segment_overrun
jmp error_code
CFI_ENDPROC
END(coprocessor_segment_overrun)
@@ -868,7 +868,7 @@ END(coprocessor_segment_overrun)
ENTRY(invalid_TSS)
RING0_EC_FRAME
ASM_CLAC
- pushl_cfi $do_invalid_TSS
+ push_cfi $do_invalid_TSS
jmp error_code
CFI_ENDPROC
END(invalid_TSS)
@@ -876,7 +876,7 @@ END(invalid_TSS)
ENTRY(segment_not_present)
RING0_EC_FRAME
ASM_CLAC
- pushl_cfi $do_segment_not_present
+ push_cfi $do_segment_not_present
jmp error_code
CFI_ENDPROC
END(segment_not_present)
@@ -884,7 +884,7 @@ END(segment_not_present)
ENTRY(stack_segment)
RING0_EC_FRAME
ASM_CLAC
- pushl_cfi $do_stack_segment
+ push_cfi $do_stack_segment
jmp error_code
CFI_ENDPROC
END(stack_segment)
@@ -892,7 +892,7 @@ END(stack_segment)
ENTRY(alignment_check)
RING0_EC_FRAME
ASM_CLAC
- pushl_cfi $do_alignment_check
+ push_cfi $do_alignment_check
jmp error_code
CFI_ENDPROC
END(alignment_check)
@@ -900,8 +900,8 @@ END(alignment_check)
ENTRY(divide_error)
RING0_INT_FRAME
ASM_CLAC
- pushl_cfi $0 # no error code
- pushl_cfi $do_divide_error
+ push_cfi $0 # no error code
+ push_cfi $do_divide_error
jmp error_code
CFI_ENDPROC
END(divide_error)
@@ -910,8 +910,8 @@ END(divide_error)
ENTRY(machine_check)
RING0_INT_FRAME
ASM_CLAC
- pushl_cfi $0
- pushl_cfi machine_check_vector
+ push_cfi $0
+ push_cfi machine_check_vector
jmp error_code
CFI_ENDPROC
END(machine_check)
@@ -920,8 +920,8 @@ END(machine_check)
ENTRY(spurious_interrupt_bug)
RING0_INT_FRAME
ASM_CLAC
- pushl_cfi $0
- pushl_cfi $do_spurious_interrupt_bug
+ push_cfi $0
+ push_cfi $do_spurious_interrupt_bug
jmp error_code
CFI_ENDPROC
END(spurious_interrupt_bug)
@@ -938,7 +938,7 @@ ENTRY(xen_sysenter_target)

ENTRY(xen_hypervisor_callback)
CFI_STARTPROC
- pushl_cfi $-1 /* orig_ax = -1 => not a system call */
+ push_cfi $-1 /* orig_ax = -1 => not a system call */
SAVE_ALL
TRACE_IRQS_OFF

@@ -977,7 +977,7 @@ ENDPROC(xen_hypervisor_callback)
# We distinguish between categories by maintaining a status value in EAX.
ENTRY(xen_failsafe_callback)
CFI_STARTPROC
- pushl_cfi %eax
+ push_cfi %eax
movl $1,%eax
1: mov 4(%esp),%ds
2: mov 8(%esp),%es
@@ -986,12 +986,12 @@ ENTRY(xen_failsafe_callback)
/* EAX == 0 => Category 1 (Bad segment)
EAX != 0 => Category 2 (Bad IRET) */
testl %eax,%eax
- popl_cfi %eax
+ pop_cfi %eax
lea 16(%esp),%esp
CFI_ADJUST_CFA_OFFSET -16
jz 5f
jmp iret_exc
-5: pushl_cfi $-1 /* orig_ax = -1 => not a system call */
+5: push_cfi $-1 /* orig_ax = -1 => not a system call */
SAVE_ALL
jmp ret_from_exception
CFI_ENDPROC
@@ -1197,7 +1197,7 @@ return_to_handler:
ENTRY(trace_page_fault)
RING0_EC_FRAME
ASM_CLAC
- pushl_cfi $trace_do_page_fault
+ push_cfi $trace_do_page_fault
jmp error_code
CFI_ENDPROC
END(trace_page_fault)
@@ -1206,23 +1206,23 @@ END(trace_page_fault)
ENTRY(page_fault)
RING0_EC_FRAME
ASM_CLAC
- pushl_cfi $do_page_fault
+ push_cfi $do_page_fault
ALIGN
error_code:
/* the function address is in %gs's slot on the stack */
- pushl_cfi %fs
+ push_cfi %fs
/*CFI_REL_OFFSET fs, 0*/
- pushl_cfi %es
+ push_cfi %es
/*CFI_REL_OFFSET es, 0*/
- pushl_cfi %ds
+ push_cfi %ds
/*CFI_REL_OFFSET ds, 0*/
- pushl_cfi_reg eax
- pushl_cfi_reg ebp
- pushl_cfi_reg edi
- pushl_cfi_reg esi
- pushl_cfi_reg edx
- pushl_cfi_reg ecx
- pushl_cfi_reg ebx
+ push_cfi_reg eax
+ push_cfi_reg ebp
+ push_cfi_reg edi
+ push_cfi_reg esi
+ push_cfi_reg edx
+ push_cfi_reg ecx
+ push_cfi_reg ebx
cld
movl $(__KERNEL_PERCPU), %ecx
movl %ecx, %fs
@@ -1263,9 +1263,9 @@ END(page_fault)
movl TSS_sysenter_sp0 + \offset(%esp), %esp
CFI_DEF_CFA esp, 0
CFI_UNDEFINED eip
- pushfl_cfi
- pushl_cfi $__KERNEL_CS
- pushl_cfi $sysenter_past_esp
+ pushf_cfi
+ push_cfi $__KERNEL_CS
+ push_cfi $sysenter_past_esp
CFI_REL_OFFSET eip, 0
.endm

@@ -1276,7 +1276,7 @@ ENTRY(debug)
jne debug_stack_correct
FIX_STACK 12, debug_stack_correct, debug_esp_fix_insn
debug_stack_correct:
- pushl_cfi $-1 # mark this as an int
+ push_cfi $-1 # mark this as an int
SAVE_ALL
TRACE_IRQS_OFF
xorl %edx,%edx # error code 0
@@ -1298,28 +1298,28 @@ ENTRY(nmi)
RING0_INT_FRAME
ASM_CLAC
#ifdef CONFIG_X86_ESPFIX32
- pushl_cfi %eax
+ push_cfi %eax
movl %ss, %eax
cmpw $__ESPFIX_SS, %ax
- popl_cfi %eax
+ pop_cfi %eax
je nmi_espfix_stack
#endif
cmpl $ia32_sysenter_target,(%esp)
je nmi_stack_fixup
- pushl_cfi %eax
+ push_cfi %eax
movl %esp,%eax
/* Do not access memory above the end of our stack page,
* it might not exist.
*/
andl $(THREAD_SIZE-1),%eax
cmpl $(THREAD_SIZE-20),%eax
- popl_cfi %eax
+ pop_cfi %eax
jae nmi_stack_correct
cmpl $ia32_sysenter_target,12(%esp)
je nmi_debug_stack_check
nmi_stack_correct:
/* We have a RING0_INT_FRAME here */
- pushl_cfi %eax
+ push_cfi %eax
SAVE_ALL
xorl %edx,%edx # zero error code
movl %esp,%eax # pt_regs pointer
@@ -1349,14 +1349,14 @@ nmi_espfix_stack:
*
* create the pointer to lss back
*/
- pushl_cfi %ss
- pushl_cfi %esp
+ push_cfi %ss
+ push_cfi %esp
addl $4, (%esp)
/* copy the iret frame of 12 bytes */
.rept 3
- pushl_cfi 16(%esp)
+ push_cfi 16(%esp)
.endr
- pushl_cfi %eax
+ push_cfi %eax
SAVE_ALL
FIXUP_ESPFIX_STACK # %eax == %esp
xorl %edx,%edx # zero error code
@@ -1372,7 +1372,7 @@ END(nmi)
ENTRY(int3)
RING0_INT_FRAME
ASM_CLAC
- pushl_cfi $-1 # mark this as an int
+ push_cfi $-1 # mark this as an int
SAVE_ALL
TRACE_IRQS_OFF
xorl %edx,%edx # zero error code
@@ -1384,7 +1384,7 @@ END(int3)

ENTRY(general_protection)
RING0_EC_FRAME
- pushl_cfi $do_general_protection
+ push_cfi $do_general_protection
jmp error_code
CFI_ENDPROC
END(general_protection)
@@ -1393,7 +1393,7 @@ END(general_protection)
ENTRY(async_page_fault)
RING0_EC_FRAME
ASM_CLAC
- pushl_cfi $do_async_page_fault
+ push_cfi $do_async_page_fault
jmp error_code
CFI_ENDPROC
END(async_page_fault)
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 4e0ed47..3f2c4b2 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -219,8 +219,8 @@ GLOBAL(system_call_after_swapgs)
movq PER_CPU_VAR(cpu_current_top_of_stack),%rsp

/* Construct struct pt_regs on stack */
- pushq_cfi $__USER_DS /* pt_regs->ss */
- pushq_cfi PER_CPU_VAR(rsp_scratch) /* pt_regs->sp */
+ push_cfi $__USER_DS /* pt_regs->ss */
+ push_cfi PER_CPU_VAR(rsp_scratch) /* pt_regs->sp */
/*
* Re-enable interrupts.
* We use 'rsp_scratch' as a scratch space, hence irq-off block above
@@ -229,20 +229,20 @@ GLOBAL(system_call_after_swapgs)
* with using rsp_scratch:
*/
ENABLE_INTERRUPTS(CLBR_NONE)
- pushq_cfi %r11 /* pt_regs->flags */
- pushq_cfi $__USER_CS /* pt_regs->cs */
- pushq_cfi %rcx /* pt_regs->ip */
+ push_cfi %r11 /* pt_regs->flags */
+ push_cfi $__USER_CS /* pt_regs->cs */
+ push_cfi %rcx /* pt_regs->ip */
CFI_REL_OFFSET rip,0
- pushq_cfi_reg rax /* pt_regs->orig_ax */
- pushq_cfi_reg rdi /* pt_regs->di */
- pushq_cfi_reg rsi /* pt_regs->si */
- pushq_cfi_reg rdx /* pt_regs->dx */
- pushq_cfi_reg rcx /* pt_regs->cx */
- pushq_cfi $-ENOSYS /* pt_regs->ax */
- pushq_cfi_reg r8 /* pt_regs->r8 */
- pushq_cfi_reg r9 /* pt_regs->r9 */
- pushq_cfi_reg r10 /* pt_regs->r10 */
- pushq_cfi_reg r11 /* pt_regs->r11 */
+ push_cfi_reg rax /* pt_regs->orig_ax */
+ push_cfi_reg rdi /* pt_regs->di */
+ push_cfi_reg rsi /* pt_regs->si */
+ push_cfi_reg rdx /* pt_regs->dx */
+ push_cfi_reg rcx /* pt_regs->cx */
+ push_cfi $-ENOSYS /* pt_regs->ax */
+ push_cfi_reg r8 /* pt_regs->r8 */
+ push_cfi_reg r9 /* pt_regs->r9 */
+ push_cfi_reg r10 /* pt_regs->r10 */
+ push_cfi_reg r11 /* pt_regs->r11 */
sub $(6*8),%rsp /* pt_regs->bp,bx,r12-15 not saved */
CFI_ADJUST_CFA_OFFSET 6*8

@@ -374,9 +374,9 @@ int_careful:
jnc int_very_careful
TRACE_IRQS_ON
ENABLE_INTERRUPTS(CLBR_NONE)
- pushq_cfi %rdi
+ push_cfi %rdi
SCHEDULE_USER
- popq_cfi %rdi
+ pop_cfi %rdi
DISABLE_INTERRUPTS(CLBR_NONE)
TRACE_IRQS_OFF
jmp int_with_check
@@ -389,10 +389,10 @@ int_very_careful:
/* Check for syscall exit trace */
testl $_TIF_WORK_SYSCALL_EXIT,%edx
jz int_signal
- pushq_cfi %rdi
+ push_cfi %rdi
leaq 8(%rsp),%rdi # &ptregs -> arg1
call syscall_trace_leave
- popq_cfi %rdi
+ pop_cfi %rdi
andl $~(_TIF_WORK_SYSCALL_EXIT|_TIF_SYSCALL_EMU),%edi
jmp int_restore_rest

@@ -603,8 +603,8 @@ ENTRY(ret_from_fork)

LOCK ; btr $TIF_FORK,TI_flags(%r8)

- pushq_cfi $0x0002
- popfq_cfi # reset kernel eflags
+ push_cfi $0x0002
+ popf_cfi # reset kernel eflags

call schedule_tail # rdi: 'prev' task parameter

@@ -640,7 +640,7 @@ ENTRY(irq_entries_start)
INTR_FRAME
vector=FIRST_EXTERNAL_VECTOR
.rept (FIRST_SYSTEM_VECTOR - FIRST_EXTERNAL_VECTOR)
- pushq_cfi $(~vector+0x80) /* Note: always in signed byte range */
+ push_cfi $(~vector+0x80) /* Note: always in signed byte range */
vector=vector+1
jmp common_interrupt
CFI_ADJUST_CFA_OFFSET -8
@@ -807,8 +807,8 @@ native_irq_return_iret:

#ifdef CONFIG_X86_ESPFIX64
native_irq_return_ldt:
- pushq_cfi %rax
- pushq_cfi %rdi
+ push_cfi %rax
+ push_cfi %rdi
SWAPGS
movq PER_CPU_VAR(espfix_waddr),%rdi
movq %rax,(0*8)(%rdi) /* RAX */
@@ -823,11 +823,11 @@ native_irq_return_ldt:
movq (5*8)(%rsp),%rax /* RSP */
movq %rax,(4*8)(%rdi)
andl $0xffff0000,%eax
- popq_cfi %rdi
+ pop_cfi %rdi
orq PER_CPU_VAR(espfix_stack),%rax
SWAPGS
movq %rax,%rsp
- popq_cfi %rax
+ pop_cfi %rax
jmp native_irq_return_iret
#endif

@@ -838,9 +838,9 @@ retint_careful:
jnc retint_signal
TRACE_IRQS_ON
ENABLE_INTERRUPTS(CLBR_NONE)
- pushq_cfi %rdi
+ push_cfi %rdi
SCHEDULE_USER
- popq_cfi %rdi
+ pop_cfi %rdi
GET_THREAD_INFO(%rcx)
DISABLE_INTERRUPTS(CLBR_NONE)
TRACE_IRQS_OFF
@@ -872,7 +872,7 @@ END(common_interrupt)
ENTRY(\sym)
INTR_FRAME
ASM_CLAC
- pushq_cfi $~(\num)
+ push_cfi $~(\num)
.Lcommon_\sym:
interrupt \do_sym
jmp ret_from_intr
@@ -974,7 +974,7 @@ ENTRY(\sym)
PARAVIRT_ADJUST_EXCEPTION_FRAME

.ifeq \has_error_code
- pushq_cfi $-1 /* ORIG_RAX: no syscall to restart */
+ push_cfi $-1 /* ORIG_RAX: no syscall to restart */
.endif

ALLOC_PT_GPREGS_ON_STACK
@@ -1091,14 +1091,14 @@ idtentry simd_coprocessor_error do_simd_coprocessor_error has_error_code=0
/* edi: new selector */
ENTRY(native_load_gs_index)
CFI_STARTPROC
- pushfq_cfi
+ pushf_cfi
DISABLE_INTERRUPTS(CLBR_ANY & ~CLBR_RDI)
SWAPGS
gs_change:
movl %edi,%gs
2: mfence /* workaround */
SWAPGS
- popfq_cfi
+ popf_cfi
ret
CFI_ENDPROC
END(native_load_gs_index)
@@ -1116,7 +1116,7 @@ bad_gs:
/* Call softirq on interrupt stack. Interrupts are off. */
ENTRY(do_softirq_own_stack)
CFI_STARTPROC
- pushq_cfi %rbp
+ push_cfi %rbp
CFI_REL_OFFSET rbp,0
mov %rsp,%rbp
CFI_DEF_CFA_REGISTER rbp
@@ -1215,9 +1215,9 @@ ENTRY(xen_failsafe_callback)
CFI_RESTORE r11
addq $0x30,%rsp
CFI_ADJUST_CFA_OFFSET -0x30
- pushq_cfi $0 /* RIP */
- pushq_cfi %r11
- pushq_cfi %rcx
+ push_cfi $0 /* RIP */
+ push_cfi %r11
+ push_cfi %rcx
jmp general_protection
CFI_RESTORE_STATE
1: /* Segment mismatch => Category 1 (Bad segment). Retry the IRET. */
@@ -1227,7 +1227,7 @@ ENTRY(xen_failsafe_callback)
CFI_RESTORE r11
addq $0x30,%rsp
CFI_ADJUST_CFA_OFFSET -0x30
- pushq_cfi $-1 /* orig_ax = -1 => not a system call */
+ push_cfi $-1 /* orig_ax = -1 => not a system call */
ALLOC_PT_GPREGS_ON_STACK
SAVE_C_REGS
SAVE_EXTRA_REGS
@@ -1422,7 +1422,7 @@ ENTRY(nmi)
*/

/* Use %rdx as our temp variable throughout */
- pushq_cfi %rdx
+ push_cfi %rdx
CFI_REL_OFFSET rdx, 0

/*
@@ -1478,18 +1478,18 @@ nested_nmi:
movq %rdx, %rsp
CFI_ADJUST_CFA_OFFSET 1*8
leaq -10*8(%rsp), %rdx
- pushq_cfi $__KERNEL_DS
- pushq_cfi %rdx
- pushfq_cfi
- pushq_cfi $__KERNEL_CS
- pushq_cfi $repeat_nmi
+ push_cfi $__KERNEL_DS
+ push_cfi %rdx
+ pushf_cfi
+ push_cfi $__KERNEL_CS
+ push_cfi $repeat_nmi

/* Put stack back */
addq $(6*8), %rsp
CFI_ADJUST_CFA_OFFSET -6*8

nested_nmi_out:
- popq_cfi %rdx
+ pop_cfi %rdx
CFI_RESTORE rdx

/* No need to check faults here */
@@ -1537,7 +1537,7 @@ first_nmi:
CFI_RESTORE rdx

/* Set the NMI executing variable on the stack. */
- pushq_cfi $1
+ push_cfi $1

/*
* Leave room for the "copied" frame
@@ -1547,7 +1547,7 @@ first_nmi:

/* Copy the stack frame to the Saved frame */
.rept 5
- pushq_cfi 11*8(%rsp)
+ push_cfi 11*8(%rsp)
.endr
CFI_DEF_CFA_OFFSET 5*8

@@ -1574,7 +1574,7 @@ repeat_nmi:
addq $(10*8), %rsp
CFI_ADJUST_CFA_OFFSET -10*8
.rept 5
- pushq_cfi -6*8(%rsp)
+ push_cfi -6*8(%rsp)
.endr
subq $(5*8), %rsp
CFI_DEF_CFA_OFFSET 5*8
@@ -1585,7 +1585,7 @@ end_repeat_nmi:
* NMI if the first NMI took an exception and reset our iret stack
* so that we repeat another NMI.
*/
- pushq_cfi $-1 /* ORIG_RAX: no syscall to restart */
+ push_cfi $-1 /* ORIG_RAX: no syscall to restart */
ALLOC_PT_GPREGS_ON_STACK

/*
diff --git a/arch/x86/lib/atomic64_386_32.S b/arch/x86/lib/atomic64_386_32.S
index 00933d5..aa17c69 100644
--- a/arch/x86/lib/atomic64_386_32.S
+++ b/arch/x86/lib/atomic64_386_32.S
@@ -15,12 +15,12 @@

/* if you want SMP support, implement these with real spinlocks */
.macro LOCK reg
- pushfl_cfi
+ pushf_cfi
cli
.endm

.macro UNLOCK reg
- popfl_cfi
+ popf_cfi
.endm

#define BEGIN(op) \
diff --git a/arch/x86/lib/atomic64_cx8_32.S b/arch/x86/lib/atomic64_cx8_32.S
index 082a851..c5dd086 100644
--- a/arch/x86/lib/atomic64_cx8_32.S
+++ b/arch/x86/lib/atomic64_cx8_32.S
@@ -57,10 +57,10 @@ ENDPROC(atomic64_xchg_cx8)
.macro addsub_return func ins insc
ENTRY(atomic64_\func\()_return_cx8)
CFI_STARTPROC
- pushl_cfi_reg ebp
- pushl_cfi_reg ebx
- pushl_cfi_reg esi
- pushl_cfi_reg edi
+ push_cfi_reg ebp
+ push_cfi_reg ebx
+ push_cfi_reg esi
+ push_cfi_reg edi

movl %eax, %esi
movl %edx, %edi
@@ -79,10 +79,10 @@ ENTRY(atomic64_\func\()_return_cx8)
10:
movl %ebx, %eax
movl %ecx, %edx
- popl_cfi_reg edi
- popl_cfi_reg esi
- popl_cfi_reg ebx
- popl_cfi_reg ebp
+ pop_cfi_reg edi
+ pop_cfi_reg esi
+ pop_cfi_reg ebx
+ pop_cfi_reg ebp
ret
CFI_ENDPROC
ENDPROC(atomic64_\func\()_return_cx8)
@@ -94,7 +94,7 @@ addsub_return sub sub sbb
.macro incdec_return func ins insc
ENTRY(atomic64_\func\()_return_cx8)
CFI_STARTPROC
- pushl_cfi_reg ebx
+ push_cfi_reg ebx

read64 %esi
1:
@@ -109,7 +109,7 @@ ENTRY(atomic64_\func\()_return_cx8)
10:
movl %ebx, %eax
movl %ecx, %edx
- popl_cfi_reg ebx
+ pop_cfi_reg ebx
ret
CFI_ENDPROC
ENDPROC(atomic64_\func\()_return_cx8)
@@ -120,7 +120,7 @@ incdec_return dec sub sbb

ENTRY(atomic64_dec_if_positive_cx8)
CFI_STARTPROC
- pushl_cfi_reg ebx
+ push_cfi_reg ebx

read64 %esi
1:
@@ -136,18 +136,18 @@ ENTRY(atomic64_dec_if_positive_cx8)
2:
movl %ebx, %eax
movl %ecx, %edx
- popl_cfi_reg ebx
+ pop_cfi_reg ebx
ret
CFI_ENDPROC
ENDPROC(atomic64_dec_if_positive_cx8)

ENTRY(atomic64_add_unless_cx8)
CFI_STARTPROC
- pushl_cfi_reg ebp
- pushl_cfi_reg ebx
+ push_cfi_reg ebp
+ push_cfi_reg ebx
/* these just push these two parameters on the stack */
- pushl_cfi_reg edi
- pushl_cfi_reg ecx
+ push_cfi_reg edi
+ push_cfi_reg ecx

movl %eax, %ebp
movl %edx, %edi
@@ -169,8 +169,8 @@ ENTRY(atomic64_add_unless_cx8)
3:
addl $8, %esp
CFI_ADJUST_CFA_OFFSET -8
- popl_cfi_reg ebx
- popl_cfi_reg ebp
+ pop_cfi_reg ebx
+ pop_cfi_reg ebp
ret
4:
cmpl %edx, 4(%esp)
@@ -182,7 +182,7 @@ ENDPROC(atomic64_add_unless_cx8)

ENTRY(atomic64_inc_not_zero_cx8)
CFI_STARTPROC
- pushl_cfi_reg ebx
+ push_cfi_reg ebx

read64 %esi
1:
@@ -199,7 +199,7 @@ ENTRY(atomic64_inc_not_zero_cx8)

movl $1, %eax
3:
- popl_cfi_reg ebx
+ pop_cfi_reg ebx
ret
CFI_ENDPROC
ENDPROC(atomic64_inc_not_zero_cx8)
diff --git a/arch/x86/lib/checksum_32.S b/arch/x86/lib/checksum_32.S
index 9bc944a..42c1f9f 100644
--- a/arch/x86/lib/checksum_32.S
+++ b/arch/x86/lib/checksum_32.S
@@ -51,8 +51,8 @@ unsigned int csum_partial(const unsigned char * buff, int len, unsigned int sum)
*/
ENTRY(csum_partial)
CFI_STARTPROC
- pushl_cfi_reg esi
- pushl_cfi_reg ebx
+ push_cfi_reg esi
+ push_cfi_reg ebx
movl 20(%esp),%eax # Function arg: unsigned int sum
movl 16(%esp),%ecx # Function arg: int len
movl 12(%esp),%esi # Function arg: unsigned char *buff
@@ -129,8 +129,8 @@ ENTRY(csum_partial)
jz 8f
roll $8, %eax
8:
- popl_cfi_reg ebx
- popl_cfi_reg esi
+ pop_cfi_reg ebx
+ pop_cfi_reg esi
ret
CFI_ENDPROC
ENDPROC(csum_partial)
@@ -141,8 +141,8 @@ ENDPROC(csum_partial)

ENTRY(csum_partial)
CFI_STARTPROC
- pushl_cfi_reg esi
- pushl_cfi_reg ebx
+ push_cfi_reg esi
+ push_cfi_reg ebx
movl 20(%esp),%eax # Function arg: unsigned int sum
movl 16(%esp),%ecx # Function arg: int len
movl 12(%esp),%esi # Function arg: const unsigned char *buf
@@ -249,8 +249,8 @@ ENTRY(csum_partial)
jz 90f
roll $8, %eax
90:
- popl_cfi_reg ebx
- popl_cfi_reg esi
+ pop_cfi_reg ebx
+ pop_cfi_reg esi
ret
CFI_ENDPROC
ENDPROC(csum_partial)
@@ -290,9 +290,9 @@ ENTRY(csum_partial_copy_generic)
CFI_STARTPROC
subl $4,%esp
CFI_ADJUST_CFA_OFFSET 4
- pushl_cfi_reg edi
- pushl_cfi_reg esi
- pushl_cfi_reg ebx
+ push_cfi_reg edi
+ push_cfi_reg esi
+ push_cfi_reg ebx
movl ARGBASE+16(%esp),%eax # sum
movl ARGBASE+12(%esp),%ecx # len
movl ARGBASE+4(%esp),%esi # src
@@ -401,10 +401,10 @@ DST( movb %cl, (%edi) )

.previous

- popl_cfi_reg ebx
- popl_cfi_reg esi
- popl_cfi_reg edi
- popl_cfi %ecx # equivalent to addl $4,%esp
+ pop_cfi_reg ebx
+ pop_cfi_reg esi
+ pop_cfi_reg edi
+ pop_cfi %ecx # equivalent to addl $4,%esp
ret
CFI_ENDPROC
ENDPROC(csum_partial_copy_generic)
@@ -427,9 +427,9 @@ ENDPROC(csum_partial_copy_generic)

ENTRY(csum_partial_copy_generic)
CFI_STARTPROC
- pushl_cfi_reg ebx
- pushl_cfi_reg edi
- pushl_cfi_reg esi
+ push_cfi_reg ebx
+ push_cfi_reg edi
+ push_cfi_reg esi
movl ARGBASE+4(%esp),%esi #src
movl ARGBASE+8(%esp),%edi #dst
movl ARGBASE+12(%esp),%ecx #len
@@ -489,9 +489,9 @@ DST( movb %dl, (%edi) )
jmp 7b
.previous

- popl_cfi_reg esi
- popl_cfi_reg edi
- popl_cfi_reg ebx
+ pop_cfi_reg esi
+ pop_cfi_reg edi
+ pop_cfi_reg ebx
ret
CFI_ENDPROC
ENDPROC(csum_partial_copy_generic)
diff --git a/arch/x86/lib/cmpxchg16b_emu.S b/arch/x86/lib/cmpxchg16b_emu.S
index 40a1725..b18f317 100644
--- a/arch/x86/lib/cmpxchg16b_emu.S
+++ b/arch/x86/lib/cmpxchg16b_emu.S
@@ -32,7 +32,7 @@ CFI_STARTPROC
# *atomic* on a single cpu (as provided by the this_cpu_xx class of
# macros).
#
- pushfq_cfi
+ pushf_cfi
cli

cmpq PER_CPU_VAR((%rsi)), %rax
@@ -44,13 +44,13 @@ CFI_STARTPROC
movq %rcx, PER_CPU_VAR(8(%rsi))

CFI_REMEMBER_STATE
- popfq_cfi
+ popf_cfi
mov $1, %al
ret

CFI_RESTORE_STATE
.Lnot_same:
- popfq_cfi
+ popf_cfi
xor %al,%al
ret

diff --git a/arch/x86/lib/cmpxchg8b_emu.S b/arch/x86/lib/cmpxchg8b_emu.S
index b4807fce..a4862d0 100644
--- a/arch/x86/lib/cmpxchg8b_emu.S
+++ b/arch/x86/lib/cmpxchg8b_emu.S
@@ -27,7 +27,7 @@ CFI_STARTPROC
# set the whole ZF thing (caller will just compare
# eax:edx with the expected value)
#
- pushfl_cfi
+ pushf_cfi
cli

cmpl (%esi), %eax
@@ -39,7 +39,7 @@ CFI_STARTPROC
movl %ecx, 4(%esi)

CFI_REMEMBER_STATE
- popfl_cfi
+ popf_cfi
ret

CFI_RESTORE_STATE
@@ -48,7 +48,7 @@ CFI_STARTPROC
.Lhalf_same:
movl 4(%esi), %edx

- popfl_cfi
+ popf_cfi
ret

CFI_ENDPROC
diff --git a/arch/x86/lib/msr-reg.S b/arch/x86/lib/msr-reg.S
index 3ca5218..046a560 100644
--- a/arch/x86/lib/msr-reg.S
+++ b/arch/x86/lib/msr-reg.S
@@ -14,8 +14,8 @@
.macro op_safe_regs op
ENTRY(\op\()_safe_regs)
CFI_STARTPROC
- pushq_cfi_reg rbx
- pushq_cfi_reg rbp
+ push_cfi_reg rbx
+ push_cfi_reg rbp
movq %rdi, %r10 /* Save pointer */
xorl %r11d, %r11d /* Return value */
movl (%rdi), %eax
@@ -35,8 +35,8 @@ ENTRY(\op\()_safe_regs)
movl %ebp, 20(%r10)
movl %esi, 24(%r10)
movl %edi, 28(%r10)
- popq_cfi_reg rbp
- popq_cfi_reg rbx
+ pop_cfi_reg rbp
+ pop_cfi_reg rbx
ret
3:
CFI_RESTORE_STATE
@@ -53,12 +53,12 @@ ENDPROC(\op\()_safe_regs)
.macro op_safe_regs op
ENTRY(\op\()_safe_regs)
CFI_STARTPROC
- pushl_cfi_reg ebx
- pushl_cfi_reg ebp
- pushl_cfi_reg esi
- pushl_cfi_reg edi
- pushl_cfi $0 /* Return value */
- pushl_cfi %eax
+ push_cfi_reg ebx
+ push_cfi_reg ebp
+ push_cfi_reg esi
+ push_cfi_reg edi
+ push_cfi $0 /* Return value */
+ push_cfi %eax
movl 4(%eax), %ecx
movl 8(%eax), %edx
movl 12(%eax), %ebx
@@ -68,9 +68,9 @@ ENTRY(\op\()_safe_regs)
movl (%eax), %eax
CFI_REMEMBER_STATE
1: \op
-2: pushl_cfi %eax
+2: push_cfi %eax
movl 4(%esp), %eax
- popl_cfi (%eax)
+ pop_cfi (%eax)
addl $4, %esp
CFI_ADJUST_CFA_OFFSET -4
movl %ecx, 4(%eax)
@@ -79,11 +79,11 @@ ENTRY(\op\()_safe_regs)
movl %ebp, 20(%eax)
movl %esi, 24(%eax)
movl %edi, 28(%eax)
- popl_cfi %eax
- popl_cfi_reg edi
- popl_cfi_reg esi
- popl_cfi_reg ebp
- popl_cfi_reg ebx
+ pop_cfi %eax
+ pop_cfi_reg edi
+ pop_cfi_reg esi
+ pop_cfi_reg ebp
+ pop_cfi_reg ebx
ret
3:
CFI_RESTORE_STATE
diff --git a/arch/x86/lib/rwsem.S b/arch/x86/lib/rwsem.S
index 2322abe..c630a80 100644
--- a/arch/x86/lib/rwsem.S
+++ b/arch/x86/lib/rwsem.S
@@ -34,10 +34,10 @@
*/

#define save_common_regs \
- pushl_cfi_reg ecx
+ push_cfi_reg ecx

#define restore_common_regs \
- popl_cfi_reg ecx
+ pop_cfi_reg ecx

/* Avoid uglifying the argument copying x86-64 needs to do. */
.macro movq src, dst
@@ -64,22 +64,22 @@
*/

#define save_common_regs \
- pushq_cfi_reg rdi; \
- pushq_cfi_reg rsi; \
- pushq_cfi_reg rcx; \
- pushq_cfi_reg r8; \
- pushq_cfi_reg r9; \
- pushq_cfi_reg r10; \
- pushq_cfi_reg r11
+ push_cfi_reg rdi; \
+ push_cfi_reg rsi; \
+ push_cfi_reg rcx; \
+ push_cfi_reg r8; \
+ push_cfi_reg r9; \
+ push_cfi_reg r10; \
+ push_cfi_reg r11

#define restore_common_regs \
- popq_cfi_reg r11; \
- popq_cfi_reg r10; \
- popq_cfi_reg r9; \
- popq_cfi_reg r8; \
- popq_cfi_reg rcx; \
- popq_cfi_reg rsi; \
- popq_cfi_reg rdi
+ pop_cfi_reg r11; \
+ pop_cfi_reg r10; \
+ pop_cfi_reg r9; \
+ pop_cfi_reg r8; \
+ pop_cfi_reg rcx; \
+ pop_cfi_reg rsi; \
+ pop_cfi_reg rdi

#endif

@@ -87,10 +87,10 @@
ENTRY(call_rwsem_down_read_failed)
CFI_STARTPROC
save_common_regs
- __ASM_SIZE(push,_cfi_reg) __ASM_REG(dx)
+ push_cfi_reg __ASM_REG(dx)
movq %rax,%rdi
call rwsem_down_read_failed
- __ASM_SIZE(pop,_cfi_reg) __ASM_REG(dx)
+ pop_cfi_reg __ASM_REG(dx)
restore_common_regs
ret
CFI_ENDPROC
@@ -122,10 +122,10 @@ ENDPROC(call_rwsem_wake)
ENTRY(call_rwsem_downgrade_wake)
CFI_STARTPROC
save_common_regs
- __ASM_SIZE(push,_cfi_reg) __ASM_REG(dx)
+ push_cfi_reg __ASM_REG(dx)
movq %rax,%rdi
call rwsem_downgrade_wake
- __ASM_SIZE(pop,_cfi_reg) __ASM_REG(dx)
+ pop_cfi_reg __ASM_REG(dx)
restore_common_regs
ret
CFI_ENDPROC
diff --git a/arch/x86/lib/thunk_32.S b/arch/x86/lib/thunk_32.S
index 5eb7150..bb370de 100644
--- a/arch/x86/lib/thunk_32.S
+++ b/arch/x86/lib/thunk_32.S
@@ -13,9 +13,9 @@
.globl \name
\name:
CFI_STARTPROC
- pushl_cfi_reg eax
- pushl_cfi_reg ecx
- pushl_cfi_reg edx
+ push_cfi_reg eax
+ push_cfi_reg ecx
+ push_cfi_reg edx

.if \put_ret_addr_in_eax
/* Place EIP in the arg1 */
@@ -23,9 +23,9 @@
.endif

call \func
- popl_cfi_reg edx
- popl_cfi_reg ecx
- popl_cfi_reg eax
+ pop_cfi_reg edx
+ pop_cfi_reg ecx
+ pop_cfi_reg eax
ret
CFI_ENDPROC
_ASM_NOKPROBE(\name)
diff --git a/arch/x86/lib/thunk_64.S b/arch/x86/lib/thunk_64.S
index f89ba4e9..39ad268 100644
--- a/arch/x86/lib/thunk_64.S
+++ b/arch/x86/lib/thunk_64.S
@@ -17,15 +17,15 @@
CFI_STARTPROC

/* this one pushes 9 elems, the next one would be %rIP */
- pushq_cfi_reg rdi
- pushq_cfi_reg rsi
- pushq_cfi_reg rdx
- pushq_cfi_reg rcx
- pushq_cfi_reg rax
- pushq_cfi_reg r8
- pushq_cfi_reg r9
- pushq_cfi_reg r10
- pushq_cfi_reg r11
+ push_cfi_reg rdi
+ push_cfi_reg rsi
+ push_cfi_reg rdx
+ push_cfi_reg rcx
+ push_cfi_reg rax
+ push_cfi_reg r8
+ push_cfi_reg r9
+ push_cfi_reg r10
+ push_cfi_reg r11

.if \put_ret_addr_in_rdi
/* 9*8(%rsp) is return addr on stack */
@@ -60,15 +60,15 @@
CFI_STARTPROC
CFI_ADJUST_CFA_OFFSET 9*8
restore:
- popq_cfi_reg r11
- popq_cfi_reg r10
- popq_cfi_reg r9
- popq_cfi_reg r8
- popq_cfi_reg rax
- popq_cfi_reg rcx
- popq_cfi_reg rdx
- popq_cfi_reg rsi
- popq_cfi_reg rdi
+ pop_cfi_reg r11
+ pop_cfi_reg r10
+ pop_cfi_reg r9
+ pop_cfi_reg r8
+ pop_cfi_reg rax
+ pop_cfi_reg rcx
+ pop_cfi_reg rdx
+ pop_cfi_reg rsi
+ pop_cfi_reg rdi
ret
CFI_ENDPROC
_ASM_NOKPROBE(restore)
--
2.1.0

2015-05-18 16:35:23

[permalink] [raw]

Subject: [PATCH v4 3/3] x86, stackvalidate: Add asm frame pointer setup macros

Add some helper macros for asm functions so that they can comply with
stackvalidate.

The FUNC_ENTER and FUNC_RETURN macros help asm functions save, set up,
and restore frame pointers.

The RET_NOVALIDATE and FILE_NOVALIDATE macros can be used to whitelist
the few locations which need a return instruction outside of a callable
function.

Signed-off-by: Josh Poimboeuf <[email protected]>
---
arch/x86/include/asm/func.h | 82 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 82 insertions(+)
create mode 100644 arch/x86/include/asm/func.h

diff --git a/arch/x86/include/asm/func.h b/arch/x86/include/asm/func.h
new file mode 100644
index 0000000..cd27ad4
--- /dev/null
+++ b/arch/x86/include/asm/func.h
@@ -0,0 +1,82 @@
+#ifndef _ASM_X86_FUNC_H
+#define _ASM_X86_FUNC_H
+
+#include <linux/linkage.h>
+#include <asm/dwarf2.h>
+#include <asm/asm.h>
+
+.macro FUNC_ENTER_NO_FP name
+ ENTRY(\name)
+ CFI_STARTPROC
+ CFI_DEF_CFA _ASM_SP, __ASM_SEL(4, 8)
+.endm
+
+.macro FUNC_RETURN_NO_FP name
+ CFI_DEF_CFA _ASM_SP, __ASM_SEL(4, 8)
+ ret
+ CFI_ENDPROC
+ ENDPROC(\name)
+.endm
+
+#ifdef CONFIG_FRAME_POINTER
+
+.macro FUNC_ENTER_FP name
+ FUNC_ENTER_NO_FP \name
+ push_cfi %_ASM_BP
+ CFI_REL_OFFSET _ASM_BP, 0
+ _ASM_MOV %_ASM_SP, %_ASM_BP
+ CFI_DEF_CFA_REGISTER _ASM_BP
+.endm
+
+.macro FUNC_RETURN_FP name
+ pop_cfi %_ASM_BP
+ CFI_RESTORE _ASM_BP
+ FUNC_RETURN_NO_FP \name
+.endm
+
+/*
+ * Every callable asm function should be bookended with FUNC_ENTER and
+ * FUNC_RETURN. They do proper frame pointer and DWARF CFI setups in order to
+ * achieve more reliable stack traces.
+ *
+ * For the sake of simplicity and correct DWARF annotations, use of the macros
+ * requires that the return instruction comes at the end of the function.
+ */
+#define FUNC_ENTER(name) FUNC_ENTER_FP name
+#define FUNC_RETURN(name) FUNC_RETURN_FP name
+
+/*
+ * RET_NOVALIDATE tells the stack validation script to whitelist the return
+ * instruction immediately after the macro. Only use it if you're completely
+ * sure you need a return instruction outside of a callable function.
+ * Otherwise, if the code can be called and you haven't annotated it with
+ * FUNC_ENTER/FUNC_RETURN, it will break stack trace reliability.
+ */
+.macro RET_NOVALIDATE
+ 163:
+ .pushsection __stackvalidate_whitelist_ret, "ae"
+ _ASM_ALIGN
+ .long 163b - .
+ .popsection
+.endm
+
+/*
+ * FILE_NOVALIDATE is like RET_NOVALIDATE except it whitelists the entire file.
+ * Use with extreme caution or you will silently break stack traces.
+ */
+.macro FILE_NOVALIDATE
+ .pushsection __stackvalidate_whitelist_file, "ae"
+ .long 0
+ .popsection
+.endm
+
+#else /* !FRAME_POINTER */
+
+#define FUNC_ENTER(name) FUNC_ENTER_NO_FP name
+#define FUNC_RETURN(name) FUNC_RETURN_NO_FP name
+#define RET_NOVALIDATE
+#define FILE_NOVALIDATE
+
+#endif /* FRAME_POINTER */
+
+#endif /* _ASM_X86_FUNC_H */
--
2.1.0

2015-05-20 10:33:46

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

* Josh Poimboeuf <[email protected]> wrote:

> In discussions around the live kernel patching consistency model RFC
> [1], Peter and Ingo correctly pointed out that stack traces aren't
> reliable. And as Ingo said, there's no "strong force" which ensures we
> can rely on them.
>
> So I've been thinking about how to fix that. My goal is to eventually
> make stack traces reliable. Or at the very least, to be able to detect
> at runtime when a given stack trace *might* be unreliable. But improved
> stack traces would broadly benefit the entire kernel, regardless of the
> outcome of the live kernel patching consistency model discussions.
>
> This patch set is just the first in a series of proposed stack trace
> reliability improvements. Future proposals will include runtime stack
> reliability checking, as well as compile-time and runtime DWARF
> validations.
>
> As far as I can tell, there are two main obstacles which prevent frame
> pointer based stack traces from being reliable:
>
> 1) Missing frame pointer logic: currently, most assembly functions don't
> set up the frame pointer.

Could you please paste here the output of what the new checks print
for x86/64 defconfig?

> As a first step, all reported non-compliances result in warnings.
> Right now I'm seeing 200+ warnings. Once we get them all cleaned
> up, we can change the warnings to build errors so the asm code can
> stay clean.

That's quite a bit ...

Thanks,

Ingo

2015-05-20 14:14:16

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Wed, May 20, 2015 at 12:33:39PM +0200, Ingo Molnar wrote:
>
> * Josh Poimboeuf <[email protected]> wrote:
>
> > In discussions around the live kernel patching consistency model RFC
> > [1], Peter and Ingo correctly pointed out that stack traces aren't
> > reliable. And as Ingo said, there's no "strong force" which ensures we
> > can rely on them.
> >
> > So I've been thinking about how to fix that. My goal is to eventually
> > make stack traces reliable. Or at the very least, to be able to detect
> > at runtime when a given stack trace *might* be unreliable. But improved
> > stack traces would broadly benefit the entire kernel, regardless of the
> > outcome of the live kernel patching consistency model discussions.
> >
> > This patch set is just the first in a series of proposed stack trace
> > reliability improvements. Future proposals will include runtime stack
> > reliability checking, as well as compile-time and runtime DWARF
> > validations.
> >
> > As far as I can tell, there are two main obstacles which prevent frame
> > pointer based stack traces from being reliable:
> >
> > 1) Missing frame pointer logic: currently, most assembly functions don't
> > set up the frame pointer.
>
> Could you please paste here the output of what the new checks print
> for x86/64 defconfig?

Here are all 89 warnings from defconfig:

arch/x86/ia32/ia32entry.o: ia32_sysenter_target() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/ia32/ia32entry.o: return instruction outside of a function at .entry.text+0x52e. Please use FUNC_ENTER.
arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x359. Please use FUNC_ENTER.
arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19be. Please use FUNC_ENTER.
arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19e5. Please use FUNC_ENTER.
arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1c21. Please use FUNC_ENTER.
arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1ceb. Please use FUNC_ENTER.
arch/x86/kernel/acpi/wakeup_64.o: wakeup_long64() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/platform/efi/efi_stub_64.o: efi_call() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x6b. Please use FUNC_ENTER.
arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0xc7. Please use FUNC_ENTER.
arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x110. Please use FUNC_ENTER.
arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x145. Please use FUNC_ENTER.
arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x1c4. Please use FUNC_ENTER.
arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x170. Please use FUNC_ENTER.
arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x176. Please use FUNC_ENTER.
arch/x86/kernel/head_64.o: return instruction outside of a function at .head.text+0x1a2. Please use FUNC_ENTER.
arch/x86/kernel/head_64.o: start_cpu0() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/kernel/head_64.o: early_idt_handler() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/realmode/rm/reboot.o: return instruction outside of a function at .text+0x2a. Please use FUNC_ENTER.
arch/x86/realmode/rm/copy.o: memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/realmode/rm/copy.o: memset() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/realmode/rm/copy.o: copy_from_fs() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/realmode/rm/copy.o: copy_to_fs() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/realmode/rm/bioscall.o: intcall() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/vdso/vdso32/int80.o: __kernel_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/vdso/vdso32/int80.o: __kernel_rt_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/vdso/vdso32/int80.o: __kernel_vsyscall() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/vdso/vdso32/syscall.o: __kernel_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/vdso/vdso32/syscall.o: __kernel_rt_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/vdso/vdso32/syscall.o: __kernel_vsyscall() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/vdso/vdso32/sysenter.o: __kernel_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/vdso/vdso32/sysenter.o: __kernel_rt_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x69. Please use FUNC_ENTER.
arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x16d. Please use FUNC_ENTER.
arch/x86/lib/msr-reg.o: rdmsr_safe_regs() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/msr-reg.o: wrmsr_safe_regs() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/iomap_copy_64.o: __iowrite32_copy() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/clear_page_64.o: clear_page() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/clear_page_64.o: clear_page_orig() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/clear_page_64.o: clear_page_c_e() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/cmpxchg16b_emu.o: this_cpu_cmpxchg16b_emu() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/copy_page_64.o: copy_page() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/copy_page_64.o: copy_page_regs() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/copy_user_64.o: _copy_to_user() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/copy_user_64.o: _copy_from_user() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/copy_user_64.o: copy_user_generic_unrolled() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/copy_user_64.o: copy_user_generic_string() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/copy_user_64.o: copy_user_enhanced_fast_string() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/copy_user_64.o: __copy_user_nocache() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/copy_user_64.o: bad_from_user() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/csum-copy_64.o: csum_partial_copy_generic() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/getuser.o: __get_user_1() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/getuser.o: __get_user_2() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/getuser.o: __get_user_4() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/getuser.o: __get_user_8() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/getuser.o: return instruction outside of a function at .text+0xc5. Please use FUNC_ENTER.
arch/x86/lib/memcpy_64.o: memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/memcpy_64.o: __memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/memcpy_64.o: memcpy_erms() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/memcpy_64.o: memcpy_orig() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/memmove_64.o: memmove() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/memmove_64.o: __memmove() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/memmove_64.o: return instruction outside of a function at .altinstr_replacement+0x5. Please use FUNC_ENTER.
arch/x86/lib/memset_64.o: memset() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/memset_64.o: __memset() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/memset_64.o: memset_erms() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/memset_64.o: memset_orig() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/putuser.o: __put_user_1() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/putuser.o: __put_user_2() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/putuser.o: __put_user_4() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/putuser.o: __put_user_8() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/putuser.o: return instruction outside of a function at .text+0xc1. Please use FUNC_ENTER.
arch/x86/lib/rwsem.o: call_rwsem_down_read_failed() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/rwsem.o: call_rwsem_down_write_failed() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/rwsem.o: call_rwsem_wake() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/lib/rwsem.o: call_rwsem_downgrade_wake() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/boot/bioscall.o: intcall() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/boot/copy.o: memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/boot/copy.o: memset() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/boot/copy.o: copy_from_fs() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/boot/copy.o: copy_to_fs() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/boot/pmjump.o: protected_mode_jump() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/boot/pmjump.o: in_pm32() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x16e. Please use FUNC_ENTER.
arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x172. Please use FUNC_ENTER.
arch/x86/boot/compressed/head_64.o: startup_32() is missing frame pointer logic. Please use FUNC_ENTER.
arch/x86/boot/header.o: die() is missing frame pointer logic. Please use FUNC_ENTER.

> > As a first step, all reported non-compliances result in warnings.
> > Right now I'm seeing 200+ warnings. Once we get them all cleaned
> > up, we can change the warnings to build errors so the asm code can
> > stay clean.
>
> That's quite a bit ...

Yeah, a Fedora-based config has over 200 warnings. Most of the
differences between the above 89 warnings for defconfig and the 200+ for
a Fedora config seem to be caused by xen, crypto and bpf.

--
Josh

2015-05-20 14:48:17

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

* Josh Poimboeuf <[email protected]> wrote:

> On Wed, May 20, 2015 at 12:33:39PM +0200, Ingo Molnar wrote:
> >
> > * Josh Poimboeuf <[email protected]> wrote:
> >
> > > In discussions around the live kernel patching consistency model RFC
> > > [1], Peter and Ingo correctly pointed out that stack traces aren't
> > > reliable. And as Ingo said, there's no "strong force" which ensures we
> > > can rely on them.
> > >
> > > So I've been thinking about how to fix that. My goal is to eventually
> > > make stack traces reliable. Or at the very least, to be able to detect
> > > at runtime when a given stack trace *might* be unreliable. But improved
> > > stack traces would broadly benefit the entire kernel, regardless of the
> > > outcome of the live kernel patching consistency model discussions.
> > >
> > > This patch set is just the first in a series of proposed stack trace
> > > reliability improvements. Future proposals will include runtime stack
> > > reliability checking, as well as compile-time and runtime DWARF
> > > validations.
> > >
> > > As far as I can tell, there are two main obstacles which prevent frame
> > > pointer based stack traces from being reliable:
> > >
> > > 1) Missing frame pointer logic: currently, most assembly functions don't
> > > set up the frame pointer.
> >
> > Could you please paste here the output of what the new checks print
> > for x86/64 defconfig?
>
> Here are all 89 warnings from defconfig:
>
> arch/x86/ia32/ia32entry.o: ia32_sysenter_target() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/ia32/ia32entry.o: return instruction outside of a function at .entry.text+0x52e. Please use FUNC_ENTER.
> arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x359. Please use FUNC_ENTER.
> arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19be. Please use FUNC_ENTER.
> arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19e5. Please use FUNC_ENTER.
> arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1c21. Please use FUNC_ENTER.
> arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1ceb. Please use FUNC_ENTER.
> arch/x86/kernel/acpi/wakeup_64.o: wakeup_long64() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/platform/efi/efi_stub_64.o: efi_call() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x6b. Please use FUNC_ENTER.
> arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0xc7. Please use FUNC_ENTER.
> arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x110. Please use FUNC_ENTER.
> arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x145. Please use FUNC_ENTER.
> arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x1c4. Please use FUNC_ENTER.
> arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x170. Please use FUNC_ENTER.
> arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x176. Please use FUNC_ENTER.
> arch/x86/kernel/head_64.o: return instruction outside of a function at .head.text+0x1a2. Please use FUNC_ENTER.
> arch/x86/kernel/head_64.o: start_cpu0() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/kernel/head_64.o: early_idt_handler() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/realmode/rm/reboot.o: return instruction outside of a function at .text+0x2a. Please use FUNC_ENTER.
> arch/x86/realmode/rm/copy.o: memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/realmode/rm/copy.o: memset() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/realmode/rm/copy.o: copy_from_fs() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/realmode/rm/copy.o: copy_to_fs() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/realmode/rm/bioscall.o: intcall() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/vdso/vdso32/int80.o: __kernel_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/vdso/vdso32/int80.o: __kernel_rt_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/vdso/vdso32/int80.o: __kernel_vsyscall() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/vdso/vdso32/syscall.o: __kernel_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/vdso/vdso32/syscall.o: __kernel_rt_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/vdso/vdso32/syscall.o: __kernel_vsyscall() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/vdso/vdso32/sysenter.o: __kernel_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/vdso/vdso32/sysenter.o: __kernel_rt_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x69. Please use FUNC_ENTER.
> arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x16d. Please use FUNC_ENTER.
> arch/x86/lib/msr-reg.o: rdmsr_safe_regs() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/msr-reg.o: wrmsr_safe_regs() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/iomap_copy_64.o: __iowrite32_copy() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/clear_page_64.o: clear_page() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/clear_page_64.o: clear_page_orig() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/clear_page_64.o: clear_page_c_e() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/cmpxchg16b_emu.o: this_cpu_cmpxchg16b_emu() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/copy_page_64.o: copy_page() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/copy_page_64.o: copy_page_regs() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/copy_user_64.o: _copy_to_user() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/copy_user_64.o: _copy_from_user() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/copy_user_64.o: copy_user_generic_unrolled() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/copy_user_64.o: copy_user_generic_string() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/copy_user_64.o: copy_user_enhanced_fast_string() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/copy_user_64.o: __copy_user_nocache() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/copy_user_64.o: bad_from_user() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/csum-copy_64.o: csum_partial_copy_generic() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/getuser.o: __get_user_1() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/getuser.o: __get_user_2() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/getuser.o: __get_user_4() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/getuser.o: __get_user_8() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/getuser.o: return instruction outside of a function at .text+0xc5. Please use FUNC_ENTER.
> arch/x86/lib/memcpy_64.o: memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/memcpy_64.o: __memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/memcpy_64.o: memcpy_erms() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/memcpy_64.o: memcpy_orig() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/memmove_64.o: memmove() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/memmove_64.o: __memmove() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/memmove_64.o: return instruction outside of a function at .altinstr_replacement+0x5. Please use FUNC_ENTER.
> arch/x86/lib/memset_64.o: memset() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/memset_64.o: __memset() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/memset_64.o: memset_erms() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/memset_64.o: memset_orig() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/putuser.o: __put_user_1() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/putuser.o: __put_user_2() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/putuser.o: __put_user_4() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/putuser.o: __put_user_8() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/putuser.o: return instruction outside of a function at .text+0xc1. Please use FUNC_ENTER.
> arch/x86/lib/rwsem.o: call_rwsem_down_read_failed() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/rwsem.o: call_rwsem_down_write_failed() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/rwsem.o: call_rwsem_wake() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/lib/rwsem.o: call_rwsem_downgrade_wake() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/boot/bioscall.o: intcall() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/boot/copy.o: memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/boot/copy.o: memset() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/boot/copy.o: copy_from_fs() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/boot/copy.o: copy_to_fs() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/boot/pmjump.o: protected_mode_jump() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/boot/pmjump.o: in_pm32() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x16e. Please use FUNC_ENTER.
> arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x172. Please use FUNC_ENTER.
> arch/x86/boot/compressed/head_64.o: startup_32() is missing frame pointer logic. Please use FUNC_ENTER.
> arch/x86/boot/header.o: die() is missing frame pointer logic. Please use FUNC_ENTER.

Yeah, so many of these seem to be 'leaf only' functions: functions
that don't ever call functions themselves.

So lets assume we always have CONFIG_FRAME_POINTERS=y.

If they don't set up a frame pointer then they in essence won't show
up in the call chain - but normally they wouldn't because they call
nothing.

If they trigger an exception/fault or if they get hit by an interrupt
then I think we'll still correctly walk the stack - just those
functions might be missing from the deterministic call chain, right?
(it will still show up as a '?' entry.)

If they crash then we'll see them because the crashing RIP will be
printed.

So I'm wondering what the x86 policy here should be: to create frame
pointers in them or not. Cc:-ed a few more gents for thoughts.

Thanks,

Ingo

2015-05-20 15:53:00

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Wed, May 20, 2015 at 04:48:10PM +0200, Ingo Molnar wrote:
>
> * Josh Poimboeuf <[email protected]> wrote:
>
> > On Wed, May 20, 2015 at 12:33:39PM +0200, Ingo Molnar wrote:
> > >
> > > * Josh Poimboeuf <[email protected]> wrote:
> > >
> > > > In discussions around the live kernel patching consistency model RFC
> > > > [1], Peter and Ingo correctly pointed out that stack traces aren't
> > > > reliable. And as Ingo said, there's no "strong force" which ensures we
> > > > can rely on them.
> > > >
> > > > So I've been thinking about how to fix that. My goal is to eventually
> > > > make stack traces reliable. Or at the very least, to be able to detect
> > > > at runtime when a given stack trace *might* be unreliable. But improved
> > > > stack traces would broadly benefit the entire kernel, regardless of the
> > > > outcome of the live kernel patching consistency model discussions.
> > > >
> > > > This patch set is just the first in a series of proposed stack trace
> > > > reliability improvements. Future proposals will include runtime stack
> > > > reliability checking, as well as compile-time and runtime DWARF
> > > > validations.
> > > >
> > > > As far as I can tell, there are two main obstacles which prevent frame
> > > > pointer based stack traces from being reliable:
> > > >
> > > > 1) Missing frame pointer logic: currently, most assembly functions don't
> > > > set up the frame pointer.
> > >
> > > Could you please paste here the output of what the new checks print
> > > for x86/64 defconfig?
> >
> > Here are all 89 warnings from defconfig:
> >
> > arch/x86/ia32/ia32entry.o: ia32_sysenter_target() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/ia32/ia32entry.o: return instruction outside of a function at .entry.text+0x52e. Please use FUNC_ENTER.
> > arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x359. Please use FUNC_ENTER.
> > arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19be. Please use FUNC_ENTER.
> > arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19e5. Please use FUNC_ENTER.
> > arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1c21. Please use FUNC_ENTER.
> > arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1ceb. Please use FUNC_ENTER.
> > arch/x86/kernel/acpi/wakeup_64.o: wakeup_long64() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/platform/efi/efi_stub_64.o: efi_call() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x6b. Please use FUNC_ENTER.
> > arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0xc7. Please use FUNC_ENTER.
> > arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x110. Please use FUNC_ENTER.
> > arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x145. Please use FUNC_ENTER.
> > arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x1c4. Please use FUNC_ENTER.
> > arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x170. Please use FUNC_ENTER.
> > arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x176. Please use FUNC_ENTER.
> > arch/x86/kernel/head_64.o: return instruction outside of a function at .head.text+0x1a2. Please use FUNC_ENTER.
> > arch/x86/kernel/head_64.o: start_cpu0() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/kernel/head_64.o: early_idt_handler() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/realmode/rm/reboot.o: return instruction outside of a function at .text+0x2a. Please use FUNC_ENTER.
> > arch/x86/realmode/rm/copy.o: memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/realmode/rm/copy.o: memset() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/realmode/rm/copy.o: copy_from_fs() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/realmode/rm/copy.o: copy_to_fs() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/realmode/rm/bioscall.o: intcall() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/int80.o: __kernel_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/int80.o: __kernel_rt_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/int80.o: __kernel_vsyscall() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/syscall.o: __kernel_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/syscall.o: __kernel_rt_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/syscall.o: __kernel_vsyscall() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/sysenter.o: __kernel_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/vdso/vdso32/sysenter.o: __kernel_rt_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x69. Please use FUNC_ENTER.
> > arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x16d. Please use FUNC_ENTER.
> > arch/x86/lib/msr-reg.o: rdmsr_safe_regs() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/msr-reg.o: wrmsr_safe_regs() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/iomap_copy_64.o: __iowrite32_copy() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/clear_page_64.o: clear_page() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/clear_page_64.o: clear_page_orig() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/clear_page_64.o: clear_page_c_e() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/cmpxchg16b_emu.o: this_cpu_cmpxchg16b_emu() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/copy_page_64.o: copy_page() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/copy_page_64.o: copy_page_regs() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/copy_user_64.o: _copy_to_user() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/copy_user_64.o: _copy_from_user() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/copy_user_64.o: copy_user_generic_unrolled() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/copy_user_64.o: copy_user_generic_string() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/copy_user_64.o: copy_user_enhanced_fast_string() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/copy_user_64.o: __copy_user_nocache() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/copy_user_64.o: bad_from_user() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/csum-copy_64.o: csum_partial_copy_generic() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/getuser.o: __get_user_1() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/getuser.o: __get_user_2() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/getuser.o: __get_user_4() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/getuser.o: __get_user_8() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/getuser.o: return instruction outside of a function at .text+0xc5. Please use FUNC_ENTER.
> > arch/x86/lib/memcpy_64.o: memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/memcpy_64.o: __memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/memcpy_64.o: memcpy_erms() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/memcpy_64.o: memcpy_orig() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/memmove_64.o: memmove() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/memmove_64.o: __memmove() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/memmove_64.o: return instruction outside of a function at .altinstr_replacement+0x5. Please use FUNC_ENTER.
> > arch/x86/lib/memset_64.o: memset() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/memset_64.o: __memset() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/memset_64.o: memset_erms() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/memset_64.o: memset_orig() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/putuser.o: __put_user_1() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/putuser.o: __put_user_2() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/putuser.o: __put_user_4() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/putuser.o: __put_user_8() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/putuser.o: return instruction outside of a function at .text+0xc1. Please use FUNC_ENTER.
> > arch/x86/lib/rwsem.o: call_rwsem_down_read_failed() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/rwsem.o: call_rwsem_down_write_failed() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/rwsem.o: call_rwsem_wake() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/lib/rwsem.o: call_rwsem_downgrade_wake() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/boot/bioscall.o: intcall() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/boot/copy.o: memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/boot/copy.o: memset() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/boot/copy.o: copy_from_fs() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/boot/copy.o: copy_to_fs() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/boot/pmjump.o: protected_mode_jump() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/boot/pmjump.o: in_pm32() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x16e. Please use FUNC_ENTER.
> > arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x172. Please use FUNC_ENTER.
> > arch/x86/boot/compressed/head_64.o: startup_32() is missing frame pointer logic. Please use FUNC_ENTER.
> > arch/x86/boot/header.o: die() is missing frame pointer logic. Please use FUNC_ENTER.
>
> Yeah, so many of these seem to be 'leaf only' functions: functions
> that don't ever call functions themselves.

Yeah, good observation.

> So lets assume we always have CONFIG_FRAME_POINTERS=y.
>
> If they don't set up a frame pointer then they in essence won't show
> up in the call chain

It's actually the _caller_ of the asm function which gets skipped in the
trace.

(Though it doesn't really matter -- either way it's unreliable.)

> but normally they wouldn't because they call nothing.
>
> If they trigger an exception/fault or if they get hit by an interrupt
> then I think we'll still correctly walk the stack - just those
> functions might be missing from the deterministic call chain, right?
> (it will still show up as a '?' entry.)

Right. This patch set takes the more conservative approach of requiring
_all_ callable asm functions to have frame pointer logic. Which has the
benefit of getting rid of some of the cases where we need the '?' stack
entries.

> If they crash then we'll see them because the crashing RIP will be
> printed.
>
> So I'm wondering what the x86 policy here should be: to create frame
> pointers in them or not. Cc:-ed a few more gents for thoughts.

I agree that frame pointers aren't strictly necessary for leaf
functions.

I could easily relax the stackvalidate restrictions to exclude the
checking of leaf functions. In fact I think that would be more
consistent with how gcc does it, so maybe that's a more reasonable
approach.

--
Josh

2015-05-20 16:04:05

by Andy Lutomirski

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Wed, May 20, 2015 at 7:48 AM, Ingo Molnar <[email protected]> wrote:
>
> * Josh Poimboeuf <[email protected]> wrote:
>
>> On Wed, May 20, 2015 at 12:33:39PM +0200, Ingo Molnar wrote:
>> >
>> > * Josh Poimboeuf <[email protected]> wrote:
>> >
>> > > In discussions around the live kernel patching consistency model RFC
>> > > [1], Peter and Ingo correctly pointed out that stack traces aren't
>> > > reliable. And as Ingo said, there's no "strong force" which ensures we
>> > > can rely on them.
>> > >
>> > > So I've been thinking about how to fix that. My goal is to eventually
>> > > make stack traces reliable. Or at the very least, to be able to detect
>> > > at runtime when a given stack trace *might* be unreliable. But improved
>> > > stack traces would broadly benefit the entire kernel, regardless of the
>> > > outcome of the live kernel patching consistency model discussions.
>> > >
>> > > This patch set is just the first in a series of proposed stack trace
>> > > reliability improvements. Future proposals will include runtime stack
>> > > reliability checking, as well as compile-time and runtime DWARF
>> > > validations.
>> > >
>> > > As far as I can tell, there are two main obstacles which prevent frame
>> > > pointer based stack traces from being reliable:
>> > >
>> > > 1) Missing frame pointer logic: currently, most assembly functions don't
>> > > set up the frame pointer.
>> >
>> > Could you please paste here the output of what the new checks print
>> > for x86/64 defconfig?
>>
>> Here are all 89 warnings from defconfig:
>>
>> arch/x86/ia32/ia32entry.o: ia32_sysenter_target() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/ia32/ia32entry.o: return instruction outside of a function at .entry.text+0x52e. Please use FUNC_ENTER.
>> arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x359. Please use FUNC_ENTER.
>> arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19be. Please use FUNC_ENTER.
>> arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19e5. Please use FUNC_ENTER.
>> arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1c21. Please use FUNC_ENTER.
>> arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1ceb. Please use FUNC_ENTER.
>> arch/x86/kernel/acpi/wakeup_64.o: wakeup_long64() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/platform/efi/efi_stub_64.o: efi_call() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x6b. Please use FUNC_ENTER.
>> arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0xc7. Please use FUNC_ENTER.
>> arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x110. Please use FUNC_ENTER.
>> arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x145. Please use FUNC_ENTER.
>> arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x1c4. Please use FUNC_ENTER.
>> arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x170. Please use FUNC_ENTER.
>> arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x176. Please use FUNC_ENTER.
>> arch/x86/kernel/head_64.o: return instruction outside of a function at .head.text+0x1a2. Please use FUNC_ENTER.
>> arch/x86/kernel/head_64.o: start_cpu0() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/kernel/head_64.o: early_idt_handler() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/realmode/rm/reboot.o: return instruction outside of a function at .text+0x2a. Please use FUNC_ENTER.
>> arch/x86/realmode/rm/copy.o: memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/realmode/rm/copy.o: memset() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/realmode/rm/copy.o: copy_from_fs() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/realmode/rm/copy.o: copy_to_fs() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/realmode/rm/bioscall.o: intcall() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/vdso/vdso32/int80.o: __kernel_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/vdso/vdso32/int80.o: __kernel_rt_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/vdso/vdso32/int80.o: __kernel_vsyscall() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/vdso/vdso32/syscall.o: __kernel_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/vdso/vdso32/syscall.o: __kernel_rt_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/vdso/vdso32/syscall.o: __kernel_vsyscall() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/vdso/vdso32/sysenter.o: __kernel_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/vdso/vdso32/sysenter.o: __kernel_rt_sigreturn() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x69. Please use FUNC_ENTER.
>> arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x16d. Please use FUNC_ENTER.
>> arch/x86/lib/msr-reg.o: rdmsr_safe_regs() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/msr-reg.o: wrmsr_safe_regs() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/iomap_copy_64.o: __iowrite32_copy() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/clear_page_64.o: clear_page() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/clear_page_64.o: clear_page_orig() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/clear_page_64.o: clear_page_c_e() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/cmpxchg16b_emu.o: this_cpu_cmpxchg16b_emu() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/copy_page_64.o: copy_page() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/copy_page_64.o: copy_page_regs() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/copy_user_64.o: _copy_to_user() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/copy_user_64.o: _copy_from_user() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/copy_user_64.o: copy_user_generic_unrolled() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/copy_user_64.o: copy_user_generic_string() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/copy_user_64.o: copy_user_enhanced_fast_string() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/copy_user_64.o: __copy_user_nocache() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/copy_user_64.o: bad_from_user() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/csum-copy_64.o: csum_partial_copy_generic() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/getuser.o: __get_user_1() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/getuser.o: __get_user_2() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/getuser.o: __get_user_4() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/getuser.o: __get_user_8() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/getuser.o: return instruction outside of a function at .text+0xc5. Please use FUNC_ENTER.
>> arch/x86/lib/memcpy_64.o: memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/memcpy_64.o: __memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/memcpy_64.o: memcpy_erms() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/memcpy_64.o: memcpy_orig() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/memmove_64.o: memmove() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/memmove_64.o: __memmove() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/memmove_64.o: return instruction outside of a function at .altinstr_replacement+0x5. Please use FUNC_ENTER.
>> arch/x86/lib/memset_64.o: memset() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/memset_64.o: __memset() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/memset_64.o: memset_erms() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/memset_64.o: memset_orig() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/putuser.o: __put_user_1() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/putuser.o: __put_user_2() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/putuser.o: __put_user_4() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/putuser.o: __put_user_8() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/putuser.o: return instruction outside of a function at .text+0xc1. Please use FUNC_ENTER.
>> arch/x86/lib/rwsem.o: call_rwsem_down_read_failed() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/rwsem.o: call_rwsem_down_write_failed() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/rwsem.o: call_rwsem_wake() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/lib/rwsem.o: call_rwsem_downgrade_wake() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/boot/bioscall.o: intcall() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/boot/copy.o: memcpy() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/boot/copy.o: memset() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/boot/copy.o: copy_from_fs() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/boot/copy.o: copy_to_fs() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/boot/pmjump.o: protected_mode_jump() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/boot/pmjump.o: in_pm32() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x16e. Please use FUNC_ENTER.
>> arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x172. Please use FUNC_ENTER.
>> arch/x86/boot/compressed/head_64.o: startup_32() is missing frame pointer logic. Please use FUNC_ENTER.
>> arch/x86/boot/header.o: die() is missing frame pointer logic. Please use FUNC_ENTER.
>
> Yeah, so many of these seem to be 'leaf only' functions: functions
> that don't ever call functions themselves.
>
> So lets assume we always have CONFIG_FRAME_POINTERS=y.
>
> If they don't set up a frame pointer then they in essence won't show
> up in the call chain - but normally they wouldn't because they call
> nothing.
>
> If they trigger an exception/fault or if they get hit by an interrupt
> then I think we'll still correctly walk the stack - just those
> functions might be missing from the deterministic call chain, right?
> (it will still show up as a '?' entry.)

I've never quite understood what the '?' means.

>
> If they crash then we'll see them because the crashing RIP will be
> printed.
>
> So I'm wondering what the x86 policy here should be: to create frame
> pointers in them or not. Cc:-ed a few more gents for thoughts.
>

I think it would be nice to have full DWARF unwind support for
everything at some point. Unfortunately, I don't see any easy path to
getting there. It doesn't help that AFAIK no one has ever proposed a
usable in-kernel DWARF unwinder.

It also doesn't help that writing correct CFI annotations in inline
asm can be very complicated.

I think that ia64 manages to have complete unwind support. How did
they manage it?

If we had an unwinder, it would be relatively straightforward to write
something perf-based that would frequently check that we can unwind
all the way out of an NMI back to userspace and warn if we couldn't.

--Andy

2015-05-20 16:10:10

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Wed, May 20, 2015 at 10:51:56AM -0500, Josh Poimboeuf wrote:
> On Wed, May 20, 2015 at 04:48:10PM +0200, Ingo Molnar wrote:
> > Yeah, so many of these seem to be 'leaf only' functions: functions
> > that don't ever call functions themselves.
>
> Yeah, good observation.
>
> > So lets assume we always have CONFIG_FRAME_POINTERS=y.
> >
> > If they don't set up a frame pointer then they in essence won't show
> > up in the call chain
>
> It's actually the _caller_ of the asm function which gets skipped in the
> trace.
>
> (Though it doesn't really matter -- either way it's unreliable.)
>
> > but normally they wouldn't because they call nothing.
> >
> > If they trigger an exception/fault or if they get hit by an interrupt
> > then I think we'll still correctly walk the stack - just those
> > functions might be missing from the deterministic call chain, right?
> > (it will still show up as a '?' entry.)
>
> Right. This patch set takes the more conservative approach of requiring
> _all_ callable asm functions to have frame pointer logic. Which has the
> benefit of getting rid of some of the cases where we need the '?' stack
> entries.
>
> > If they crash then we'll see them because the crashing RIP will be
> > printed.
> >
> > So I'm wondering what the x86 policy here should be: to create frame
> > pointers in them or not. Cc:-ed a few more gents for thoughts.
>
> I agree that frame pointers aren't strictly necessary for leaf
> functions.
>
> I could easily relax the stackvalidate restrictions to exclude the
> checking of leaf functions. In fact I think that would be more
> consistent with how gcc does it, so maybe that's a more reasonable
> approach.

I remembered another reason why I went with the more conservative
approach of requiring frame pointers in leaf functions.

It's often hard to pin down where an asm function begins and where it
ends. For example, you might have something like:

ENTRY(callable_asm_func)
jmp label
ENDPROC(callable_asm_func)

label:
call some_c_function
ret

If we were to relax the stackvalidate restrictions then we'd miss that
kind of (surprisingly common) situation, where a function jumps outside
of its scope.

Then again, I guess it would be pretty easy to add checks for that.

--
Josh

2015-05-20 16:25:43

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Wed, May 20, 2015 at 09:03:37AM -0700, Andy Lutomirski wrote:
> On Wed, May 20, 2015 at 7:48 AM, Ingo Molnar <[email protected]> wrote:
> > Yeah, so many of these seem to be 'leaf only' functions: functions
> > that don't ever call functions themselves.
> >
> > So lets assume we always have CONFIG_FRAME_POINTERS=y.
> >
> > If they don't set up a frame pointer then they in essence won't show
> > up in the call chain - but normally they wouldn't because they call
> > nothing.
> >
> > If they trigger an exception/fault or if they get hit by an interrupt
> > then I think we'll still correctly walk the stack - just those
> > functions might be missing from the deterministic call chain, right?
> > (it will still show up as a '?' entry.)
>
> I've never quite understood what the '?' means.

It basically means "here's a function address we found on the stack,
which may or may not have been called." It's needed because stack
walking isn't currently 100% reliable.

> > If they crash then we'll see them because the crashing RIP will be
> > printed.
> >
> > So I'm wondering what the x86 policy here should be: to create frame
> > pointers in them or not. Cc:-ed a few more gents for thoughts.
> >
>
> I think it would be nice to have full DWARF unwind support for
> everything at some point. Unfortunately, I don't see any easy path to
> getting there. It doesn't help that AFAIK no one has ever proposed a
> usable in-kernel DWARF unwinder.
>
> It also doesn't help that writing correct CFI annotations in inline
> asm can be very complicated.
>
> I think that ia64 manages to have complete unwind support. How did
> they manage it?
>
> If we had an unwinder, it would be relatively straightforward to write
> something perf-based that would frequently check that we can unwind
> all the way out of an NMI back to userspace and warn if we couldn't.

I agree that DWARF unwind support would be nice. I have some plans
about how to achieve that in future patch sets. It includes several
pieces:

- compile-time DWARF data validation (using some similar approaches to
this patch set)

- run time DWARF data validation, including:
- a DWARF unwinder which doesn't blindly trust any of the DWARF data
- ensuring DWARF and frame pointer data are consistent with each other
- ensuring it can walk all the way to the bottom of the stack
- a DEBUG option which validates the stack periodically from an NMI
and/or schedule()

--
Josh

2015-05-20 16:40:27

by Andy Lutomirski

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Wed, May 20, 2015 at 9:25 AM, Josh Poimboeuf <[email protected]> wrote:
> On Wed, May 20, 2015 at 09:03:37AM -0700, Andy Lutomirski wrote:
>> On Wed, May 20, 2015 at 7:48 AM, Ingo Molnar <[email protected]> wrote:
>> > Yeah, so many of these seem to be 'leaf only' functions: functions
>> > that don't ever call functions themselves.
>> >
>> > So lets assume we always have CONFIG_FRAME_POINTERS=y.
>> >
>> > If they don't set up a frame pointer then they in essence won't show
>> > up in the call chain - but normally they wouldn't because they call
>> > nothing.
>> >
>> > If they trigger an exception/fault or if they get hit by an interrupt
>> > then I think we'll still correctly walk the stack - just those
>> > functions might be missing from the deterministic call chain, right?
>> > (it will still show up as a '?' entry.)
>>
>> I've never quite understood what the '?' means.
>
> It basically means "here's a function address we found on the stack,
> which may or may not have been called." It's needed because stack
> walking isn't currently 100% reliable.
>
>> > If they crash then we'll see them because the crashing RIP will be
>> > printed.
>> >
>> > So I'm wondering what the x86 policy here should be: to create frame
>> > pointers in them or not. Cc:-ed a few more gents for thoughts.
>> >
>>
>> I think it would be nice to have full DWARF unwind support for
>> everything at some point. Unfortunately, I don't see any easy path to
>> getting there. It doesn't help that AFAIK no one has ever proposed a
>> usable in-kernel DWARF unwinder.
>>
>> It also doesn't help that writing correct CFI annotations in inline
>> asm can be very complicated.
>>
>> I think that ia64 manages to have complete unwind support. How did
>> they manage it?
>>
>> If we had an unwinder, it would be relatively straightforward to write
>> something perf-based that would frequently check that we can unwind
>> all the way out of an NMI back to userspace and warn if we couldn't.
>
> I agree that DWARF unwind support would be nice. I have some plans
> about how to achieve that in future patch sets. It includes several
> pieces:
>
> - compile-time DWARF data validation (using some similar approaches to
> this patch set)
>
> - run time DWARF data validation, including:
> - a DWARF unwinder which doesn't blindly trust any of the DWARF data

Fantastic!

> - ensuring DWARF and frame pointer data are consistent with each other
> - ensuring it can walk all the way to the bottom of the stack
> - a DEBUG option which validates the stack periodically from an NMI
> and/or schedule()

We think alike :)

NMI will be much more interesting than schedule.

--Andy

2015-05-20 16:52:30

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Wed, May 20, 2015 at 11:25:37AM -0500, Josh Poimboeuf wrote:
> > I've never quite understood what the '?' means.
>
> It basically means "here's a function address we found on the stack,
> which may or may not have been called." It's needed because stack
> walking isn't currently 100% reliable.

Yeah, that was not that trivial to figure out at the time:

unsigned long
print_context_stack(struct thread_info *tinfo,
...

if (__kernel_text_address(addr)) {
if ((unsigned long) stack == bp + sizeof(long)) {
ops->address(data, addr, 1);
frame = frame->next_frame;
bp = (unsigned long) frame;
} else {
ops->address(data, addr, 0);
}

and that ops->address is

print_trace_address()
|-> printk_stack_address()

So if I'm understanding this correctly, if rBP+8 is equal to rSP, i.e.
return address is on the stack, then this frame got called.

Otherwise -> "?".

I might be missing something though...

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

2015-05-20 16:59:22

by Linus Torvalds

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Wed, May 20, 2015 at 9:25 AM, Josh Poimboeuf <[email protected]> wrote:
> On Wed, May 20, 2015 at 09:03:37AM -0700, Andy Lutomirski wrote:
>>
>> I've never quite understood what the '?' means.
>
> It basically means "here's a function address we found on the stack,
> which may or may not have been called." It's needed because stack
> walking isn't currently 100% reliable.

It is often quite interesting and helpful, because it shows stale data
on the stack, giving clues about what happened just before.

Now, I'd like gcc to generally be better about not wasting so much
stack frame, so in that sense I'd like to see fewer '?" entries just
from a code quality standpoint, but when debugging those things, the
downside of "noise" is often cancelled by the upside of "ahh, it
happens after calling X".

So the "perfect stack frames" is actually not as great a thing as some
people want to make it seem.

Linus

2015-05-20 17:21:49

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Wed, May 20, 2015 at 09:59:18AM -0700, Linus Torvalds wrote:
> On Wed, May 20, 2015 at 9:25 AM, Josh Poimboeuf <[email protected]> wrote:
> > On Wed, May 20, 2015 at 09:03:37AM -0700, Andy Lutomirski wrote:
> >>
> >> I've never quite understood what the '?' means.
> >
> > It basically means "here's a function address we found on the stack,
> > which may or may not have been called." It's needed because stack
> > walking isn't currently 100% reliable.
>
> It is often quite interesting and helpful, because it shows stale data
> on the stack, giving clues about what happened just before.
>
> Now, I'd like gcc to generally be better about not wasting so much
> stack frame, so in that sense I'd like to see fewer '?" entries just
> from a code quality standpoint, but when debugging those things, the
> downside of "noise" is often cancelled by the upside of "ahh, it
> happens after calling X".
>
> So the "perfect stack frames" is actually not as great a thing as some
> people want to make it seem.

Ok, I can see how looking at stale stack data could be useful for some
of the really tough problems.

But right now, the meaning of '?' is ambiguous. It could be stale data,
or it could be part of a frame for the current stack which was skipped
due to missing frame pointers or an exception.

If we can somehow make the stack unwinder reliable, then it would at
least allow us to remove the ambiguity of the '?' entries. And it would
reduce the "noise" for the majority of issues where we don't care about
stale stack data, and can simply ignore it.

--
Josh

2015-05-20 17:27:24

by Peter Zijlstra

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Wed, May 20, 2015 at 09:03:37AM -0700, Andy Lutomirski wrote:
> I think it would be nice to have full DWARF unwind support for
> everything at some point. Unfortunately, I don't see any easy path to
> getting there. It doesn't help that AFAIK no one has ever proposed a
> usable in-kernel DWARF unwinder.

There's a bit of history here; SuSE (iirc) actually has one, however:

https://lkml.org/lkml/2012/2/10/356

2015-05-20 19:10:21

by Jiri Kosina

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Wed, 20 May 2015, Peter Zijlstra wrote:

> > I think it would be nice to have full DWARF unwind support for
> > everything at some point. Unfortunately, I don't see any easy path to
> > getting there. It doesn't help that AFAIK no one has ever proposed a
> > usable in-kernel DWARF unwinder.
>
> There's a bit of history here; SuSE (iirc) actually has one, however:
>
> https://lkml.org/lkml/2012/2/10/356

Oh absolutely, there are stories behind this :)

Just for the sake of completness -- the current implementation can be
found in our public GIT repository, for not-really-complete picture see
[1] [2] [3] [4].

It turned out to be rather useful on many ocasions when debugging customer
reports, but I of course also understand what Linus is saying above. The
bugs in unwinder can be *really* painful. Our experience so far has been
that it did pay off at the end of the day (and of course analyzing
stacktraces is our daily bread).

[1] http://kernel.suse.com/cgit/kernel-source/tree/patches.suse/stack-unwind?h=SLE12
[2] http://kernel.suse.com/cgit/kernel-source/tree/patches.suse/no-frame-pointer-select?h=SLE12
[3] http://kernel.suse.com/cgit/kernel-source/tree/patches.arch/stack-unwind-cfi_ignore-takes-more-arguments?h=SLE12
[4] http://kernel.suse.com/cgit/kernel-source/tree/patches.arch/x86_64-unwind-annotations?h=SLE12

--
Jiri Kosina
SUSE Labs

2015-05-21 07:52:37

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

* Linus Torvalds <[email protected]> wrote:

> On Wed, May 20, 2015 at 9:25 AM, Josh Poimboeuf <[email protected]> wrote:
> > On Wed, May 20, 2015 at 09:03:37AM -0700, Andy Lutomirski wrote:
> >>
> >> I've never quite understood what the '?' means.
> >
> > It basically means "here's a function address we found on the
> > stack, which may or may not have been called." It's needed
> > because stack walking isn't currently 100% reliable.
>
> It is often quite interesting and helpful, because it shows stale
> data on the stack, giving clues about what happened just before.

Yes, it's basically a zero-cost tracer: often showing a partial trace
of events that happened before.

> Now, I'd like gcc to generally be better about not wasting so much
> stack frame, so in that sense I'd like to see fewer '?" entries just
> from a code quality standpoint, but when debugging those things, the
> downside of "noise" is often cancelled by the upside of "ahh, it
> happens after calling X".
>
> So the "perfect stack frames" is actually not as great a thing as
> some people want to make it seem.

We should definitely also print out the '?' entries, they are very
useful especially when analyzing rare, difficult to reproduce,
sporadic bugs - which are usually the hardest to fix bugs.

The biggest long term plus of 'perfect stack frames' would not be to
skip the '?' entries (we don't want to skip them!), but to be able to
eventually build the kernel without frame pointers.

Especially on modern x86 CPUs with stack engines (latest Intel and AMD
CPUs) that keeps ESP updates out of the later stages of execution
pipelines, going from RBP framepointers to direct ESP use is
beneficial to performance and compresses I$ footprint as well:

text data bss dec hex filename
12150606 2565544 1634304 16350454 f97cf6 linux-CONFIG_FRAME_POINTERS=n/vmlinux
13282884 2571744 1617920 17472548 10a9c24 linux-CONFIG_FRAME_POINTERS=y/vmlinux

Here's the I$ cachemiss rate with the 'vfs-mix' workload that I used
in the -falign-functions measuremenst gives this for
CONFIG_FRAMEPOINTERS=y, on Intel Sandy Bridge (best of 9x10 runs):

#
# CONFIG_FRAMEPOINTERS=y
#
Performance counter stats for 'system wide' (10 runs):

728,328,347 L1-icache-load-misses ( +- 0.08% ) (100.00%)
11,891,931,664 instructions ( +- 0.00% )
300,023 context-switches ( +- 0.00% )

7.324048170 seconds time elapsed ( +- 0.09% )

... and these are the I$ miss perf stats from running the same
workload on a CONFIG_FRAMEPOINTERS=n kernel:

#
# CONFIG_FRAMEPOINTERS are not set
#
Performance counter stats for 'system wide' (10 runs):

687,758,078 L1-icache-load-misses ( +- 0.10% ) (100.00%)
10,984,908,013 instructions ( +- 0.01% )
300,021 context-switches ( +- 0.00% )

7.120867260 seconds time elapsed ( +- 0.29% )

So if we disable frame pointers, then on this workload:

- the kernel text size is 9.3% smaller
- the number of instructions executed went down by about 8.2%
- the cachemiss rate went down by about 5.9%
- performance went up by about 2.8%.

The speedup is actually even better than 2.8%, if you look at average
execution time:

linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.324048170 seconds time elapsed ( +- 0.09% )
linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.470166715 seconds time elapsed ( +- 1.01% )
linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.365047474 seconds time elapsed ( +- 0.25% )
linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.828223324 seconds time elapsed ( +- 2.04% )
linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.427164489 seconds time elapsed ( +- 0.70% )
linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.385565350 seconds time elapsed ( +- 0.35% )
linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.560782318 seconds time elapsed ( +- 1.68% )
linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.399741309 seconds time elapsed ( +- 0.74% )
linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.303746766 seconds time elapsed ( +- 0.04% )

avg = 7.451609

linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.201498813 seconds time elapsed ( +- 0.86% )
linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.120867260 seconds time elapsed ( +- 0.29% )
linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.141642635 seconds time elapsed ( +- 0.15% )
linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.217213506 seconds time elapsed ( +- 0.85% )
linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.163046581 seconds time elapsed ( +- 0.56% )
linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.128939439 seconds time elapsed ( +- 0.23% )
linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.256172853 seconds time elapsed ( +- 0.82% )
linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.122946768 seconds time elapsed ( +- 0.23% )
linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.126018578 seconds time elapsed ( +- 0.18% )

avg = 7.164260

Then with framepointers disabled this workload gets faster by 4.0% on
average.

The average result is also pretty stable in the no-framepointers case,
while it fluctuates more in the framepointers case. (and this is why
the 'best runtime' favors the framepointers case - the average is
closer to reality.)

So the performance advantages of not doing framepointers is not
something we can ignore IMHO: but obviously performance isn't
everything - so if stack unwinding is unrobust, then we need and
want frame pointers.

Thanks,

Ingo

2015-05-21 10:16:21

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

* Borislav Petkov <[email protected]> wrote:

> On Wed, May 20, 2015 at 11:25:37AM -0500, Josh Poimboeuf wrote:
> > > I've never quite understood what the '?' means.
> >
> > It basically means "here's a function address we found on the stack,
> > which may or may not have been called." It's needed because stack
> > walking isn't currently 100% reliable.
>
> Yeah, that was not that trivial to figure out at the time:
>
> unsigned long
> print_context_stack(struct thread_info *tinfo,
> ...
>
> if (__kernel_text_address(addr)) {
> if ((unsigned long) stack == bp + sizeof(long)) {
> ops->address(data, addr, 1);
> frame = frame->next_frame;
> bp = (unsigned long) frame;
> } else {
> ops->address(data, addr, 0);
> }
>
> and that ops->address is
>
> print_trace_address()
> |-> printk_stack_address()
>
> So if I'm understanding this correctly, if rBP+8 is equal to rSP, i.e.
> return address is on the stack, then this frame got called.
>
> Otherwise -> "?".
>
> I might be missing something though...

So this is how we are printing backtraces on x86:

We always scan the full kernel stack for return addresses stored on
the kernel stack(s) [*], from stack top to stack bottom, and print out
anything that 'looks like' a kernel text address.

If it fits into the frame pointer chain, we print it without a
question mark, knowing that it's part of the real backtrace.

If the address does not fit into our expected frame pointer chain we
still print it, but we print a '?'. It can mean two things:

- either the address is not part of the call chain: it's just stale
values on the kernel stack, from earlier function calls. This is
the common case.

- or it is part of the call chain, but the frame pointer was not set
up properly within the function, so we don't recognize it. See the
200+ assembly functions that Josh's build time validation found.

This way we will always print out the real call chain (plus a few more
entries), regardless of whether the frame pointer was set up correctly
or not - but in most cases we'll get the call chain right as well. The
entries printed are strictly in stack order, so you can deduce more
information from that as well.

The most important property of this method is that we _never_ lose
information: we always strive to print _all_ addresses on the stack(s)
that look like kernel text addresses, so if debug information is
wrong, we still print out the real call chain as well - just with more
question marks than ideal.

Thanks,

Ingo

[*] For things like IRQ stacks and ISTs we also scan those stacks, in
the right order, and try to cross from one stack into another
reconstructing the call chain. This works most of the time.

2015-05-21 10:27:18

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

* Josh Poimboeuf <[email protected]> wrote:

> On Wed, May 20, 2015 at 09:59:18AM -0700, Linus Torvalds wrote:
> > On Wed, May 20, 2015 at 9:25 AM, Josh Poimboeuf <[email protected]> wrote:
> > > On Wed, May 20, 2015 at 09:03:37AM -0700, Andy Lutomirski wrote:
> > >>
> > >> I've never quite understood what the '?' means.
> > >
> > > It basically means "here's a function address we found on the
> > > stack, which may or may not have been called." It's needed
> > > because stack walking isn't currently 100% reliable.
> >
> > It is often quite interesting and helpful, because it shows stale
> > data on the stack, giving clues about what happened just before.
> >
> > Now, I'd like gcc to generally be better about not wasting so much
> > stack frame, so in that sense I'd like to see fewer '?" entries
> > just from a code quality standpoint, but when debugging those
> > things, the downside of "noise" is often cancelled by the upside
> > of "ahh, it happens after calling X".
> >
> > So the "perfect stack frames" is actually not as great a thing as
> > some people want to make it seem.
>
> Ok, I can see how looking at stale stack data could be useful for
> some of the really tough problems.

And note that the tough problems are actually the ones where we need
that information the most. So any stack backtrace printing method must
be biased towards helping the difficult scenarios - not the trivial
crashes. That is one of the reasons why we are always printing the
question marks.

> But right now, the meaning of '?' is ambiguous. It could be stale
> data, or it could be part of a frame for the current stack which was
> skipped due to missing frame pointers or an exception.

Yes, of course. That's not a big problem as the actual symbolic
information will tell us a lot, which allows us to reconstruct the
real call chain, plus allows us to see any 'recent execution activity'
that might be on the stack as stale entries.

> If we can somehow make the stack unwinder reliable, then it would at
> least allow us to remove the ambiguity of the '?' entries. And it
> would reduce the "noise" for the majority of issues where we don't
> care about stale stack data, and can simply ignore it.

Yes, but note the above consideration - the probability distribution
of kernel bugs tends to have a _very_ long tail, with bugs that
sometimes take years to trigger and fix. Kernel developers upstream
and at distros tend to spend a disproportionately large amount of time
staring at difficult to decode bugs.

For that reason it is far more important to still stay maintainable
with those kinds of difficult bugs, than to make the resolution of
trivial, unambiguous crashes a tiny bit easier by printing fewer
'distractions'...

Also, note that the '?' entries have another role: they cross-check
the unwinder.

If you think we'll be able to do a perfect unwinder then think again:
debug info _will_ be messed up periodically, either by us or by
tooling, because right now no kernel code or other functionality
relies on perfect unwinding.

So this is not like C++ exception handling where broken unwinding will
break the code. This is something that is literally only visible in
kernel logs currently, as a slight anomaly.

So any x86 stack unwinder code must be fundamentally based on the idea
and expectation that stack unwinding is always going to be somewhat
imperfect and somewhat statistical.

Thanks,

Ingo

2015-05-21 10:47:21

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Thu, May 21, 2015 at 12:16:14PM +0200, Ingo Molnar wrote:
> So this is how we are printing backtraces on x86:

<snip useful info>

This is pretty useful info and the question about the '?' keeps popping
up.

How about I moved Documentation/x86/x86_64/kernel-stacks to
Documentation/x86/kernel-stacks and added that info to it?

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

2015-05-21 11:11:28

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

* Borislav Petkov <[email protected]> wrote:

> On Thu, May 21, 2015 at 12:16:14PM +0200, Ingo Molnar wrote:
> > So this is how we are printing backtraces on x86:
>
> <snip useful info>
>
> This is pretty useful info and the question about the '?' keeps popping
> up.
>
> How about I moved Documentation/x86/x86_64/kernel-stacks to
> Documentation/x86/kernel-stacks and added that info to it?

Yeah, please do!

Thanks,

Ingo

2015-05-21 12:12:27

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

* Ingo Molnar <[email protected]> wrote:

> Especially on modern x86 CPUs with stack engines (latest Intel and
> AMD CPUs) that keeps ESP updates out of the later stages of
> execution pipelines, going from RBP framepointers to direct ESP use
> is beneficial to performance and compresses I$ footprint as well:
>
> text data bss dec hex filename
> 12150606 2565544 1634304 16350454 f97cf6 linux-CONFIG_FRAME_POINTERS=n/vmlinux
> 13282884 2571744 1617920 17472548 10a9c24 linux-CONFIG_FRAME_POINTERS=y/vmlinux

Correction: I ran that with a 1-byte alignment patch still applied.

I reran all the numbers with the default 16-bytes alignment as well,
and the gap between framepointers and no-framepointers become smaller,
but the various trends and conclusions still hold.

Here are the updated numbers:

text data bss dec hex filename
13548564 2571744 1617920 17738228 10ea9f4 linux-CONFIG_FRAME_POINTERS=n/vmlinux
13797773 2571744 1617920 17987437 112776d linux-CONFIG_FRAME_POINTERS=y/vmlinux

> Here's the I$ cachemiss rate with the 'vfs-mix' workload that I used
> in the -falign-functions measuremenst gives this for
> CONFIG_FRAMEPOINTERS=y, on Intel Sandy Bridge (best of 9x10 runs):
>
> #
> # CONFIG_FRAMEPOINTERS=y
> #
> Performance counter stats for 'system wide' (10 runs):
>
> 728,328,347 L1-icache-load-misses ( +- 0.08% ) (100.00%)
> 11,891,931,664 instructions ( +- 0.00% )
> 300,023 context-switches ( +- 0.00% )
>
> 7.324048170 seconds time elapsed ( +- 0.09% )

Performance counter stats for 'system wide' (10 runs):

701,525,006 L1-icache-load-misses ( +- 0.06% ) (100.00%)
11,891,793,196 instructions ( +- 0.01% )
300,036 context-switches ( +- 0.00% )

7.354372294 seconds time elapsed ( +- 0.82% )

>
> ... and these are the I$ miss perf stats from running the same
> workload on a CONFIG_FRAMEPOINTERS=n kernel:
>
> #
> # CONFIG_FRAMEPOINTERS are not set
> #
> Performance counter stats for 'system wide' (10 runs):
>
> 687,758,078 L1-icache-load-misses ( +- 0.10% ) (100.00%)
> 10,984,908,013 instructions ( +- 0.01% )
> 300,021 context-switches ( +- 0.00% )
>
> 7.120867260 seconds time elapsed ( +- 0.29% )

Performance counter stats for 'system wide' (10 runs):

685,107,089 L1-icache-load-misses ( +- 0.08% ) (100.00%)
10,983,861,590 instructions ( +- 0.01% )
300,031 context-switches ( +- 0.00% )

7.120738452 seconds time elapsed ( +- 0.35% )

> So if we disable frame pointers, then on this workload:
>
> - the kernel text size is 9.3% smaller
> - the number of instructions executed went down by about 8.2%
> - the cachemiss rate went down by about 5.9%
> - performance went up by about 2.8%.

- the kernel text size is 1.8% smaller: with 16 bytes alignment
there's quite some extra free space the frame pointer code can
grow into, which reduces the size win.

- the number of instructions executed went down by about 8.2% (as
expected this is invariant of alignment.)

- the cachemiss rate went down by about 2.7%: this is a smaller
win again, partly because of the 'free space' 16-byte alignment
gives us.

- the best 'time elapsed' numbers out of 10 runs show a speedup of
2.0% - close to the 2.8% with 1-byte alignment.

> The speedup is actually even better than 2.8%, if you look at
> average execution time:
>
> linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.324048170 seconds time elapsed ( +- 0.09% )
> linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.470166715 seconds time elapsed ( +- 1.01% )
> linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.365047474 seconds time elapsed ( +- 0.25% )
> linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.828223324 seconds time elapsed ( +- 2.04% )
> linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.427164489 seconds time elapsed ( +- 0.70% )
> linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.385565350 seconds time elapsed ( +- 0.35% )
> linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.560782318 seconds time elapsed ( +- 1.68% )
> linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.399741309 seconds time elapsed ( +- 0.74% )
> linux-CONFIG_FRAME_POINTERS=y/res.txt: 7.303746766 seconds time elapsed ( +- 0.04% )
>
> avg = 7.451609

linux-CONFIG_FRAME_POINTERS=y/res2.txt: 7.300875812 seconds time elapsed ( +- 0.17% )
linux-CONFIG_FRAME_POINTERS=y/res2.txt: 7.491652338 seconds time elapsed ( +- 1.33% )
linux-CONFIG_FRAME_POINTERS=y/res2.txt: 7.307877300 seconds time elapsed ( +- 0.20% )
linux-CONFIG_FRAME_POINTERS=y/res2.txt: 7.258946461 seconds time elapsed ( +- 0.23% )
linux-CONFIG_FRAME_POINTERS=y/res2.txt: 7.295113779 seconds time elapsed ( +- 0.30% )
linux-CONFIG_FRAME_POINTERS=y/res2.txt: 7.283375859 seconds time elapsed ( +- 0.21% )
linux-CONFIG_FRAME_POINTERS=y/res2.txt: 7.319320205 seconds time elapsed ( +- 0.38% )
linux-CONFIG_FRAME_POINTERS=y/res2.txt: 7.354372294 seconds time elapsed ( +- 0.82% )
linux-CONFIG_FRAME_POINTERS=y/res2.txt: 7.308955558 seconds time elapsed ( +- 0.26% )
linux-CONFIG_FRAME_POINTERS=y/res2.txt: 7.295267101 seconds time elapsed ( +- 0.26% )

avg=7.32

> linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.201498813 seconds time elapsed ( +- 0.86% )
> linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.120867260 seconds time elapsed ( +- 0.29% )
> linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.141642635 seconds time elapsed ( +- 0.15% )
> linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.217213506 seconds time elapsed ( +- 0.85% )
> linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.163046581 seconds time elapsed ( +- 0.56% )
> linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.128939439 seconds time elapsed ( +- 0.23% )
> linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.256172853 seconds time elapsed ( +- 0.82% )
> linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.122946768 seconds time elapsed ( +- 0.23% )
> linux-CONFIG_FRAME_POINTERS=n/res.txt: 7.126018578 seconds time elapsed ( +- 0.18% )
>
> avg = 7.164260

linux-CONFIG_FRAME_POINTERS=n/res2.txt: 7.135061084 seconds time elapsed ( +- 0.39% )
linux-CONFIG_FRAME_POINTERS=n/res2.txt: 7.132738388 seconds time elapsed ( +- 0.34% )
linux-CONFIG_FRAME_POINTERS=n/res2.txt: 7.174334895 seconds time elapsed ( +- 0.32% )
linux-CONFIG_FRAME_POINTERS=n/res2.txt: 7.215143851 seconds time elapsed ( +- 0.71% )
linux-CONFIG_FRAME_POINTERS=n/res2.txt: 7.131166029 seconds time elapsed ( +- 0.19% )
linux-CONFIG_FRAME_POINTERS=n/res2.txt: 7.270427197 seconds time elapsed ( +- 1.22% )
linux-CONFIG_FRAME_POINTERS=n/res2.txt: 7.120738452 seconds time elapsed ( +- 0.35% )
linux-CONFIG_FRAME_POINTERS=n/res2.txt: 7.168856127 seconds time elapsed ( +- 0.27% )
linux-CONFIG_FRAME_POINTERS=n/res2.txt: 7.268637173 seconds time elapsed ( +- 1.28% )
linux-CONFIG_FRAME_POINTERS=n/res2.txt: 7.178431781 seconds time elapsed ( +- 0.32% )

avg=7.18

> Then with framepointers disabled this workload gets faster by 4.0%
> on average.

With 16-byte alignment the average gets faster by 2.8%.

The conclusions are unchanged:

> The average result is also pretty stable in the no-framepointers
> case, while it fluctuates more in the framepointers case. (and this
> is why the 'best runtime' favors the framepointers case - the
> average is closer to reality.)
>
> So the performance advantages of not doing framepointers is not
> something we can ignore IMHO: but obviously performance isn't
> everything - so if stack unwinding is unrobust, then we need and
> want frame pointers.

Thanks,

Ingo

2015-05-21 15:49:32

[permalink] [raw]

Subject: [PATCH 1/3] x86/documentation: Move kernel-stacks doc one level up

From: Borislav Petkov <[email protected]>

... to Documentation/x86/ as it is going to collect more and not only
64-bit specific info.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Michal Marek <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: X86 ML <[email protected]>
Cc: [email protected]
Cc: lkml <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Denys Vlasenko <[email protected]>
Cc: Brian Gerst <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Andrew Morton <[email protected]>
---
Documentation/x86/{x86_64 => }/kernel-stacks | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename Documentation/x86/{x86_64 => }/kernel-stacks (100%)

diff --git a/Documentation/x86/x86_64/kernel-stacks b/Documentation/x86/kernel-stacks
similarity index 100%
rename from Documentation/x86/x86_64/kernel-stacks
rename to Documentation/x86/kernel-stacks
--
2.3.5

2015-05-21 15:49:30

[permalink] [raw]

Subject: [PATCH 2/3] x86/documentation: Remove STACKFAULT_STACK bulletpoint

From: Borislav Petkov <[email protected]>

Update the documentation after

6f442be2fb22 ("x86_64, traps: Stop using IST for #SS").

Signed-off-by: Borislav Petkov <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Michal Marek <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: X86 ML <[email protected]>
Cc: [email protected]
Cc: lkml <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Denys Vlasenko <[email protected]>
Cc: Brian Gerst <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Andrew Morton <[email protected]>
---
Documentation/x86/kernel-stacks | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/Documentation/x86/kernel-stacks b/Documentation/x86/kernel-stacks
index e3c8a49d1a2f..c3c935b9d56e 100644
--- a/Documentation/x86/kernel-stacks
+++ b/Documentation/x86/kernel-stacks
@@ -1,3 +1,6 @@
+Kernel stacks on x86-64 bit
+---------------------------
+
Most of the text from Keith Owens, hacked by AK

x86_64 page size (PAGE_SIZE) is 4K.
@@ -56,13 +59,6 @@ If that assumption is ever broken then the stacks will become corrupt.

The currently assigned IST stacks are :-

-* STACKFAULT_STACK. EXCEPTION_STKSZ (PAGE_SIZE).
-
- Used for interrupt 12 - Stack Fault Exception (#SS).
-
- This allows the CPU to recover from invalid stack segments. Rarely
- happens.
-
* DOUBLEFAULT_STACK. EXCEPTION_STKSZ (PAGE_SIZE).

Used for interrupt 8 - Double Fault Exception (#DF).
--
2.3.5

2015-05-21 15:49:26

[permalink] [raw]

Subject: [PATCH 3/3] x86/documentation: Adapt Ingo's explanation on printing backtraces

From: Borislav Petkov <[email protected]>

Hold it down for future reference, as the question about the question
mark in stack traces keeps popping up.

Signed-off-by: Borislav Petkov <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Michal Marek <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: X86 ML <[email protected]>
Cc: [email protected]
Cc: lkml <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Denys Vlasenko <[email protected]>
Cc: Brian Gerst <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Andrew Morton <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
---
Documentation/x86/kernel-stacks | 44 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)

diff --git a/Documentation/x86/kernel-stacks b/Documentation/x86/kernel-stacks
index c3c935b9d56e..0f3a6c201943 100644
--- a/Documentation/x86/kernel-stacks
+++ b/Documentation/x86/kernel-stacks
@@ -95,3 +95,47 @@ The currently assigned IST stacks are :-
assumptions about the previous state of the kernel stack.

For more details see the Intel IA32 or AMD AMD64 architecture manuals.
+
+
+Printing backtraces on x86
+--------------------------
+
+The question about the '?' preceding function names in an x86 stacktrace
+keeps popping up, here's an indepth explanation. It helps if the reader
+stares at print_context_stack() and the whole machinery in and around
+arch/x86/kernel/dumpstack.c.
+
+Adapted from Ingo's mail, Message-ID: <[email protected]>:
+
+We always scan the full kernel stack for return addresses stored on
+the kernel stack(s) [*], from stack top to stack bottom, and print out
+anything that 'looks like' a kernel text address.
+
+If it fits into the frame pointer chain, we print it without a question
+mark, knowing that it's part of the real backtrace.
+
+If the address does not fit into our expected frame pointer chain we
+still print it, but we print a '?'. It can mean two things:
+
+ - either the address is not part of the call chain: it's just stale
+ values on the kernel stack, from earlier function calls. This is
+ the common case.
+
+ - or it is part of the call chain, but the frame pointer was not set
+ up properly within the function, so we don't recognize it.
+
+This way we will always print out the real call chain (plus a few more
+entries), regardless of whether the frame pointer was set up correctly
+or not - but in most cases we'll get the call chain right as well. The
+entries printed are strictly in stack order, so you can deduce more
+information from that as well.
+
+The most important property of this method is that we _never_ lose
+information: we always strive to print _all_ addresses on the stack(s)
+that look like kernel text addresses, so if debug information is wrong,
+we still print out the real call chain as well - just with more question
+marks than ideal.
+
+[*] For things like IRQ and IST stacks, we also scan those stacks, in
+ the right order, and try to cross from one stack into another
+ reconstructing the call chain. This works most of the time.
--
2.3.5

2015-05-21 21:51:04

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Wed, May 20, 2015 at 04:48:10PM +0200, Ingo Molnar wrote:
> Yeah, so many of these seem to be 'leaf only' functions: functions
> that don't ever call functions themselves.
>
> So lets assume we always have CONFIG_FRAME_POINTERS=y.
>
> If they don't set up a frame pointer then they in essence won't show
> up in the call chain - but normally they wouldn't because they call
> nothing.
>
> If they trigger an exception/fault or if they get hit by an interrupt
> then I think we'll still correctly walk the stack - just those
> functions might be missing from the deterministic call chain, right?
> (it will still show up as a '?' entry.)
>
> If they crash then we'll see them because the crashing RIP will be
> printed.
>
> So I'm wondering what the x86 policy here should be: to create frame
> pointers in them or not. Cc:-ed a few more gents for thoughts.

After removing the frame pointer checks for leaf functions, and adding a
check for all functions which jump outside of their scope, the number of
defconfig warnings dropped from 89 -> 47. The Fedora config warning
count dropped from 207 -> 83.

Here are the remaining 47 warnings for defconfig:

stackvalidate: arch/x86/ia32/ia32entry.o: ia32_sysenter_target() is missing frame pointer logic
stackvalidate: arch/x86/ia32/ia32entry.o: return instruction outside of a function at .entry.text+0x52e
stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x359
stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19be
stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19e5
stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1c21
stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1ceb
stackvalidate: arch/x86/kernel/acpi/wakeup_64.o: unsupported jump to outside of the function at wakeup_long64+0x15
stackvalidate: arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel() is missing frame pointer logic
stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x6b
stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0xc7
stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x110
stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x145
stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x1c4
stackvalidate: arch/x86/kernel/head_64.o: return instruction outside of a function at .head.text+0x1a2
stackvalidate: arch/x86/kernel/head_64.o: early_idt_handler() is missing frame pointer logic
stackvalidate: arch/x86/platform/efi/efi_stub_64.o: efi_call() is missing frame pointer logic
stackvalidate: arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x170
stackvalidate: arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x176
stackvalidate: arch/x86/realmode/rm/reboot.o: return instruction outside of a function at .text+0x2a
stackvalidate: arch/x86/realmode/rm/copy.o: copy_from_fs() is missing frame pointer logic
stackvalidate: arch/x86/realmode/rm/copy.o: copy_to_fs() is missing frame pointer logic
stackvalidate: arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x69
stackvalidate: arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x16d
stackvalidate: arch/x86/lib/copy_user_64.o: unsupported jump to outside of the function at _copy_to_user+0x25
stackvalidate: arch/x86/lib/copy_user_64.o: unsupported jump to outside of the function at _copy_from_user+0x25
stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_1+0x14
stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_2+0x4
stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_4+0x4
stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_8+0x4
stackvalidate: arch/x86/lib/getuser.o: return instruction outside of a function at .text+0xc5
stackvalidate: arch/x86/lib/memmove_64.o: return instruction outside of a function at .altinstr_replacement+0x5
stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_1+0x14
stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_2+0x1b
stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_4+0x1b
stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_8+0x1b
stackvalidate: arch/x86/lib/putuser.o: return instruction outside of a function at .text+0xc1
stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_down_read_failed() is missing frame pointer logic
stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_down_write_failed() is missing frame pointer logic
stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_wake() is missing frame pointer logic
stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_downgrade_wake() is missing frame pointer logic
stackvalidate: arch/x86/boot/copy.o: copy_from_fs() is missing frame pointer logic
stackvalidate: arch/x86/boot/copy.o: copy_to_fs() is missing frame pointer logic
stackvalidate: arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x16e
stackvalidate: arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x172
stackvalidate: arch/x86/boot/compressed/head_64.o: startup_32() is missing frame pointer logic
stackvalidate: arch/x86/boot/pmjump.o: unsupported jump to outside of the function at in_pm32+0x1c

Note that only 13 of the 47 warnings are actually due to missing frame
pointer logic. The rest are ambiguous conditions which prevent
stackvalidate from being able to make sense of things: returning from
outside of a proper ELF function, or jumping from inside of a function
to outside of its scope.

Similarly, in the Fedora config case, only 27 of the 83 warnings are for
missing frame pointer logic.

If there are no objections, I'll go with this approach in the next
version of the patch set.

Thanks!

--
Josh

2015-05-21 21:53:35

by Andy Lutomirski

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Thu, May 21, 2015 at 1:54 PM, Josh Poimboeuf <[email protected]> wrote:
> On Wed, May 20, 2015 at 04:48:10PM +0200, Ingo Molnar wrote:
>> Yeah, so many of these seem to be 'leaf only' functions: functions
>> that don't ever call functions themselves.
>>
>> So lets assume we always have CONFIG_FRAME_POINTERS=y.
>>
>> If they don't set up a frame pointer then they in essence won't show
>> up in the call chain - but normally they wouldn't because they call
>> nothing.
>>
>> If they trigger an exception/fault or if they get hit by an interrupt
>> then I think we'll still correctly walk the stack - just those
>> functions might be missing from the deterministic call chain, right?
>> (it will still show up as a '?' entry.)
>>
>> If they crash then we'll see them because the crashing RIP will be
>> printed.
>>
>> So I'm wondering what the x86 policy here should be: to create frame
>> pointers in them or not. Cc:-ed a few more gents for thoughts.
>
> After removing the frame pointer checks for leaf functions, and adding a
> check for all functions which jump outside of their scope, the number of
> defconfig warnings dropped from 89 -> 47. The Fedora config warning
> count dropped from 207 -> 83.
>
> Here are the remaining 47 warnings for defconfig:
>
> stackvalidate: arch/x86/ia32/ia32entry.o: ia32_sysenter_target() is missing frame pointer logic
> stackvalidate: arch/x86/ia32/ia32entry.o: return instruction outside of a function at .entry.text+0x52e
> stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x359
> stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19be
> stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19e5
> stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1c21
> stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1ceb
> stackvalidate: arch/x86/kernel/acpi/wakeup_64.o: unsupported jump to outside of the function at wakeup_long64+0x15
> stackvalidate: arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel() is missing frame pointer logic
> stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x6b
> stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0xc7
> stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x110
> stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x145
> stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x1c4
> stackvalidate: arch/x86/kernel/head_64.o: return instruction outside of a function at .head.text+0x1a2
> stackvalidate: arch/x86/kernel/head_64.o: early_idt_handler() is missing frame pointer logic
> stackvalidate: arch/x86/platform/efi/efi_stub_64.o: efi_call() is missing frame pointer logic
> stackvalidate: arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x170
> stackvalidate: arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x176
> stackvalidate: arch/x86/realmode/rm/reboot.o: return instruction outside of a function at .text+0x2a
> stackvalidate: arch/x86/realmode/rm/copy.o: copy_from_fs() is missing frame pointer logic
> stackvalidate: arch/x86/realmode/rm/copy.o: copy_to_fs() is missing frame pointer logic
> stackvalidate: arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x69
> stackvalidate: arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x16d
> stackvalidate: arch/x86/lib/copy_user_64.o: unsupported jump to outside of the function at _copy_to_user+0x25
> stackvalidate: arch/x86/lib/copy_user_64.o: unsupported jump to outside of the function at _copy_from_user+0x25
> stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_1+0x14
> stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_2+0x4
> stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_4+0x4
> stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_8+0x4
> stackvalidate: arch/x86/lib/getuser.o: return instruction outside of a function at .text+0xc5
> stackvalidate: arch/x86/lib/memmove_64.o: return instruction outside of a function at .altinstr_replacement+0x5
> stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_1+0x14
> stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_2+0x1b
> stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_4+0x1b
> stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_8+0x1b
> stackvalidate: arch/x86/lib/putuser.o: return instruction outside of a function at .text+0xc1
> stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_down_read_failed() is missing frame pointer logic
> stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_down_write_failed() is missing frame pointer logic
> stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_wake() is missing frame pointer logic
> stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_downgrade_wake() is missing frame pointer logic
> stackvalidate: arch/x86/boot/copy.o: copy_from_fs() is missing frame pointer logic
> stackvalidate: arch/x86/boot/copy.o: copy_to_fs() is missing frame pointer logic
> stackvalidate: arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x16e
> stackvalidate: arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x172
> stackvalidate: arch/x86/boot/compressed/head_64.o: startup_32() is missing frame pointer logic
> stackvalidate: arch/x86/boot/pmjump.o: unsupported jump to outside of the function at in_pm32+0x1c
>
> Note that only 13 of the 47 warnings are actually due to missing frame
> pointer logic. The rest are ambiguous conditions which prevent
> stackvalidate from being able to make sense of things: returning from
> outside of a proper ELF function, or jumping from inside of a function
> to outside of its scope.
>
> Similarly, in the Fedora config case, only 27 of the 83 warnings are for
> missing frame pointer logic.
>
> If there are no objections, I'll go with this approach in the next
> version of the patch set.

I'm willing to review anything with "entry" in its filename.

--Andy

2015-05-21 22:02:05

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Thu, May 21, 2015 at 03:54:25PM -0500, Josh Poimboeuf wrote:
> stackvalidate: arch/x86/lib/memmove_64.o: return instruction outside of a function at .altinstr_replacement+0x5

That must be something like this:

0000000000000000 <.altinstr_replacement>:
0: 48 89 d1 mov %rdx,%rcx
3: f3 a4 rep movsb %ds:(%rsi),%es:(%rdi)
5: c3 retq

right?

In any case, anything with alternatives is probably a false positive
because even if instructions appear outside of the containing function,
they get patched in and are actually inside. Jump offsets get fixed up
properly too. Should, at least :-)

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

2015-05-22 14:32:20

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Fri, May 22, 2015 at 12:01:58AM +0200, Borislav Petkov wrote:
> On Thu, May 21, 2015 at 03:54:25PM -0500, Josh Poimboeuf wrote:
> > stackvalidate: arch/x86/lib/memmove_64.o: return instruction outside of a function at .altinstr_replacement+0x5
>
> That must be something like this:
>
> 0000000000000000 <.altinstr_replacement>:
> 0: 48 89 d1 mov %rdx,%rcx
> 3: f3 a4 rep movsb %ds:(%rsi),%es:(%rdi)
> 5: c3 retq
>
> right?
>
> In any case, anything with alternatives is probably a false positive
> because even if instructions appear outside of the containing function,
> they get patched in and are actually inside. Jump offsets get fixed up
> properly too. Should, at least :-)

Hm, alternatives do complicate things a bit. It *is* a false positive,
but not necessarily because its part of an alternative instruction
block.

The above code would be patched into memmove(), which is a leaf function
because it doesn't call any other functions. Leaf functions don't need
frame pointer logic, so we can ignore them.

If instead the above code were patched into a non-leaf function, we'd
have to change it to restore the frame pointer before returning.

--
Josh

2015-05-22 14:54:29

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Thu, May 21, 2015 at 02:53:07PM -0700, Andy Lutomirski wrote:
> On Thu, May 21, 2015 at 1:54 PM, Josh Poimboeuf <[email protected]> wrote:
> > After removing the frame pointer checks for leaf functions, and adding a
> > check for all functions which jump outside of their scope, the number of
> > defconfig warnings dropped from 89 -> 47. The Fedora config warning
> > count dropped from 207 -> 83.
> >
> > Here are the remaining 47 warnings for defconfig:
> >
> > stackvalidate: arch/x86/ia32/ia32entry.o: ia32_sysenter_target() is missing frame pointer logic
> > stackvalidate: arch/x86/ia32/ia32entry.o: return instruction outside of a function at .entry.text+0x52e
> > stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x359
> > stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19be
> > stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x19e5
> > stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1c21
> > stackvalidate: arch/x86/kernel/entry_64.o: return instruction outside of a function at .entry.text+0x1ceb
> > stackvalidate: arch/x86/kernel/acpi/wakeup_64.o: unsupported jump to outside of the function at wakeup_long64+0x15
> > stackvalidate: arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel() is missing frame pointer logic
> > stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x6b
> > stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0xc7
> > stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x110
> > stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x145
> > stackvalidate: arch/x86/kernel/relocate_kernel_64.o: return instruction outside of a function at .text+0x1c4
> > stackvalidate: arch/x86/kernel/head_64.o: return instruction outside of a function at .head.text+0x1a2
> > stackvalidate: arch/x86/kernel/head_64.o: early_idt_handler() is missing frame pointer logic
> > stackvalidate: arch/x86/platform/efi/efi_stub_64.o: efi_call() is missing frame pointer logic
> > stackvalidate: arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x170
> > stackvalidate: arch/x86/realmode/rm/trampoline_64.o: return instruction outside of a function at .text+0x176
> > stackvalidate: arch/x86/realmode/rm/reboot.o: return instruction outside of a function at .text+0x2a
> > stackvalidate: arch/x86/realmode/rm/copy.o: copy_from_fs() is missing frame pointer logic
> > stackvalidate: arch/x86/realmode/rm/copy.o: copy_to_fs() is missing frame pointer logic
> > stackvalidate: arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x69
> > stackvalidate: arch/x86/power/hibernate_asm_64.o: return instruction outside of a function at .text+0x16d
> > stackvalidate: arch/x86/lib/copy_user_64.o: unsupported jump to outside of the function at _copy_to_user+0x25
> > stackvalidate: arch/x86/lib/copy_user_64.o: unsupported jump to outside of the function at _copy_from_user+0x25
> > stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_1+0x14
> > stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_2+0x4
> > stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_4+0x4
> > stackvalidate: arch/x86/lib/getuser.o: unsupported jump to outside of the function at __get_user_8+0x4
> > stackvalidate: arch/x86/lib/getuser.o: return instruction outside of a function at .text+0xc5
> > stackvalidate: arch/x86/lib/memmove_64.o: return instruction outside of a function at .altinstr_replacement+0x5
> > stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_1+0x14
> > stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_2+0x1b
> > stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_4+0x1b
> > stackvalidate: arch/x86/lib/putuser.o: unsupported jump to outside of the function at __put_user_8+0x1b
> > stackvalidate: arch/x86/lib/putuser.o: return instruction outside of a function at .text+0xc1
> > stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_down_read_failed() is missing frame pointer logic
> > stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_down_write_failed() is missing frame pointer logic
> > stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_wake() is missing frame pointer logic
> > stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_downgrade_wake() is missing frame pointer logic
> > stackvalidate: arch/x86/boot/copy.o: copy_from_fs() is missing frame pointer logic
> > stackvalidate: arch/x86/boot/copy.o: copy_to_fs() is missing frame pointer logic
> > stackvalidate: arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x16e
> > stackvalidate: arch/x86/boot/compressed/head_64.o: return instruction outside of a function at .text+0x172
> > stackvalidate: arch/x86/boot/compressed/head_64.o: startup_32() is missing frame pointer logic
> > stackvalidate: arch/x86/boot/pmjump.o: unsupported jump to outside of the function at in_pm32+0x1c
> >
> > Note that only 13 of the 47 warnings are actually due to missing frame
> > pointer logic. The rest are ambiguous conditions which prevent
> > stackvalidate from being able to make sense of things: returning from
> > outside of a proper ELF function, or jumping from inside of a function
> > to outside of its scope.
> >
> > Similarly, in the Fedora config case, only 27 of the 83 warnings are for
> > missing frame pointer logic.
> >
> > If there are no objections, I'll go with this approach in the next
> > version of the patch set.
>
> I'm willing to review anything with "entry" in its filename.

Thanks. I think the "entry" warnings may be false positives, since that
code isn't called by any C kernel code. (Now that I'm ignoring leaf
functions, the ratio of false positives to true positives has gone up.)

The false positives for "return instruction outside of a function" can
be marked with the RET_NOVALIDATE macro to tell stackvalidate to ignore
the return instruction, or with FILE_NOVALIDATE to tell it to ignore the
entire file.

I can add some patches to fix the warnings. I'll put you on CC for the
"entry" changes.

--
Josh

2015-05-22 21:19:09

by Jiri Kosina

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Fri, 22 May 2015, Josh Poimboeuf wrote:

> Hm, alternatives do complicate things a bit. It *is* a false positive,
> but not necessarily because its part of an alternative instruction
> block.
>
> The above code would be patched into memmove(), which is a leaf function
> because it doesn't call any other functions. Leaf functions don't need
> frame pointer logic, so we can ignore them.
>
> If instead the above code were patched into a non-leaf function, we'd
> have to change it to restore the frame pointer before returning.

Is this really only a problem of alternatives? How about
dynamically-enabled tracepoints?

--
Jiri Kosina
SUSE Labs

2015-05-22 22:22:56

[permalink] [raw]

Subject: Re: [PATCH v4 0/3] Compile-time stack frame pointer validation

On Fri, May 22, 2015 at 11:18:57PM +0200, Jiri Kosina wrote:
> On Fri, 22 May 2015, Josh Poimboeuf wrote:
>
> > Hm, alternatives do complicate things a bit. It *is* a false positive,
> > but not necessarily because its part of an alternative instruction
> > block.
> >
> > The above code would be patched into memmove(), which is a leaf function
> > because it doesn't call any other functions. Leaf functions don't need
> > frame pointer logic, so we can ignore them.
> >
> > If instead the above code were patched into a non-leaf function, we'd
> > have to change it to restore the frame pointer before returning.
>
> Is this really only a problem of alternatives? How about
> dynamically-enabled tracepoints?

I think tracepoints are only in C code, right? stackvalidate only
analyzes asm code, so it's not a concern for this patch set.

And I think tracepoints rely on normal call instructions, so they
shouldn't cause any problems with frame pointers as far as I can tell.

--
Josh

2015-05-23 08:38:41