Skip to content

Commit ea24213

Browse files
Peter ZijlstraIngo Molnar
authored andcommitted
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: Peter Zijlstra (Intel) <[email protected]> Acked-by: Josh Poimboeuf <[email protected]> Cc: Borislav Petkov <[email protected]> Cc: Linus Torvalds <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Thomas Gleixner <[email protected]> Signed-off-by: Ingo Molnar <[email protected]>
1 parent 54262aa commit ea24213

File tree

10 files changed

+226
-21
lines changed

10 files changed

+226
-21
lines changed

scripts/Makefile.build

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,9 @@ endif
222222
ifdef CONFIG_RETPOLINE
223223
objtool_args += --retpoline
224224
endif
225+
ifdef CONFIG_X86_SMAP
226+
objtool_args += --uaccess
227+
endif
225228

226229
# 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
227230
# 'OBJECT_FILES_NON_STANDARD_foo.o := 'y': skip objtool checking for a file

tools/objtool/arch.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,17 @@
3333
#define INSN_STACK 8
3434
#define INSN_BUG 9
3535
#define INSN_NOP 10
36-
#define INSN_OTHER 11
36+
#define INSN_STAC 11
37+
#define INSN_CLAC 12
38+
#define INSN_OTHER 13
3739
#define INSN_LAST INSN_OTHER
3840

3941
enum op_dest_type {
4042
OP_DEST_REG,
4143
OP_DEST_REG_INDIRECT,
4244
OP_DEST_MEM,
4345
OP_DEST_PUSH,
46+
OP_DEST_PUSHF,
4447
OP_DEST_LEAVE,
4548
};
4649

@@ -55,6 +58,7 @@ enum op_src_type {
5558
OP_SRC_REG_INDIRECT,
5659
OP_SRC_CONST,
5760
OP_SRC_POP,
61+
OP_SRC_POPF,
5862
OP_SRC_ADD,
5963
OP_SRC_AND,
6064
};

tools/objtool/arch/x86/decode.c

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -357,19 +357,26 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
357357
/* pushf */
358358
*type = INSN_STACK;
359359
op->src.type = OP_SRC_CONST;
360-
op->dest.type = OP_DEST_PUSH;
360+
op->dest.type = OP_DEST_PUSHF;
361361
break;
362362

363363
case 0x9d:
364364
/* popf */
365365
*type = INSN_STACK;
366-
op->src.type = OP_SRC_POP;
366+
op->src.type = OP_SRC_POPF;
367367
op->dest.type = OP_DEST_MEM;
368368
break;
369369

370370
case 0x0f:
371371

372-
if (op2 >= 0x80 && op2 <= 0x8f) {
372+
if (op2 == 0x01) {
373+
374+
if (modrm == 0xca)
375+
*type = INSN_CLAC;
376+
else if (modrm == 0xcb)
377+
*type = INSN_STAC;
378+
379+
} else if (op2 >= 0x80 && op2 <= 0x8f) {
373380

374381
*type = INSN_JUMP_CONDITIONAL;
375382

tools/objtool/builtin-check.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
#include "builtin.h"
3030
#include "check.h"
3131

32-
bool no_fp, no_unreachable, retpoline, module, backtrace;
32+
bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess;
3333

3434
static const char * const check_usage[] = {
3535
"objtool check [<options>] file.o",
@@ -42,6 +42,7 @@ const struct option check_options[] = {
4242
OPT_BOOLEAN('r', "retpoline", &retpoline, "Validate retpoline assumptions"),
4343
OPT_BOOLEAN('m', "module", &module, "Indicates the object will be part of a kernel module"),
4444
OPT_BOOLEAN('b', "backtrace", &backtrace, "unwind on error"),
45+
OPT_BOOLEAN('a', "uaccess", &uaccess, "enable uaccess checking"),
4546
OPT_END(),
4647
};
4748

tools/objtool/builtin.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
#include <subcmd/parse-options.h>
2121

2222
extern const struct option check_options[];
23-
extern bool no_fp, no_unreachable, retpoline, module, backtrace;
23+
extern bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess;
2424

2525
extern int cmd_check(int argc, const char **argv);
2626
extern int cmd_orc(int argc, const char **argv);

tools/objtool/check.c

Lines changed: 183 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,82 @@ static void add_ignores(struct objtool_file *file)
442442
}
443443
}
444444

445+
/*
446+
* This is a whitelist of functions that is allowed to be called with AC set.
447+
* The list is meant to be minimal and only contains compiler instrumentation
448+
* ABI and a few functions used to implement *_{to,from}_user() functions.
449+
*
450+
* These functions must not directly change AC, but may PUSHF/POPF.
451+
*/
452+
static const char *uaccess_safe_builtin[] = {
453+
/* KASAN */
454+
"kasan_report",
455+
"check_memory_region",
456+
/* KASAN out-of-line */
457+
"__asan_loadN_noabort",
458+
"__asan_load1_noabort",
459+
"__asan_load2_noabort",
460+
"__asan_load4_noabort",
461+
"__asan_load8_noabort",
462+
"__asan_load16_noabort",
463+
"__asan_storeN_noabort",
464+
"__asan_store1_noabort",
465+
"__asan_store2_noabort",
466+
"__asan_store4_noabort",
467+
"__asan_store8_noabort",
468+
"__asan_store16_noabort",
469+
/* KASAN in-line */
470+
"__asan_report_load_n_noabort",
471+
"__asan_report_load1_noabort",
472+
"__asan_report_load2_noabort",
473+
"__asan_report_load4_noabort",
474+
"__asan_report_load8_noabort",
475+
"__asan_report_load16_noabort",
476+
"__asan_report_store_n_noabort",
477+
"__asan_report_store1_noabort",
478+
"__asan_report_store2_noabort",
479+
"__asan_report_store4_noabort",
480+
"__asan_report_store8_noabort",
481+
"__asan_report_store16_noabort",
482+
/* KCOV */
483+
"write_comp_data",
484+
"__sanitizer_cov_trace_pc",
485+
"__sanitizer_cov_trace_const_cmp1",
486+
"__sanitizer_cov_trace_const_cmp2",
487+
"__sanitizer_cov_trace_const_cmp4",
488+
"__sanitizer_cov_trace_const_cmp8",
489+
"__sanitizer_cov_trace_cmp1",
490+
"__sanitizer_cov_trace_cmp2",
491+
"__sanitizer_cov_trace_cmp4",
492+
"__sanitizer_cov_trace_cmp8",
493+
/* UBSAN */
494+
"ubsan_type_mismatch_common",
495+
"__ubsan_handle_type_mismatch",
496+
"__ubsan_handle_type_mismatch_v1",
497+
/* misc */
498+
"csum_partial_copy_generic",
499+
"__memcpy_mcsafe",
500+
"ftrace_likely_update", /* CONFIG_TRACE_BRANCH_PROFILING */
501+
NULL
502+
};
503+
504+
static void add_uaccess_safe(struct objtool_file *file)
505+
{
506+
struct symbol *func;
507+
const char **name;
508+
509+
if (!uaccess)
510+
return;
511+
512+
for (name = uaccess_safe_builtin; *name; name++) {
513+
func = find_symbol_by_name(file->elf, *name);
514+
if (!func)
515+
continue;
516+
517+
func->alias->uaccess_safe = true;
518+
}
519+
}
520+
445521
/*
446522
* FIXME: For now, just ignore any alternatives which add retpolines. This is
447523
* 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)
818894

819895
alt->insn = new_insn;
820896
alt->skip_orig = special_alt->skip_orig;
897+
orig_insn->ignore_alts |= special_alt->skip_alt;
821898
list_add_tail(&alt->list, &orig_insn->alts);
822899

823900
list_del(&special_alt->list);
@@ -1239,6 +1316,7 @@ static int decode_sections(struct objtool_file *file)
12391316
return ret;
12401317

12411318
add_ignores(file);
1319+
add_uaccess_safe(file);
12421320

12431321
ret = add_ignore_alternatives(file);
12441322
if (ret)
@@ -1320,11 +1398,11 @@ static int update_insn_state_regs(struct instruction *insn, struct insn_state *s
13201398
return 0;
13211399

13221400
/* push */
1323-
if (op->dest.type == OP_DEST_PUSH)
1401+
if (op->dest.type == OP_DEST_PUSH || op->dest.type == OP_DEST_PUSHF)
13241402
cfa->offset += 8;
13251403

13261404
/* pop */
1327-
if (op->src.type == OP_SRC_POP)
1405+
if (op->src.type == OP_SRC_POP || op->src.type == OP_SRC_POPF)
13281406
cfa->offset -= 8;
13291407

13301408
/* add immediate to sp */
@@ -1581,6 +1659,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
15811659
break;
15821660

15831661
case OP_SRC_POP:
1662+
case OP_SRC_POPF:
15841663
if (!state->drap && op->dest.type == OP_DEST_REG &&
15851664
op->dest.reg == cfa->base) {
15861665

@@ -1645,6 +1724,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
16451724
break;
16461725

16471726
case OP_DEST_PUSH:
1727+
case OP_DEST_PUSHF:
16481728
state->stack_size += 8;
16491729
if (cfa->base == CFI_SP)
16501730
cfa->offset += 8;
@@ -1735,7 +1815,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
17351815
break;
17361816

17371817
case OP_DEST_MEM:
1738-
if (op->src.type != OP_SRC_POP) {
1818+
if (op->src.type != OP_SRC_POP && op->src.type != OP_SRC_POPF) {
17391819
WARN_FUNC("unknown stack-related memory operation",
17401820
insn->sec, insn->offset);
17411821
return -1;
@@ -1799,6 +1879,33 @@ static bool insn_state_match(struct instruction *insn, struct insn_state *state)
17991879
return false;
18001880
}
18011881

1882+
static inline bool func_uaccess_safe(struct symbol *func)
1883+
{
1884+
if (func)
1885+
return func->alias->uaccess_safe;
1886+
1887+
return false;
1888+
}
1889+
1890+
static inline const char *insn_dest_name(struct instruction *insn)
1891+
{
1892+
if (insn->call_dest)
1893+
return insn->call_dest->name;
1894+
1895+
return "{dynamic}";
1896+
}
1897+
1898+
static int validate_call(struct instruction *insn, struct insn_state *state)
1899+
{
1900+
if (state->uaccess && !func_uaccess_safe(insn->call_dest)) {
1901+
WARN_FUNC("call to %s() with UACCESS enabled",
1902+
insn->sec, insn->offset, insn_dest_name(insn));
1903+
return 1;
1904+
}
1905+
1906+
return 0;
1907+
}
1908+
18021909
static int validate_sibling_call(struct instruction *insn, struct insn_state *state)
18031910
{
18041911
if (has_modified_stack_frame(state)) {
@@ -1807,7 +1914,7 @@ static int validate_sibling_call(struct instruction *insn, struct insn_state *st
18071914
return 1;
18081915
}
18091916

1810-
return 0;
1917+
return validate_call(insn, state);
18111918
}
18121919

18131920
/*
@@ -1855,7 +1962,9 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
18551962
if (!insn->hint && !insn_state_match(insn, &state))
18561963
return 1;
18571964

1858-
return 0;
1965+
/* If we were here with AC=0, but now have AC=1, go again */
1966+
if (insn->state.uaccess || !state.uaccess)
1967+
return 0;
18591968
}
18601969

18611970
if (insn->hint) {
@@ -1925,6 +2034,16 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
19252034
switch (insn->type) {
19262035

19272036
case INSN_RETURN:
2037+
if (state.uaccess && !func_uaccess_safe(func)) {
2038+
WARN_FUNC("return with UACCESS enabled", sec, insn->offset);
2039+
return 1;
2040+
}
2041+
2042+
if (!state.uaccess && func_uaccess_safe(func)) {
2043+
WARN_FUNC("return with UACCESS disabled from a UACCESS-safe function", sec, insn->offset);
2044+
return 1;
2045+
}
2046+
19282047
if (func && has_modified_stack_frame(&state)) {
19292048
WARN_FUNC("return with modified stack frame",
19302049
sec, insn->offset);
@@ -1940,17 +2059,22 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
19402059
return 0;
19412060

19422061
case INSN_CALL:
1943-
if (is_fentry_call(insn))
1944-
break;
2062+
case INSN_CALL_DYNAMIC:
2063+
ret = validate_call(insn, &state);
2064+
if (ret)
2065+
return ret;
19452066

1946-
ret = dead_end_function(file, insn->call_dest);
1947-
if (ret == 1)
1948-
return 0;
1949-
if (ret == -1)
1950-
return 1;
2067+
if (insn->type == INSN_CALL) {
2068+
if (is_fentry_call(insn))
2069+
break;
2070+
2071+
ret = dead_end_function(file, insn->call_dest);
2072+
if (ret == 1)
2073+
return 0;
2074+
if (ret == -1)
2075+
return 1;
2076+
}
19512077

1952-
/* fallthrough */
1953-
case INSN_CALL_DYNAMIC:
19542078
if (!no_fp && func && !has_valid_stack_frame(&state)) {
19552079
WARN_FUNC("call without frame pointer save/setup",
19562080
sec, insn->offset);
@@ -2003,6 +2127,49 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
20032127
if (update_insn_state(insn, &state))
20042128
return 1;
20052129

2130+
if (insn->stack_op.dest.type == OP_DEST_PUSHF) {
2131+
if (!state.uaccess_stack) {
2132+
state.uaccess_stack = 1;
2133+
} else if (state.uaccess_stack >> 31) {
2134+
WARN_FUNC("PUSHF stack exhausted", sec, insn->offset);
2135+
return 1;
2136+
}
2137+
state.uaccess_stack <<= 1;
2138+
state.uaccess_stack |= state.uaccess;
2139+
}
2140+
2141+
if (insn->stack_op.src.type == OP_SRC_POPF) {
2142+
if (state.uaccess_stack) {
2143+
state.uaccess = state.uaccess_stack & 1;
2144+
state.uaccess_stack >>= 1;
2145+
if (state.uaccess_stack == 1)
2146+
state.uaccess_stack = 0;
2147+
}
2148+
}
2149+
2150+
break;
2151+
2152+
case INSN_STAC:
2153+
if (state.uaccess) {
2154+
WARN_FUNC("recursive UACCESS enable", sec, insn->offset);
2155+
return 1;
2156+
}
2157+
2158+
state.uaccess = true;
2159+
break;
2160+
2161+
case INSN_CLAC:
2162+
if (!state.uaccess && insn->func) {
2163+
WARN_FUNC("redundant UACCESS disable", sec, insn->offset);
2164+
return 1;
2165+
}
2166+
2167+
if (func_uaccess_safe(func) && !state.uaccess_stack) {
2168+
WARN_FUNC("UACCESS-safe disables UACCESS", sec, insn->offset);
2169+
return 1;
2170+
}
2171+
2172+
state.uaccess = false;
20062173
break;
20072174

20082175
default:
@@ -2168,6 +2335,8 @@ static int validate_functions(struct objtool_file *file)
21682335
if (!insn || insn->ignore)
21692336
continue;
21702337

2338+
state.uaccess = func->alias->uaccess_safe;
2339+
21712340
ret = validate_branch(file, insn, state);
21722341
if (ret && backtrace)
21732342
BT_FUNC("<=== (func)", insn);

0 commit comments

Comments
 (0)