Re: [PATCH] sched/x86: Save [ER]FLAGS on context switch
flat view
From: Peter Zijlstra <peterz@infradead.org>
Date: 2019-02-22 12:58:41
Also in:
lkml
Subsystem:
objtool, the rest, x86 architecture (32-bit and 64-bit) · Maintainers:
Josh Poimboeuf, Peter Zijlstra, Linus Torvalds, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen
On Thu, Feb 21, 2019 at 02:08:11PM -0800, Linus Torvalds wrote:
On Thu, Feb 21, 2019 at 1:35 PM Thomas Gleixner [off-list ref] wrote:quoted
IMNSHO any call inside a AC region is a bug lurking round the corner. The only thing which is tolerable is an exception of some sort. Enforce that with objtool. End of story.Not quite that simple. There are some use cases where you really do want a call - albeit to special functions - inside the AC region. The prime example of this is likely "filldir()" in fs/readdir.c, which is actually somewhat important for some loads (eg samba). And the whole AC thing made it horribly horribly slow because readdir is one of those things that doesn't just copy one value (or one structure) to user space, but writes several different things. Kind of like signal frame setup does. You may not realize that right now signal frame setup *does* actually do a call with AC set. See ia32_setup_rt_frame() that does put_user_try { ... compat_save_altstack_ex(&frame->uc.uc_stack, regs->sp); ... } put_user_catch(err); is a macro, but inside that macro is a call to sas_ss_reset(). Now, that is an inline function, so it hopefully doesn't actually cause a call. But it's "only" an inline function, not "always_inline". We did already have (and I rejected, for now) patches that made those inlines not very strong...
That one does indeed not generate a call, but: arch/x86/ia32/ia32_signal.o: warning: objtool: ia32_restore_sigcontext()+0x38: call to native_load_gs_index() with AC set I've yet to look at detectoring __preempt_count mucking. ---
diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
index 43c029cdc3fe..cd31e4433f4c 100644
--- a/arch/x86/include/asm/special_insns.h
+++ b/arch/x86/include/asm/special_insns.h@@ -5,6 +5,7 @@ #ifdef __KERNEL__ +#include <linux/frame.h> #include <asm/nops.h> /*
@@ -135,6 +136,7 @@ static inline void native_wbinvd(void) } extern asmlinkage void native_load_gs_index(unsigned); +AC_SAFE(native_load_gs_index); static inline unsigned long __read_cr4(void) {
diff --git a/include/linux/frame.h b/include/linux/frame.h
index 02d3ca2d9598..644662d8b8e8 100644
--- a/include/linux/frame.h
+++ b/include/linux/frame.h@@ -15,9 +15,14 @@ static void __used __section(.discard.func_stack_frame_non_standard) \ *__func_stack_frame_non_standard_##func = func +#define AC_SAFE(func) \ + static void __used __section(.discard.ac_safe) \ + *__func_ac_safe_##func = func + #else /* !CONFIG_STACK_VALIDATION */ #define STACK_FRAME_NON_STANDARD(func) +#define AC_SAFE(func) #endif /* CONFIG_STACK_VALIDATION */
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index c9d038f91af6..5a64e5fb9d7a 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile@@ -31,7 +31,7 @@ INCLUDES := -I$(srctree)/tools/include \ -I$(srctree)/tools/arch/$(HOSTARCH)/include/uapi \ -I$(srctree)/tools/objtool/arch/$(ARCH)/include WARNINGS := $(EXTRA_WARNINGS) -Wno-switch-default -Wno-switch-enum -Wno-packed -CFLAGS += -Werror $(WARNINGS) $(KBUILD_HOSTCFLAGS) -g $(INCLUDES) +CFLAGS += -Werror $(WARNINGS) $(KBUILD_HOSTCFLAGS) -g $(INCLUDES) -ggdb3 LDFLAGS += -lelf $(LIBSUBCMD) $(KBUILD_HOSTLDFLAGS) # Allow old libelf to be used:
diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h
index b0d7dc3d71b5..48327099466d 100644
--- a/tools/objtool/arch.h
+++ b/tools/objtool/arch.h@@ -33,7 +33,9 @@ #define INSN_STACK 8 #define INSN_BUG 9 #define INSN_NOP 10 -#define INSN_OTHER 11 +#define INSN_STAC 11 +#define INSN_CLAC 12 +#define INSN_OTHER 13 #define INSN_LAST INSN_OTHER enum op_dest_type {
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 540a209b78ab..d1e99d1460a5 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c@@ -369,7 +369,19 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, case 0x0f: - if (op2 >= 0x80 && op2 <= 0x8f) { + if (op2 == 0x01) { + + if (modrm == 0xca) { + + *type = INSN_CLAC; + + } else if (modrm == 0xcb) { + + *type = INSN_STAC; + + } + + } else if (op2 >= 0x80 && op2 <= 0x8f) { *type = INSN_JUMP_CONDITIONAL;
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 0414a0d52262..6fef157e244b 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c@@ -127,6 +127,24 @@ static bool ignore_func(struct objtool_file *file, struct symbol *func) return false; } +static bool ac_safe_func(struct objtool_file *file, struct symbol *func) +{ + struct rela *rela; + + /* check for AC_SAFE */ + if (file->ac_safe && file->ac_safe->rela) + list_for_each_entry(rela, &file->ac_safe->rela->rela_list, list) { + if (rela->sym->type == STT_SECTION && + rela->sym->sec == func->sec && + rela->addend == func->offset) + return true; + if (/* rela->sym->type == STT_FUNC && */ rela->sym == func) + return true; + } + + return false; +} + /* * This checks to see if the given function is a "noreturn" function. *
@@ -439,6 +457,8 @@ static void add_ignores(struct objtool_file *file) for_each_sec(file, sec) { list_for_each_entry(func, &sec->symbol_list, list) { + func->ac_safe = ac_safe_func(file, func); + if (func->type != STT_FUNC) continue;
@@ -1897,11 +1917,16 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, if (ret) return 1; } - } + } else printf("ponies\n"); switch (insn->type) { case INSN_RETURN: + if (state.ac) { + WARN_FUNC("return with AC set", sec, insn->offset); + return 1; + } + if (func && has_modified_stack_frame(&state)) { WARN_FUNC("return with modified stack frame", sec, insn->offset);
@@ -1917,6 +1942,12 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, return 0; case INSN_CALL: + if (state.ac && !insn->call_dest->ac_safe) { + WARN_FUNC("call to %s() with AC set", sec, insn->offset, + insn->call_dest->name); + return 1; + } + if (is_fentry_call(insn)) break;
@@ -1928,6 +1959,11 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, /* fallthrough */ case INSN_CALL_DYNAMIC: + if (state.ac && !insn->call_dest->ac_safe) { + WARN_FUNC("call to %s() with AC set", sec, insn->offset, + insn->call_dest->name); + return 1; + } if (!no_fp && func && !has_valid_stack_frame(&state)) { WARN_FUNC("call without frame pointer save/setup", sec, insn->offset);
@@ -1980,6 +2016,14 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, break; + case INSN_STAC: + state.ac = true; + break; + + case INSN_CLAC: + state.ac = false; + break; + default: break; }
@@ -2198,6 +2242,7 @@ int check(const char *_objname, bool orc) INIT_LIST_HEAD(&file.insn_list); hash_init(file.insn_hash); file.whitelist = find_section_by_name(file.elf, ".discard.func_stack_frame_non_standard"); + file.ac_safe = find_section_by_name(file.elf, ".discard.ac_safe"); file.c_file = find_section_by_name(file.elf, ".comment"); file.ignore_unreachables = no_unreachable; file.hints = false;
diff --git a/tools/objtool/check.h b/tools/objtool/check.h
index e6e8a655b556..d4ae59454fff 100644
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h@@ -31,7 +31,7 @@ struct insn_state { int stack_size; unsigned char type; bool bp_scratch; - bool drap, end; + bool drap, end, ac; int drap_reg, drap_offset; struct cfi_reg vals[CFI_NUM_REGS]; };
@@ -61,6 +61,7 @@ struct objtool_file { struct list_head insn_list; DECLARE_HASHTABLE(insn_hash, 16); struct section *whitelist; + struct section *ac_safe; bool ignore_unreachables, c_file, hints, rodata; };
diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
index bc97ed86b9cd..064c3df31e40 100644
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h@@ -62,6 +62,7 @@ struct symbol { unsigned long offset; unsigned int len; struct symbol *pfunc, *cfunc; + bool ac_safe; }; struct rela {
diff --git a/tools/objtool/special.c b/tools/objtool/special.c
index 50af4e1274b3..72084cae8f35 100644
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c@@ -166,8 +166,8 @@ int special_get_alts(struct elf *elf, struct list_head *alts) continue; if (sec->len % entry->size != 0) { - WARN("%s size not a multiple of %d", - sec->name, entry->size); + WARN("%s size (%d) not a multiple of %d", + sec->name, sec->len, entry->size); return -1; }
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel