From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>,
Andy Lutomirski <luto@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
the arch/x86 maintainers <x86@kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: objtool clac/stac handling change..
Date: Wed, 1 Jul 2020 19:00:41 -0500 [thread overview]
Message-ID: <20200702000041.movaiqpyzdmhlu67@treble> (raw)
In-Reply-To: <CAHk-=wiY-67yt4kGd2EW-7kChQgnLHZ_2aJ+=ps7i7rCz894PQ@mail.gmail.com>
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;
next prev parent reply other threads:[~2020-07-02 0:00 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-01 18:22 objtool clac/stac handling change Linus Torvalds
2020-07-01 18:29 ` Andy Lutomirski
2020-07-01 19:35 ` Linus Torvalds
2020-07-01 20:36 ` Andy Lutomirski
2020-07-01 20:51 ` Josh Poimboeuf
2020-07-01 21:02 ` Linus Torvalds
2020-07-02 0:00 ` Josh Poimboeuf [this message]
2020-07-02 8:05 ` Peter Zijlstra
2020-07-01 20:51 ` Linus Torvalds
2020-07-02 0:47 ` Andy Lutomirski
2020-07-02 2:30 ` Linus Torvalds
2020-07-02 2:35 ` Linus Torvalds
2020-07-02 3:08 ` Andy Lutomirski
2020-07-01 18:41 ` Al Viro
2020-07-01 19:04 ` Linus Torvalds
2020-07-01 19:59 ` Al Viro
2020-07-01 20:25 ` Linus Torvalds
2020-07-02 13:34 ` Michael Ellerman
2020-07-02 14:01 ` Al Viro
2020-07-02 14:04 ` Al Viro
2020-07-02 15:13 ` Christophe Leroy
2020-07-02 20:13 ` Linus Torvalds
2020-07-03 3:59 ` Michael Ellerman
2020-07-03 3:17 ` Michael Ellerman
2020-07-03 5:27 ` Christophe Leroy
2020-07-02 19:52 ` Linus Torvalds
2020-07-02 20:17 ` Al Viro
2020-07-02 20:32 ` Linus Torvalds
2020-07-02 20:59 ` Al Viro
2020-07-02 21:55 ` Linus Torvalds
2020-07-03 1:33 ` Al Viro
2020-07-03 3:32 ` Linus Torvalds
2020-07-03 21:02 ` Al Viro
2020-07-03 21:10 ` Linus Torvalds
2020-07-03 21:41 ` Andy Lutomirski
2020-07-03 22:25 ` Al Viro
2020-07-03 21:59 ` Al Viro
2020-07-03 22:04 ` Al Viro
2020-07-03 22:12 ` Al Viro
2020-07-04 0:49 ` Al Viro
2020-07-04 1:54 ` Linus Torvalds
2020-07-04 2:30 ` Al Viro
2020-07-04 3:06 ` Linus Torvalds
2020-07-04 2:11 ` Al Viro
2020-07-07 12:35 ` David Laight
2020-07-10 22:37 ` Linus Torvalds
2020-07-13 9:32 ` David Laight
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200702000041.movaiqpyzdmhlu67@treble \
--to=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=luto@kernel.org \
--cc=peterz@infradead.org \
--cc=torvalds@linux-foundation.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).