]> git.proxmox.com Git - mirror_ubuntu-eoan-kernel.git/commitdiff
objtool: Add UACCESS validation
authorPeter Zijlstra <peterz@infradead.org>
Mon, 25 Feb 2019 11:50:09 +0000 (12:50 +0100)
committerIngo Molnar <mingo@kernel.org>
Wed, 3 Apr 2019 09:02:24 +0000 (11:02 +0200)
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) <peterz@infradead.org>
Acked-by: Josh 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: Ingo Molnar <mingo@kernel.org>
scripts/Makefile.build
tools/objtool/arch.h
tools/objtool/arch/x86/decode.c
tools/objtool/builtin-check.c
tools/objtool/builtin.h
tools/objtool/check.c
tools/objtool/check.h
tools/objtool/elf.h
tools/objtool/special.c
tools/objtool/special.h

index 76ca30cc4791912fde4d7f36e4a90549e653cfbb..0c5969fa795f8423bfe6bde5a0444f27b2ff9538 100644 (file)
@@ -222,6 +222,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
index b0d7dc3d71b5ac21e8acb36b1c1f38bcf06879c4..467c2fe798a9ea5c0e1666cf3d74ae17eafabc8a 100644 (file)
@@ -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 {
@@ -41,6 +43,7 @@ enum op_dest_type {
        OP_DEST_REG_INDIRECT,
        OP_DEST_MEM,
        OP_DEST_PUSH,
+       OP_DEST_PUSHF,
        OP_DEST_LEAVE,
 };
 
@@ -55,6 +58,7 @@ enum op_src_type {
        OP_SRC_REG_INDIRECT,
        OP_SRC_CONST,
        OP_SRC_POP,
+       OP_SRC_POPF,
        OP_SRC_ADD,
        OP_SRC_AND,
 };
index 540a209b78ab3cd6ae3b972c57b338dc0aa9b58d..ab20a96fee50d01aaf6252892de65d1c44deefe6 100644 (file)
@@ -357,19 +357,26 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
                /* pushf */
                *type = INSN_STACK;
                op->src.type = OP_SRC_CONST;
-               op->dest.type = OP_DEST_PUSH;
+               op->dest.type = OP_DEST_PUSHF;
                break;
 
        case 0x9d:
                /* popf */
                *type = INSN_STACK;
-               op->src.type = OP_SRC_POP;
+               op->src.type = OP_SRC_POPF;
                op->dest.type = OP_DEST_MEM;
                break;
 
        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;
 
index 99f10c585cbeeb6586607eac2e8b69cc1f445434..f3b378126011f5eeb8b77e330e6c2a0ae2624a50 100644 (file)
@@ -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(),
 };
 
index 65fd3cc3c98b541d10054b9b6b4ed38d1f184d1e..69762f9c5602cf43bc9aac083d89f52d361c3013 100644 (file)
@@ -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);
index 8118361295ddd322ed65f3aa3d644de752c15a2e..965e954e07f43c7c293ad5e7b54dea69f0d28148 100644 (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
  * 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)
 
                alt->insn = new_insn;
                alt->skip_orig = special_alt->skip_orig;
+               orig_insn->ignore_alts |= special_alt->skip_alt;
                list_add_tail(&alt->list, &orig_insn->alts);
 
                list_del(&special_alt->list);
@@ -1239,6 +1316,7 @@ static int decode_sections(struct objtool_file *file)
                return ret;
 
        add_ignores(file);
+       add_uaccess_safe(file);
 
        ret = add_ignore_alternatives(file);
        if (ret)
@@ -1320,11 +1398,11 @@ static int update_insn_state_regs(struct instruction *insn, struct insn_state *s
                return 0;
 
        /* push */
-       if (op->dest.type == OP_DEST_PUSH)
+       if (op->dest.type == OP_DEST_PUSH || op->dest.type == OP_DEST_PUSHF)
                cfa->offset += 8;
 
        /* pop */
-       if (op->src.type == OP_SRC_POP)
+       if (op->src.type == OP_SRC_POP || op->src.type == OP_SRC_POPF)
                cfa->offset -= 8;
 
        /* add immediate to sp */
@@ -1581,6 +1659,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
                        break;
 
                case OP_SRC_POP:
+               case OP_SRC_POPF:
                        if (!state->drap && op->dest.type == OP_DEST_REG &&
                            op->dest.reg == cfa->base) {
 
@@ -1645,6 +1724,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
                break;
 
        case OP_DEST_PUSH:
+       case OP_DEST_PUSHF:
                state->stack_size += 8;
                if (cfa->base == CFI_SP)
                        cfa->offset += 8;
@@ -1735,7 +1815,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
                break;
 
        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",
                                  insn->sec, insn->offset);
                        return -1;
@@ -1799,6 +1879,33 @@ static bool insn_state_match(struct instruction *insn, struct insn_state *state)
        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)
 {
        if (has_modified_stack_frame(state)) {
@@ -1807,7 +1914,7 @@ static int validate_sibling_call(struct instruction *insn, struct insn_state *st
                return 1;
        }
 
-       return 0;
+       return validate_call(insn, state);
 }
 
 /*
@@ -1855,7 +1962,9 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
                        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) {
@@ -1925,6 +2034,16 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
                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);
@@ -1940,17 +2059,22 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
                        return 0;
 
                case INSN_CALL:
-                       if (is_fentry_call(insn))
-                               break;
+               case INSN_CALL_DYNAMIC:
+                       ret = validate_call(insn, &state);
+                       if (ret)
+                               return ret;
 
-                       ret = dead_end_function(file, insn->call_dest);
-                       if (ret == 1)
-                               return 0;
-                       if (ret == -1)
-                               return 1;
+                       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;
+                       }
 
-                       /* fallthrough */
-               case INSN_CALL_DYNAMIC:
                        if (!no_fp && func && !has_valid_stack_frame(&state)) {
                                WARN_FUNC("call without frame pointer save/setup",
                                          sec, insn->offset);
@@ -2003,6 +2127,49 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
                        if (update_insn_state(insn, &state))
                                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;
 
                default:
@@ -2168,6 +2335,8 @@ static int validate_functions(struct objtool_file *file)
                        if (!insn || insn->ignore)
                                continue;
 
+                       state.uaccess = func->alias->uaccess_safe;
+
                        ret = validate_branch(file, insn, state);
                        if (ret && backtrace)
                                BT_FUNC("<=== (func)", insn);
index d8896eb43521284135c77073733beb3f7d58b37d..78a95d06c165eee1b985cb165742075389de4316 100644 (file)
@@ -31,7 +31,8 @@ struct insn_state {
        int stack_size;
        unsigned char type;
        bool bp_scratch;
-       bool drap, end;
+       bool drap, end, uaccess;
+       unsigned int uaccess_stack;
        int drap_reg, drap_offset;
        struct cfi_reg vals[CFI_NUM_REGS];
 };
index 968265b4b4cd8c4d4f2fddb6466221748d3fa65d..2cc2ed49322d10c97ae450f2e373865b04647d92 100644 (file)
@@ -62,6 +62,7 @@ struct symbol {
        unsigned long offset;
        unsigned int len;
        struct symbol *pfunc, *cfunc, *alias;
+       bool uaccess_safe;
 };
 
 struct rela {
index 50af4e1274b39d20758208a4944453c46b3aa448..4e50563d87c6466aca09ec1ec756503d97e1921a 100644 (file)
@@ -23,6 +23,7 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "builtin.h"
 #include "special.h"
 #include "warn.h"
 
@@ -42,6 +43,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;
@@ -110,6 +112,22 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
                 */
                if (feature == X86_FEATURE_POPCNT)
                        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);
index fad1d092f679e30129983071f058cfcde4234e1f..d5c062e718eff7e7f77008b0c2e75171ee91fff5 100644 (file)
@@ -26,6 +26,7 @@ struct special_alt {
 
        bool group;
        bool skip_orig;
+       bool skip_alt;
        bool jump_or_nop;
 
        struct section *orig_sec;