linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: torvalds@linux-foundation.org, tglx@linutronix.de, hpa@zytor.com,
	julien.thierry@arm.com, will.deacon@arm.com, luto@amacapital.net,
	mingo@kernel.org, catalin.marinas@arm.com, james.morse@arm.com,
	valentin.schneider@arm.com, brgerst@gmail.com,
	jpoimboe@redhat.com, luto@kernel.org, bp@alien8.de,
	dvlasenk@redhat.com
Cc: linux-kernel@vger.kernel.org, peterz@infradead.org
Subject: [PATCH 5/6] objtool: Add UACCESS validation
Date: Mon, 25 Feb 2019 13:43:35 +0100	[thread overview]
Message-ID: <20190225125232.191698923@infradead.org> (raw)
In-Reply-To: 20190225124330.613028745@infradead.org

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__()
tracing calls 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 dedundant UACCESS disable instruction;
therefore ignore this warning for !STT_FUNC code (exception handlers
are not normal functions).

It also provides a UACCESS_SAFE() annotation which allows explicit
annotation. This is meant to be used for future things like:
unsafe_copy_{to,from}_user().

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/frame.h           |   30 +++++++++++-
 tools/objtool/arch.h            |    4 +
 tools/objtool/arch/x86/decode.c |   14 +++++
 tools/objtool/check.c           |  100 ++++++++++++++++++++++++++++++++++++----
 tools/objtool/check.h           |    2 
 tools/objtool/elf.h             |    1 
 6 files changed, 138 insertions(+), 13 deletions(-)

--- a/include/linux/frame.h
+++ b/include/linux/frame.h
@@ -28,10 +28,38 @@ asm (".pushsection .discard.nonstd_frame
      ".byte 0\n\t"
      ".popsection\n\t");
 
+/*
+ * This macro marks functions as UACCESS-safe, that is, it is safe to call from an
+ * UACCESS enabled region (typically user_access_begin() /
+ * user_access_end()).
+ *
+ * These functions in turn will only call UACCESS-safe functions themselves (which
+ * precludes tracing, including __fentry__ and scheduling, including
+ * preempt_enable).
+ *
+ * UACCESS-safe functions will obviously also not change UACCESS themselves.
+ */
+#define UACCESS_SAFE(func)						\
+	asm (".pushsection .discard.uaccess_safe_strtab, \"S\", @3\n\t"	\
+	     "999: .ascii \"" #func "\"\n\t"				\
+	     "     .byte 0\n\t"						\
+	     ".popsection\n\t"						\
+	     ".pushsection .discard.uaccess_safe\n\t"			\
+	     ".long 999b - .\n\t"					\
+	     ".popsection")
+
+/*
+ * SHT_STRTAB(@3) sections should start with a \0 byte such that the 0 offset
+ * encodes the NULL string.
+ */
+asm(".pushsection .discard.uaccess_safe_strtab, \"S\", @3\n\t"
+    ".byte 0\n\t"
+    ".popsection\n\t");
+
 #else /* !CONFIG_STACK_VALIDATION */
 
 #define STACK_FRAME_NON_STANDARD(func)
+#define AC_SAFE(func)
 
 #endif /* CONFIG_STACK_VALIDATION */
-
 #endif /* _LINUX_FRAME_H */
--- 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/check.c
+++ b/tools/objtool/check.c
@@ -444,6 +444,40 @@ static void add_ignores(struct objtool_f
 	return;
 }
 
+static void add_uaccess_safe(struct objtool_file *file)
+{
+	struct section *strtab_sec, *sec;
+	struct symbol *func;
+	struct rela *rela;
+	const char *name;
+
+	sec = find_section_by_name(file->elf, ".rela.discard.uaccess_safe");
+	if (!sec)
+		return;
+
+	strtab_sec = find_section_by_name(file->elf, ".discard.uaccess_safe_strtab");
+	if (!strtab_sec) {
+		WARN("missing uaccess_safe strtab");
+		return;
+	}
+
+	list_for_each_entry(rela, &sec->rela_list, list) {
+		if (rela->sym->type != STT_SECTION) {
+			WARN("unexpected relocation symbol type in %s", sec->name);
+			return;
+		}
+
+		name = elf_strptr(file->elf->elf, strtab_sec->idx, rela->addend);
+		func = find_symbol_by_name(file->elf, name);
+		if (!func)
+			continue;
+
+		func->uaccess_safe = true;
+	}
+
+	return;
+}
+
 /*
  * 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.
@@ -1232,6 +1266,7 @@ static int decode_sections(struct objtoo
 		return ret;
 
 	add_ignores(file);
+	add_uaccess_safe(file);
 
 	ret = add_nospec_ignores(file);
 	if (ret)
@@ -1792,6 +1827,22 @@ static bool insn_state_match(struct inst
 	return false;
 }
 
+static inline bool insn_dest_uaccess_safe(struct instruction *insn)
+{
+	if (!insn->call_dest)
+		return false;
+
+	return insn->call_dest->uaccess_safe;
+}
+
+static inline const char *insn_dest_name(struct instruction *insn)
+{
+	if (!insn->call_dest)
+		return "{dynamic}";
+
+	return insn->call_dest->name;
+}
+
 /*
  * Follow the branch starting at the given instruction, and recursively follow
  * any other branches (jumps).  Meanwhile, track the frame pointer state at
@@ -1896,6 +1947,9 @@ static int validate_branch(struct objtoo
 		switch (insn->type) {
 
 		case INSN_RETURN:
+			if (state.uaccess)
+				WARN_FUNC("return with UACCESS enabled", sec, insn->offset);
+
 			if (func && has_modified_stack_frame(&state)) {
 				WARN_FUNC("return with modified stack frame",
 					  sec, insn->offset);
@@ -1911,17 +1965,24 @@ static int validate_branch(struct objtoo
 			return 0;
 
 		case INSN_CALL:
-			if (is_fentry_call(insn))
-				break;
+		case INSN_CALL_DYNAMIC:
+			if ((state.uaccess_safe || state.uaccess) &&
+			    !insn_dest_uaccess_safe(insn)) {
+				WARN_FUNC("call to %s() with UACCESS enabled",
+					  sec, insn->offset, insn_dest_name(insn));
+			}
 
-			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);
@@ -1974,6 +2035,25 @@ static int validate_branch(struct objtoo
 
 			break;
 
+		case INSN_STAC:
+			if (state.uaccess_safe || state.uaccess)
+				WARN_FUNC("recursive UACCESS enable", sec, insn->offset);
+
+			state.uaccess = true;
+			break;
+
+		case INSN_CLAC:
+			if (!state.uaccess && insn->func)
+				WARN_FUNC("redundant UACCESS disable", sec, insn->offset);
+
+			if (state.uaccess_safe) {
+				WARN_FUNC("UACCESS-safe disables UACCESS", sec, insn->offset);
+				break;
+			}
+
+			state.uaccess = false;
+			break;
+
 		default:
 			break;
 		}
@@ -2135,6 +2215,8 @@ static int validate_functions(struct obj
 			if (!insn || insn->ignore)
 				continue;
 
+			state.uaccess_safe = func->uaccess_safe;
+
 			ret = validate_branch(file, insn, state);
 			warnings += ret;
 		}
--- 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, uaccess_safe;
 	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;
+	bool uaccess_safe;
 };
 
 struct rela {



  parent reply	other threads:[~2019-02-25 12:53 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-25 12:43 [PATCH 0/6] objtool: UACCESS validation Peter Zijlstra
2019-02-25 12:43 ` [PATCH 1/6] x86/uaccess: Dont evaluate argument inside AC region Peter Zijlstra
2019-02-25 15:43   ` Andy Lutomirski
2019-02-25 16:02     ` Peter Zijlstra
2019-02-25 16:36     ` Borislav Petkov
2019-02-25 16:50       ` Andy Lutomirski
2019-02-25 19:09     ` Linus Torvalds
2019-02-25 19:18       ` Borislav Petkov
2019-02-25 18:10   ` [tip:x86/urgent] x86/uaccess: Don't leak the AC flag into __put_user() value evaluation tip-bot for Andy Lutomirski
2019-02-25 19:46   ` tip-bot for Andy Lutomirski
2019-02-25 12:43 ` [PATCH 2/6] x86/ia32: Fix ia32_restore_sigcontext AC leak Peter Zijlstra
2019-02-25 15:41   ` Andy Lutomirski
2019-02-25 16:10     ` Peter Zijlstra
2019-02-25 16:29       ` Andy Lutomirski
2019-02-25 16:37         ` Peter Zijlstra
2019-02-25 16:41           ` Peter Zijlstra
2019-02-25 16:49           ` Andy Lutomirski
2019-02-25 12:43 ` [PATCH 3/6] objtool: Set insn->func for alternatives Peter Zijlstra
2019-02-25 12:43 ` [PATCH 4/6] objtool: Replace STACK_FRAME_NON_STANDARD annotation Peter Zijlstra
2019-02-25 16:11   ` Josh Poimboeuf
2019-02-25 16:17     ` Peter Zijlstra
2019-02-25 16:23       ` Josh Poimboeuf
2019-02-27 12:20     ` Peter Zijlstra
2019-02-28  0:30       ` Andy Lutomirski
2019-02-25 12:43 ` Peter Zijlstra [this message]
2019-02-25 15:53   ` [PATCH 5/6] objtool: Add UACCESS validation Andy Lutomirski
2019-02-25 16:12     ` Peter Zijlstra
2019-02-25 17:15       ` Peter Zijlstra
2019-02-25 17:34         ` Linus Torvalds
2019-02-25 17:38         ` Josh Poimboeuf
2019-02-27 14:08   ` Peter Zijlstra
2019-02-27 14:17     ` Andrey Ryabinin
2019-02-27 14:26       ` Peter Zijlstra
2019-02-27 14:33         ` Peter Zijlstra
2019-02-27 15:40           ` Dmitry Vyukov
2019-02-27 17:28             ` Peter Zijlstra
2019-02-28  9:40               ` Peter Zijlstra
2019-02-28  9:59                 ` Dmitry Vyukov
2019-02-28 10:05                   ` Dmitry Vyukov
2019-02-28 10:52                     ` Peter Zijlstra
2019-02-27 16:18     ` Linus Torvalds
2019-02-27 17:30       ` Peter Zijlstra
2019-02-27 17:36         ` Linus Torvalds
2019-02-25 12:43 ` [PATCH 6/6] objtool: Add Direction Flag validation Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190225125232.191698923@infradead.org \
    --to=peterz@infradead.org \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=catalin.marinas@arm.com \
    --cc=dvlasenk@redhat.com \
    --cc=hpa@zytor.com \
    --cc=james.morse@arm.com \
    --cc=jpoimboe@redhat.com \
    --cc=julien.thierry@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=valentin.schneider@arm.com \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).