Commit ea24213d authored by Peter Zijlstra's avatar Peter Zijlstra Committed by Ingo Molnar

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).
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: default avatarJosh Poimboeuf <jpoimboe@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent 54262aa2
...@@ -222,6 +222,9 @@ endif ...@@ -222,6 +222,9 @@ endif
ifdef CONFIG_RETPOLINE ifdef CONFIG_RETPOLINE
objtool_args += --retpoline objtool_args += --retpoline
endif endif
ifdef CONFIG_X86_SMAP
objtool_args += --uaccess
endif
# 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory # 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
# 'OBJECT_FILES_NON_STANDARD_foo.o := 'y': skip objtool checking for a file # 'OBJECT_FILES_NON_STANDARD_foo.o := 'y': skip objtool checking for a file
......
...@@ -33,7 +33,9 @@ ...@@ -33,7 +33,9 @@
#define INSN_STACK 8 #define INSN_STACK 8
#define INSN_BUG 9 #define INSN_BUG 9
#define INSN_NOP 10 #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 #define INSN_LAST INSN_OTHER
enum op_dest_type { enum op_dest_type {
...@@ -41,6 +43,7 @@ enum op_dest_type { ...@@ -41,6 +43,7 @@ enum op_dest_type {
OP_DEST_REG_INDIRECT, OP_DEST_REG_INDIRECT,
OP_DEST_MEM, OP_DEST_MEM,
OP_DEST_PUSH, OP_DEST_PUSH,
OP_DEST_PUSHF,
OP_DEST_LEAVE, OP_DEST_LEAVE,
}; };
...@@ -55,6 +58,7 @@ enum op_src_type { ...@@ -55,6 +58,7 @@ enum op_src_type {
OP_SRC_REG_INDIRECT, OP_SRC_REG_INDIRECT,
OP_SRC_CONST, OP_SRC_CONST,
OP_SRC_POP, OP_SRC_POP,
OP_SRC_POPF,
OP_SRC_ADD, OP_SRC_ADD,
OP_SRC_AND, OP_SRC_AND,
}; };
......
...@@ -357,19 +357,26 @@ int arch_decode_instruction(struct elf *elf, struct section *sec, ...@@ -357,19 +357,26 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
/* pushf */ /* pushf */
*type = INSN_STACK; *type = INSN_STACK;
op->src.type = OP_SRC_CONST; op->src.type = OP_SRC_CONST;
op->dest.type = OP_DEST_PUSH; op->dest.type = OP_DEST_PUSHF;
break; break;
case 0x9d: case 0x9d:
/* popf */ /* popf */
*type = INSN_STACK; *type = INSN_STACK;
op->src.type = OP_SRC_POP; op->src.type = OP_SRC_POPF;
op->dest.type = OP_DEST_MEM; op->dest.type = OP_DEST_MEM;
break; break;
case 0x0f: 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; *type = INSN_JUMP_CONDITIONAL;
......
...@@ -29,7 +29,7 @@ ...@@ -29,7 +29,7 @@
#include "builtin.h" #include "builtin.h"
#include "check.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[] = { static const char * const check_usage[] = {
"objtool check [<options>] file.o", "objtool check [<options>] file.o",
...@@ -42,6 +42,7 @@ const struct option check_options[] = { ...@@ -42,6 +42,7 @@ const struct option check_options[] = {
OPT_BOOLEAN('r', "retpoline", &retpoline, "Validate retpoline assumptions"), 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('m', "module", &module, "Indicates the object will be part of a kernel module"),
OPT_BOOLEAN('b', "backtrace", &backtrace, "unwind on error"), OPT_BOOLEAN('b', "backtrace", &backtrace, "unwind on error"),
OPT_BOOLEAN('a', "uaccess", &uaccess, "enable uaccess checking"),
OPT_END(), OPT_END(),
}; };
......
...@@ -20,7 +20,7 @@ ...@@ -20,7 +20,7 @@
#include <subcmd/parse-options.h> #include <subcmd/parse-options.h>
extern const struct option check_options[]; 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_check(int argc, const char **argv);
extern int cmd_orc(int argc, const char **argv); extern int cmd_orc(int argc, const char **argv);
......
...@@ -442,6 +442,82 @@ static void add_ignores(struct objtool_file *file) ...@@ -442,6 +442,82 @@ static void add_ignores(struct objtool_file *file)
} }
} }
/*
* This is a whitelist of functions that is allowed to be called with AC set.
* The list is meant to be minimal and only contains compiler instrumentation
* ABI and a few functions used to implement *_{to,from}_user() functions.
*
* These functions must not directly change AC, but may PUSHF/POPF.
*/
static const char *uaccess_safe_builtin[] = {
/* KASAN */
"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 * 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. * a temporary hack, as it doesn't allow ORC to unwind from inside a retpoline.
...@@ -818,6 +894,7 @@ static int add_special_section_alts(struct objtool_file *file) ...@@ -818,6 +894,7 @@ static int add_special_section_alts(struct objtool_file *file)
alt->insn = new_insn; alt->insn = new_insn;
alt->skip_orig = special_alt->skip_orig; alt->skip_orig = special_alt->skip_orig;
orig_insn->ignore_alts |= special_alt->skip_alt;
list_add_tail(&alt->list, &orig_insn->alts); list_add_tail(&alt->list, &orig_insn->alts);
list_del(&special_alt->list); list_del(&special_alt->list);
...@@ -1239,6 +1316,7 @@ static int decode_sections(struct objtool_file *file) ...@@ -1239,6 +1316,7 @@ static int decode_sections(struct objtool_file *file)
return ret; return ret;
add_ignores(file); add_ignores(file);
add_uaccess_safe(file);
ret = add_ignore_alternatives(file); ret = add_ignore_alternatives(file);
if (ret) if (ret)
...@@ -1320,11 +1398,11 @@ static int update_insn_state_regs(struct instruction *insn, struct insn_state *s ...@@ -1320,11 +1398,11 @@ static int update_insn_state_regs(struct instruction *insn, struct insn_state *s
return 0; return 0;
/* push */ /* push */
if (op->dest.type == OP_DEST_PUSH) if (op->dest.type == OP_DEST_PUSH || op->dest.type == OP_DEST_PUSHF)
cfa->offset += 8; cfa->offset += 8;
/* pop */ /* pop */
if (op->src.type == OP_SRC_POP) if (op->src.type == OP_SRC_POP || op->src.type == OP_SRC_POPF)
cfa->offset -= 8; cfa->offset -= 8;
/* add immediate to sp */ /* add immediate to sp */
...@@ -1581,6 +1659,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state) ...@@ -1581,6 +1659,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
break; break;
case OP_SRC_POP: case OP_SRC_POP:
case OP_SRC_POPF:
if (!state->drap && op->dest.type == OP_DEST_REG && if (!state->drap && op->dest.type == OP_DEST_REG &&
op->dest.reg == cfa->base) { op->dest.reg == cfa->base) {
...@@ -1645,6 +1724,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state) ...@@ -1645,6 +1724,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
break; break;
case OP_DEST_PUSH: case OP_DEST_PUSH:
case OP_DEST_PUSHF:
state->stack_size += 8; state->stack_size += 8;
if (cfa->base == CFI_SP) if (cfa->base == CFI_SP)
cfa->offset += 8; cfa->offset += 8;
...@@ -1735,7 +1815,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state) ...@@ -1735,7 +1815,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
break; break;
case OP_DEST_MEM: case OP_DEST_MEM:
if (op->src.type != OP_SRC_POP) { if (op->src.type != OP_SRC_POP && op->src.type != OP_SRC_POPF) {
WARN_FUNC("unknown stack-related memory operation", WARN_FUNC("unknown stack-related memory operation",
insn->sec, insn->offset); insn->sec, insn->offset);
return -1; return -1;
...@@ -1799,6 +1879,33 @@ static bool insn_state_match(struct instruction *insn, struct insn_state *state) ...@@ -1799,6 +1879,33 @@ static bool insn_state_match(struct instruction *insn, struct insn_state *state)
return false; 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}";
}
static int validate_call(struct instruction *insn, struct insn_state *state)
{
if (state->uaccess && !func_uaccess_safe(insn->call_dest)) {
WARN_FUNC("call to %s() with UACCESS enabled",
insn->sec, insn->offset, insn_dest_name(insn));
return 1;
}
return 0;
}
static int validate_sibling_call(struct instruction *insn, struct insn_state *state) static int validate_sibling_call(struct instruction *insn, struct insn_state *state)
{ {
if (has_modified_stack_frame(state)) { if (has_modified_stack_frame(state)) {
...@@ -1807,7 +1914,7 @@ static int validate_sibling_call(struct instruction *insn, struct insn_state *st ...@@ -1807,7 +1914,7 @@ static int validate_sibling_call(struct instruction *insn, struct insn_state *st
return 1; return 1;
} }
return 0; return validate_call(insn, state);
} }
/* /*
...@@ -1855,7 +1962,9 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, ...@@ -1855,7 +1962,9 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
if (!insn->hint && !insn_state_match(insn, &state)) if (!insn->hint && !insn_state_match(insn, &state))
return 1; 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) { if (insn->hint) {
...@@ -1925,6 +2034,16 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, ...@@ -1925,6 +2034,16 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
switch (insn->type) { switch (insn->type) {
case INSN_RETURN: 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)) { if (func && has_modified_stack_frame(&state)) {
WARN_FUNC("return with modified stack frame", WARN_FUNC("return with modified stack frame",
sec, insn->offset); sec, insn->offset);
...@@ -1940,17 +2059,22 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, ...@@ -1940,17 +2059,22 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
return 0; return 0;
case INSN_CALL: case INSN_CALL:
if (is_fentry_call(insn)) case INSN_CALL_DYNAMIC:
break; ret = validate_call(insn, &state);
if (ret)
return ret;
ret = dead_end_function(file, insn->call_dest); if (insn->type == INSN_CALL) {
if (ret == 1) if (is_fentry_call(insn))
return 0; break;
if (ret == -1)
return 1; ret = dead_end_function(file, insn->call_dest);
if (ret == 1)
return 0;
if (ret == -1)
return 1;
}
/* fallthrough */
case INSN_CALL_DYNAMIC:
if (!no_fp && func && !has_valid_stack_frame(&state)) { if (!no_fp && func && !has_valid_stack_frame(&state)) {
WARN_FUNC("call without frame pointer save/setup", WARN_FUNC("call without frame pointer save/setup",
sec, insn->offset); sec, insn->offset);
...@@ -2003,6 +2127,49 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, ...@@ -2003,6 +2127,49 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
if (update_insn_state(insn, &state)) if (update_insn_state(insn, &state))
return 1; return 1;
if (insn->stack_op.dest.type == OP_DEST_PUSHF) {
if (!state.uaccess_stack) {
state.uaccess_stack = 1;
} else if (state.uaccess_stack >> 31) {
WARN_FUNC("PUSHF stack exhausted", sec, insn->offset);
return 1;
}
state.uaccess_stack <<= 1;
state.uaccess_stack |= state.uaccess;
}
if (insn->stack_op.src.type == OP_SRC_POPF) {
if (state.uaccess_stack) {
state.uaccess = state.uaccess_stack & 1;
state.uaccess_stack >>= 1;
if (state.uaccess_stack == 1)
state.uaccess_stack = 0;
}
}
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) && !state.uaccess_stack) {
WARN_FUNC("UACCESS-safe disables UACCESS", sec, insn->offset);
return 1;
}
state.uaccess = false;
break; break;
default: default:
...@@ -2168,6 +2335,8 @@ static int validate_functions(struct objtool_file *file) ...@@ -2168,6 +2335,8 @@ static int validate_functions(struct objtool_file *file)
if (!insn || insn->ignore) if (!insn || insn->ignore)
continue; continue;
state.uaccess = func->alias->uaccess_safe;
ret = validate_branch(file, insn, state); ret = validate_branch(file, insn, state);
if (ret && backtrace) if (ret && backtrace)
BT_FUNC("<=== (func)", insn); BT_FUNC("<=== (func)", insn);
......
...@@ -31,7 +31,8 @@ struct insn_state { ...@@ -31,7 +31,8 @@ struct insn_state {
int stack_size; int stack_size;
unsigned char type; unsigned char type;
bool bp_scratch; bool bp_scratch;
bool drap, end; bool drap, end, uaccess;
unsigned int uaccess_stack;
int drap_reg, drap_offset; int drap_reg, drap_offset;
struct cfi_reg vals[CFI_NUM_REGS]; struct cfi_reg vals[CFI_NUM_REGS];
}; };
......
...@@ -62,6 +62,7 @@ struct symbol { ...@@ -62,6 +62,7 @@ struct symbol {
unsigned long offset; unsigned long offset;
unsigned int len; unsigned int len;
struct symbol *pfunc, *cfunc, *alias; struct symbol *pfunc, *cfunc, *alias;
bool uaccess_safe;
}; };
struct rela { struct rela {
......
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#include "builtin.h"
#include "special.h" #include "special.h"
#include "warn.h" #include "warn.h"
...@@ -42,6 +43,7 @@ ...@@ -42,6 +43,7 @@
#define ALT_NEW_LEN_OFFSET 11 #define ALT_NEW_LEN_OFFSET 11
#define X86_FEATURE_POPCNT (4*32+23) #define X86_FEATURE_POPCNT (4*32+23)
#define X86_FEATURE_SMAP (9*32+20)
struct special_entry { struct special_entry {
const char *sec; const char *sec;
...@@ -110,6 +112,22 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry, ...@@ -110,6 +112,22 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
*/ */
if (feature == X86_FEATURE_POPCNT) if (feature == X86_FEATURE_POPCNT)
alt->skip_orig = true; alt->skip_orig = true;
/*
* If UACCESS validation is enabled; force that alternative;
* otherwise force it the other way.
*
* What we want to avoid is having both the original and the
* alternative code flow at the same time, in that case we can
* find paths that see the STAC but take the NOP instead of
* CLAC and the other way around.
*/
if (feature == X86_FEATURE_SMAP) {
if (uaccess)
alt->skip_orig = true;
else
alt->skip_alt = true;
}
} }
orig_rela = find_rela_by_dest(sec, offset + entry->orig); orig_rela = find_rela_by_dest(sec, offset + entry->orig);
......
...@@ -26,6 +26,7 @@ struct special_alt { ...@@ -26,6 +26,7 @@ struct special_alt {
bool group; bool group;
bool skip_orig; bool skip_orig;
bool skip_alt;
bool jump_or_nop; bool jump_or_nop;
struct section *orig_sec; struct section *orig_sec;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment