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;
