| 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: <20190307115200.697533978@infradead.org>
Date: Thu, 07 Mar 2019 12:45:29 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: torvalds@...ux-foundation.org, tglx@...utronix.de, hpa@...or.com,
julien.thierry@....com, will.deacon@....com, luto@...capital.net,
mingo@...nel.org, catalin.marinas@....com, james.morse@....com,
valentin.schneider@....com, brgerst@...il.com, jpoimboe@...hat.com,
luto@...nel.org, bp@...en8.de, dvlasenk@...hat.com
Cc: linux-kernel@...r.kernel.org, peterz@...radead.org,
dvyukov@...gle.com, rostedt@...dmis.org
Subject: [PATCH 18/20] objtool: Add UACCESS validation
It is important that UACCESS regions are as small as possible;
furthermore the UACCESS state is not scheduled, so doing anything that
might directly call into the scheduler will cause random code to be
ran with UACCESS enabled.
Teach objtool too track UACCESS state and warn about any CALL made
while UACCESS is enabled. This very much includes the __fentry__()
and __preempt_schedule() calls.
Note that exceptions _do_ save/restore the UACCESS state, and therefore
they can drive preemption. This also means that all exception handlers
must have an otherwise redundant UACCESS disable instruction;
therefore ignore this warning for !STT_FUNC code (exception handlers
are not normal functions).
XXX: users hard-coded list of uaccess-safe functions because I've
failed to come up with a sensible annotation and the list should be
fairly static.
XXX: are we sure we want __memset marked AC-safe?
Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
---
scripts/Makefile.build | 3
tools/objtool/arch.h | 4
tools/objtool/arch/x86/decode.c | 14 +++
tools/objtool/builtin-check.c | 3
tools/objtool/builtin.h | 2
tools/objtool/check.c | 164 +++++++++++++++++++++++++++++++++++++---
tools/objtool/check.h | 2
tools/objtool/elf.h | 1
tools/objtool/special.c | 10 ++
9 files changed, 188 insertions(+), 15 deletions(-)
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -223,6 +223,9 @@ endif
ifdef CONFIG_RETPOLINE
objtool_args += --retpoline
endif
+ifdef CONFIG_X86_SMAP
+ objtool_args += --uaccess
+endif
# 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
# 'OBJECT_FILES_NON_STANDARD_foo.o := 'y': skip objtool checking for a file
--- 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 {
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -369,7 +369,19 @@ int arch_decode_instruction(struct elf *
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;
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -29,7 +29,7 @@
#include "builtin.h"
#include "check.h"
-bool no_fp, no_unreachable, retpoline, module, backtrace;
+bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess;
static const char * const check_usage[] = {
"objtool check [<options>] file.o",
@@ -42,6 +42,7 @@ const struct option check_options[] = {
OPT_BOOLEAN('r', "retpoline", &retpoline, "Validate retpoline assumptions"),
OPT_BOOLEAN('m', "module", &module, "Indicates the object will be part of a kernel module"),
OPT_BOOLEAN('b', "backtrace", &backtrace, "unwind on error"),
+ OPT_BOOLEAN('a', "uaccess", &uaccess, "enable uaccess checking"),
OPT_END(),
};
--- a/tools/objtool/builtin.h
+++ b/tools/objtool/builtin.h
@@ -20,7 +20,7 @@
#include <subcmd/parse-options.h>
extern const struct option check_options[];
-extern bool no_fp, no_unreachable, retpoline, module, backtrace;
+extern bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess;
extern int cmd_check(int argc, const char **argv);
extern int cmd_orc(int argc, const char **argv);
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -442,6 +442,81 @@ static void add_ignores(struct objtool_f
}
}
+static const char *uaccess_safe_builtin[] = {
+ /* KASAN */
+ "memset_orig", /* XXX why not memset_erms */
+ "__memset",
+ "kasan_poison_shadow",
+ "kasan_unpoison_shadow",
+ "__asan_poison_stack_memory",
+ "__asan_unpoison_stack_memory",
+ "kasan_report",
+ "check_memory_region",
+ /* KASAN out-of-line */
+ "__asan_loadN_noabort",
+ "__asan_load1_noabort",
+ "__asan_load2_noabort",
+ "__asan_load4_noabort",
+ "__asan_load8_noabort",
+ "__asan_load16_noabort",
+ "__asan_storeN_noabort",
+ "__asan_store1_noabort",
+ "__asan_store2_noabort",
+ "__asan_store4_noabort",
+ "__asan_store8_noabort",
+ "__asan_store16_noabort",
+ /* KASAN in-line */
+ "__asan_report_load_n_noabort",
+ "__asan_report_load1_noabort",
+ "__asan_report_load2_noabort",
+ "__asan_report_load4_noabort",
+ "__asan_report_load8_noabort",
+ "__asan_report_load16_noabort",
+ "__asan_report_store_n_noabort",
+ "__asan_report_store1_noabort",
+ "__asan_report_store2_noabort",
+ "__asan_report_store4_noabort",
+ "__asan_report_store8_noabort",
+ "__asan_report_store16_noabort",
+ /* KCOV */
+ "write_comp_data",
+ "__sanitizer_cov_trace_pc",
+ "__sanitizer_cov_trace_const_cmp1",
+ "__sanitizer_cov_trace_const_cmp2",
+ "__sanitizer_cov_trace_const_cmp4",
+ "__sanitizer_cov_trace_const_cmp8",
+ "__sanitizer_cov_trace_cmp1",
+ "__sanitizer_cov_trace_cmp2",
+ "__sanitizer_cov_trace_cmp4",
+ "__sanitizer_cov_trace_cmp8",
+ /* UBSAN */
+ "ubsan_type_mismatch_common",
+ "__ubsan_handle_type_mismatch",
+ "__ubsan_handle_type_mismatch_v1",
+ /* misc */
+ "csum_partial_copy_generic",
+ "__memcpy_mcsafe",
+ "ftrace_likely_update", /* CONFIG_TRACE_BRANCH_PROFILING */
+ NULL
+};
+
+static void add_uaccess_safe(struct objtool_file *file)
+{
+ struct symbol *func;
+ const char **name;
+
+ if (!uaccess)
+ return;
+
+ for (name = uaccess_safe_builtin; *name; name++) {
+ func = find_symbol_by_name(file->elf, *name);
+ if (!func)
+ continue;
+
+ func->alias->uaccess_safe = true;
+ }
+}
+
/*
* FIXME: For now, just ignore any alternatives which add retpolines. This is
* a temporary hack, as it doesn't allow ORC to unwind from inside a retpoline.
@@ -1239,6 +1314,7 @@ static int decode_sections(struct objtoo
return ret;
add_ignores(file);
+ add_uaccess_safe(file);
ret = add_nospec_ignores(file);
if (ret)
@@ -1799,6 +1875,22 @@ static bool insn_state_match(struct inst
return false;
}
+static inline bool func_uaccess_safe(struct symbol *func)
+{
+ if (func)
+ return func->alias->uaccess_safe;
+
+ return false;
+}
+
+static inline const char *insn_dest_name(struct instruction *insn)
+{
+ if (insn->call_dest)
+ return insn->call_dest->name;
+
+ return "{dynamic}";
+}
+
/*
* Follow the branch starting at the given instruction, and recursively follow
* any other branches (jumps). Meanwhile, track the frame pointer state at
@@ -1844,7 +1936,9 @@ static int validate_branch(struct objtoo
if (!insn->hint && !insn_state_match(insn, &state))
return 1;
- return 0;
+ /* If we were here with AC=0, but now have AC=1, go again */
+ if (insn->state.uaccess || !state.uaccess)
+ return 0;
}
if (insn->hint) {
@@ -1914,6 +2008,16 @@ static int validate_branch(struct objtoo
switch (insn->type) {
case INSN_RETURN:
+ if (state.uaccess && !func_uaccess_safe(func)) {
+ WARN_FUNC("return with UACCESS enabled", sec, insn->offset);
+ return 1;
+ }
+
+ if (!state.uaccess && func_uaccess_safe(func)) {
+ WARN_FUNC("return with UACCESS disabled from a UACCESS-safe function", sec, insn->offset);
+ return 1;
+ }
+
if (func && has_modified_stack_frame(&state)) {
WARN_FUNC("return with modified stack frame",
sec, insn->offset);
@@ -1929,17 +2033,32 @@ static int validate_branch(struct objtoo
return 0;
case INSN_CALL:
- if (is_fentry_call(insn))
- break;
+ case INSN_CALL_DYNAMIC:
+do_call:
+ if (state.uaccess && !func_uaccess_safe(insn->call_dest)) {
+ WARN_FUNC("call to %s() with UACCESS enabled",
+ sec, insn->offset, insn_dest_name(insn));
+ return 1;
+ }
- ret = dead_end_function(file, insn->call_dest);
- if (ret == 1)
+ if (insn->type == INSN_JUMP_UNCONDITIONAL ||
+ insn->type == INSN_JUMP_DYNAMIC)
return 0;
- if (ret == -1)
- return 1;
- /* fallthrough */
- case INSN_CALL_DYNAMIC:
+ if (insn->type == INSN_JUMP_CONDITIONAL)
+ break;
+
+ if (insn->type == INSN_CALL) {
+ if (is_fentry_call(insn))
+ break;
+
+ ret = dead_end_function(file, insn->call_dest);
+ if (ret == 1)
+ return 0;
+ if (ret == -1)
+ return 1;
+ }
+
if (!no_fp && func && !has_valid_stack_frame(&state)) {
WARN_FUNC("call without frame pointer save/setup",
sec, insn->offset);
@@ -1956,6 +2075,8 @@ static int validate_branch(struct objtoo
sec, insn->offset);
return 1;
}
+ goto do_call;
+
} else if (insn->jump_dest &&
(!func || !insn->jump_dest->func ||
insn->jump_dest->func->pfunc == func)) {
@@ -1994,6 +2115,29 @@ static int validate_branch(struct objtoo
break;
+ case INSN_STAC:
+ if (state.uaccess) {
+ WARN_FUNC("recursive UACCESS enable", sec, insn->offset);
+ return 1;
+ }
+
+ state.uaccess = true;
+ break;
+
+ case INSN_CLAC:
+ if (!state.uaccess && insn->func) {
+ WARN_FUNC("redundant UACCESS disable", sec, insn->offset);
+ return 1;
+ }
+
+ if (func_uaccess_safe(func)) {
+ WARN_FUNC("UACCESS-safe disables UACCESS", sec, insn->offset);
+ return 1;
+ }
+
+ state.uaccess = false;
+ break;
+
default:
break;
}
@@ -2157,6 +2301,8 @@ static int validate_functions(struct obj
if (!insn || insn->ignore)
continue;
+ state.uaccess = func->alias->uaccess_safe;
+
ret = validate_branch(file, insn, state);
if (ret && backtrace)
BT_FUNC("<=== (func)", insn);
--- 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, uaccess;
int drap_reg, drap_offset;
struct cfi_reg vals[CFI_NUM_REGS];
};
--- 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, *alias;
+ bool uaccess_safe;
};
struct rela {
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -42,6 +42,7 @@
#define ALT_NEW_LEN_OFFSET 11
#define X86_FEATURE_POPCNT (4*32+23)
+#define X86_FEATURE_SMAP (9*32+20)
struct special_entry {
const char *sec;
@@ -107,8 +108,15 @@ static int get_alt_entry(struct elf *elf
* It has been requested that we don't validate the !POPCNT
* feature path which is a "very very small percentage of
* machines".
+ *
+ * Also, unconditionally enable SMAP; this avoids seeing paths
+ * that pass through the STAC alternative and through the CLAC
+ * NOPs.
+ *
+ * XXX: We could do this for all binary NOP/single-INSN
+ * alternatives.
*/
- if (feature == X86_FEATURE_POPCNT)
+ if (feature == X86_FEATURE_POPCNT || feature == X86_FEATURE_SMAP)
alt->skip_orig = true;
}
Powered by blists - more mailing lists