| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20200702000041.movaiqpyzdmhlu67@treble>
Date: Wed, 1 Jul 2020 19:00:41 -0500
From: Josh Poimboeuf <jpoimboe@...hat.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Andy Lutomirski <luto@...capital.net>,
Andy Lutomirski <luto@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
the arch/x86 maintainers <x86@...nel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
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;
Powered by blists - more mailing lists