Messages in this thread Patch in this message | | | Date | Wed, 1 Jul 2020 19:00:41 -0500 | From | Josh Poimboeuf <> | Subject | Re: objtool clac/stac handling change.. |
| |
On Wed, Jul 01, 2020 at 02:02:42PM -0700, Linus Torvalds wrote: > So the objtool rule might be: > > - in a STAC region, no exception handlers at all except for that > ex_handler_uaccess case > > - and that case will clear AC if it triggers. > > and maybe such an objtool check would show some case where I'm wrong, > and we do some MSR read other other fault thing within a STAC region. > That _sounds_ wrong to me, but maybe we have reason to do so that I > just can't think or right now?
Here's an attempt at implementing this, in case anybody wants to play with it. Usual disclaimers apply...
diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 5e0d70a89fb8..470447225314 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -24,7 +24,7 @@ struct alternative { struct list_head list; struct instruction *insn; - bool skip_orig; + bool skip_orig, exception, uaccess; }; const char *objname; @@ -1023,8 +1023,13 @@ static int add_special_section_alts(struct objtool_file *file) } alt->insn = new_insn; + alt->skip_orig = special_alt->skip_orig; + alt->exception = special_alt->exception; + alt->uaccess = special_alt->uaccess; + orig_insn->ignore_alts |= special_alt->skip_alt; + list_add_tail(&alt->list, &orig_insn->alts); list_del(&special_alt->list); @@ -2335,6 +2340,35 @@ static void fill_alternative_cfi(struct objtool_file *file, struct instruction * } } +static int handle_stac(struct symbol *func, struct instruction *insn, + struct insn_state *state) +{ + if (state->uaccess) { + WARN_FUNC("recursive UACCESS enable", insn->sec, insn->offset); + return -1; + } + + state->uaccess = true; + return 0; +} + +static int handle_clac(struct symbol *func, struct instruction *insn, + struct insn_state *state) +{ + if (!state->uaccess && func) { + WARN_FUNC("redundant UACCESS disable", insn->sec, insn->offset); + return -1; + } + + if (func_uaccess_safe(func) && !state->uaccess_stack) { + WARN_FUNC("UACCESS-safe disables UACCESS", insn->sec, insn->offset); + return -1; + } + + state->uaccess = false; + return 0; +} + /* * Follow the branch starting at the given instruction, and recursively follow * any other branches (jumps). Meanwhile, track the frame pointer state at @@ -2393,6 +2427,17 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, if (alt->skip_orig) skip_orig = true; + if (alt->exception) { + if (!alt->uaccess && state.uaccess) { + WARN_FUNC("non-user-access exception with uaccess enabled", + sec, insn->offset); + return 1; + } + + if (alt->uaccess && handle_clac(func, insn, &state)) + return 1; + } + ret = validate_branch(file, func, alt->insn, state); if (ret) { if (backtrace) @@ -2478,26 +2523,13 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, return 0; case INSN_STAC: - if (state.uaccess) { - WARN_FUNC("recursive UACCESS enable", sec, insn->offset); + if (handle_stac(func, insn, &state)) return 1; - } - - state.uaccess = true; break; case INSN_CLAC: - if (!state.uaccess && func) { - WARN_FUNC("redundant UACCESS disable", sec, insn->offset); + if (handle_clac(func, insn, &state)) return 1; - } - - if (func_uaccess_safe(func) && !state.uaccess_stack) { - WARN_FUNC("UACCESS-safe disables UACCESS", sec, insn->offset); - return 1; - } - - state.uaccess = false; break; case INSN_STD: diff --git a/tools/objtool/special.c b/tools/objtool/special.c index e74e0189de22..41f27199cae6 100644 --- a/tools/objtool/special.c +++ b/tools/objtool/special.c @@ -18,6 +18,7 @@ #define EX_ENTRY_SIZE 12 #define EX_ORIG_OFFSET 0 #define EX_NEW_OFFSET 4 +#define EX_HANDLER_OFFSET 8 #define JUMP_ENTRY_SIZE 16 #define JUMP_ORIG_OFFSET 0 @@ -35,8 +36,9 @@ struct special_entry { const char *sec; - bool group, jump_or_nop; + bool group, jump_or_nop, exception; unsigned char size, orig, new; + unsigned char handler; /* __ex_table only */ unsigned char orig_len, new_len; /* group only */ unsigned char feature; /* ALTERNATIVE macro CPU feature */ }; @@ -61,9 +63,11 @@ struct special_entry entries[] = { }, { .sec = "__ex_table", + .exception = true, .size = EX_ENTRY_SIZE, .orig = EX_ORIG_OFFSET, .new = EX_NEW_OFFSET, + .handler = EX_HANDLER_OFFSET, }, {}, }; @@ -118,6 +122,20 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry, } } + if (entry->exception) { + struct rela *handler_rela; + + alt->exception = true; + + handler_rela = find_rela_by_dest(elf, sec, offset + entry->handler); + if (!handler_rela) { + WARN_FUNC("can't find handler rela", sec, offset + entry->handler); + return -1; + } + if (!strcmp(handler_rela->sym->name, "ex_handler_uaccess")) + alt->uaccess = true; + } + orig_rela = find_rela_by_dest(elf, sec, offset + entry->orig); if (!orig_rela) { WARN_FUNC("can't find orig rela", sec, offset + entry->orig); diff --git a/tools/objtool/special.h b/tools/objtool/special.h index 35061530e46e..3a62daef14b3 100644 --- a/tools/objtool/special.h +++ b/tools/objtool/special.h @@ -16,6 +16,7 @@ struct special_alt { bool skip_orig; bool skip_alt; bool jump_or_nop; + bool exception, uaccess; struct section *orig_sec; unsigned long orig_off;
| |