[PATCH 0/7] objtool: retpoline validation

[PATCH 0/7] objtool: retpoline validation

[Peter Zijlstra]

There are the retpoline validation patches; they work with the __noretpoline
thing from David.

[PATCH 1/7] objtool: Use existing global variables for options

[Peter Zijlstra]

[-- Attachment #1: peterz-objtool-unclutter.patch --]
[-- Type: text/plain, Size: 2802 bytes --]

Use the existing global variables instead of passing them around and
creating duplicate global variables.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/builtin-check.c |    2 +-
 tools/objtool/builtin-orc.c   |    6 +-----
 tools/objtool/builtin.h       |    5 +++++
 tools/objtool/check.c         |    5 ++---
 tools/objtool/check.h         |    2 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -53,5 +53,5 @@ int cmd_check(int argc, const char **arg
 
 	objname = argv[0];
 
-	return check(objname, no_fp, no_unreachable, false);
+	return check(objname, false);
 }
--- a/tools/objtool/builtin-orc.c
+++ b/tools/objtool/builtin-orc.c
@@ -25,7 +25,6 @@
  */
 
 #include <string.h>
-#include <subcmd/parse-options.h>
 #include "builtin.h"
 #include "check.h"
 
@@ -36,9 +35,6 @@ static const char *orc_usage[] = {
 	NULL,
 };
 
-extern const struct option check_options[];
-extern bool no_fp, no_unreachable;
-
 int cmd_orc(int argc, const char **argv)
 {
 	const char *objname;
@@ -54,7 +50,7 @@ int cmd_orc(int argc, const char **argv)
 
 		objname = argv[0];
 
-		return check(objname, no_fp, no_unreachable, true);
+		return check(objname, true);
 	}
 
 	if (!strcmp(argv[0], "dump")) {
--- a/tools/objtool/builtin.h
+++ b/tools/objtool/builtin.h
@@ -17,6 +17,11 @@
 #ifndef _BUILTIN_H
 #define _BUILTIN_H
 
+#include <subcmd/parse-options.h>
+
+extern const struct option check_options[];
+extern bool no_fp, no_unreachable;
+
 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
@@ -18,6 +18,7 @@
 #include <string.h>
 #include <stdlib.h>
 
+#include "builtin.h"
 #include "check.h"
 #include "elf.h"
 #include "special.h"
@@ -33,7 +34,6 @@ struct alternative {
 };
 
 const char *objname;
-static bool no_fp;
 struct cfi_state initial_func_cfi;
 
 struct instruction *find_insn(struct objtool_file *file,
@@ -1978,13 +1978,12 @@ static void cleanup(struct objtool_file
 	elf_close(file->elf);
 }
 
-int check(const char *_objname, bool _no_fp, bool no_unreachable, bool orc)
+int check(const char *_objname, bool orc)
 {
 	struct objtool_file file;
 	int ret, warnings = 0;
 
 	objname = _objname;
-	no_fp = _no_fp;
 
 	file.elf = elf_open(objname, orc ? O_RDWR : O_RDONLY);
 	if (!file.elf)
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -62,7 +62,7 @@ struct objtool_file {
 	bool ignore_unreachables, c_file, hints;
 };
 
-int check(const char *objname, bool no_fp, bool no_unreachable, bool orc);
+int check(const char *objname, bool orc);
 
 struct instruction *find_insn(struct objtool_file *file,
 			      struct section *sec, unsigned long offset);

[PATCH 2/7] objtool: Add retpoline validation

[Peter Zijlstra]

[-- Attachment #1: peterz-objtool-indirect.patch --]
[-- Type: text/plain, Size: 5537 bytes --]

David requested a objtool validation pass for RETPOLINE enabled
builds, where it validates no unannotated indirect  jumps or calls are
left.

Add an additional .discard.retpoline_safe section to allow annotating
the few indirect sites that are required and safe.

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Requested-by: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 scripts/Makefile.build        |    4 +
 tools/objtool/builtin-check.c |    3 -
 tools/objtool/builtin.h       |    2 
 tools/objtool/check.c         |   86 +++++++++++++++++++++++++++++++++++++++++-
 tools/objtool/check.h         |    1 
 5 files changed, 93 insertions(+), 3 deletions(-)

--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -269,6 +269,10 @@ objtool_args += --no-unreachable
 else
 objtool_args += $(call cc-ifversion, -lt, 0405, --no-unreachable)
 endif
+ifdef CONFIG_RETPOLINE
+  objtool_args += --retpoline
+endif
+
 
 ifdef CONFIG_MODVERSIONS
 objtool_o = $(@D)/.tmp_$(@F)
--- 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;
+bool no_fp, no_unreachable, retpoline;
 
 static const char * const check_usage[] = {
 	"objtool check [<options>] file.o",
@@ -39,6 +39,7 @@ static const char * const check_usage[]
 const struct option check_options[] = {
 	OPT_BOOLEAN('f', "no-fp", &no_fp, "Skip frame pointer validation"),
 	OPT_BOOLEAN('u', "no-unreachable", &no_unreachable, "Skip 'unreachable instruction' warnings"),
+	OPT_BOOLEAN('r', "retpoline", &retpoline, "Validate retpoline assumptions"),
 	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;
+extern bool no_fp, no_unreachable, retpoline;
 
 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
@@ -496,6 +496,7 @@ static int add_jump_destinations(struct
 			 * disguise, so convert them accordingly.
 			 */
 			insn->type = INSN_JUMP_DYNAMIC;
+			insn->retpoline_safe = true;
 			continue;
 		} else {
 			/* sibling call */
@@ -547,7 +548,8 @@ static int add_call_destinations(struct
 			if (!insn->call_dest && !insn->ignore) {
 				WARN_FUNC("unsupported intra-function call",
 					  insn->sec, insn->offset);
-				WARN("If this is a retpoline, please patch it in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE.");
+				if (retpoline)
+					WARN("If this is a retpoline, please patch it in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE.");
 				return -1;
 			}
 
@@ -1070,6 +1072,54 @@ static int read_unwind_hints(struct objt
 	return 0;
 }
 
+static int read_retpoline_hints(struct objtool_file *file)
+{
+	struct section *sec, *relasec;
+	struct instruction *insn;
+	struct rela *rela;
+	int i;
+
+	sec = find_section_by_name(file->elf, ".discard.retpoline_safe");
+	if (!sec)
+		return 0;
+
+	relasec = sec->rela;
+	if (!relasec) {
+		WARN("missing .rela.discard.retpoline_safe section");
+		return -1;
+	}
+
+	if (sec->len % sizeof(unsigned long)) {
+		WARN("retpoline_safe size mismatch: %d %ld", sec->len, sizeof(unsigned long));
+		return -1;
+	}
+
+	for (i = 0; i < sec->len / sizeof(unsigned long); i++) {
+		rela = find_rela_by_dest(sec, i * sizeof(unsigned long));
+		if (!rela) {
+			WARN("can't find rela for retpoline_safe[%d]", i);
+			return -1;
+		}
+
+		insn = find_insn(file, rela->sym->sec, rela->addend);
+		if (!insn) {
+			WARN("can't find insn for retpoline_safe[%d]", i);
+			return -1;
+		}
+
+		if (insn->type != INSN_JUMP_DYNAMIC &&
+		    insn->type != INSN_CALL_DYNAMIC) {
+			WARN_FUNC("retpoline_safe hint not a indirect jump/call",
+				  insn->sec, insn->offset);
+			return -1;
+		}
+
+		insn->retpoline_safe = true;
+	}
+
+	return 0;
+}
+
 static int decode_sections(struct objtool_file *file)
 {
 	int ret;
@@ -1108,6 +1158,10 @@ static int decode_sections(struct objtoo
 	if (ret)
 		return ret;
 
+	ret = read_retpoline_hints(file);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
@@ -1853,6 +1907,29 @@ static int validate_unwind_hints(struct
 	return warnings;
 }
 
+static int validate_retpoline(struct objtool_file *file)
+{
+	struct instruction *insn;
+	int warnings = 0;
+
+	for_each_insn(file, insn) {
+		if (insn->type != INSN_JUMP_DYNAMIC &&
+		    insn->type != INSN_CALL_DYNAMIC)
+			continue;
+
+		if (insn->retpoline_safe)
+			continue;
+
+		WARN_FUNC("indirect %s found in RETPOLINE build",
+			  insn->sec, insn->offset,
+			  insn->type == INSN_JUMP_DYNAMIC ? "jump" : "call");
+
+		warnings++;
+	}
+
+	return warnings;
+}
+
 static bool is_kasan_insn(struct instruction *insn)
 {
 	return (insn->type == INSN_CALL &&
@@ -2028,6 +2105,13 @@ int check(const char *_objname, bool orc
 	if (list_empty(&file.insn_list))
 		goto out;
 
+	if (retpoline) {
+		ret = validate_retpoline(&file);
+		if (ret < 0)
+			return ret;
+		warnings += ret;
+	}
+
 	ret = validate_functions(&file);
 	if (ret < 0)
 		goto out;
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -45,6 +45,7 @@ struct instruction {
 	unsigned char type;
 	unsigned long immediate;
 	bool alt_group, visited, dead_end, ignore, hint, save, restore, ignore_alts;
+	bool retpoline_safe;
 	struct symbol *call_dest;
 	struct instruction *jump_dest;
 	struct list_head alts;

[PATCH 3/7] objtool: Add module specific retpoline rules

[Peter Zijlstra]

[-- Attachment #1: peterz-objtool-module-rules.patch --]
[-- Type: text/plain, Size: 2443 bytes --]

David wanted to not use retpolines in .init.text but that will trip up
objtool retpoline validation, fix that.

Requested-by: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 scripts/Makefile.build        |    2 ++
 tools/objtool/builtin-check.c |    3 ++-
 tools/objtool/builtin.h       |    2 +-
 tools/objtool/check.c         |    9 +++++++++
 4 files changed, 14 insertions(+), 2 deletions(-)

--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -256,6 +256,8 @@ __objtool_obj := $(objtree)/tools/objtoo
 
 objtool_args = $(if $(CONFIG_UNWINDER_ORC),orc generate,check)
 
+objtool_args += $(if $(part-of-module), --module,)
+
 ifndef CONFIG_FRAME_POINTER
 objtool_args += --no-fp
 endif
--- 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;
+bool no_fp, no_unreachable, retpoline, module;
 
 static const char * const check_usage[] = {
 	"objtool check [<options>] file.o",
@@ -40,6 +40,7 @@ const struct option check_options[] = {
 	OPT_BOOLEAN('f', "no-fp", &no_fp, "Skip frame pointer validation"),
 	OPT_BOOLEAN('u', "no-unreachable", &no_unreachable, "Skip 'unreachable instruction' warnings"),
 	OPT_BOOLEAN('r', "retpoline", &retpoline, "Validate retpoline assumptions"),
+	OPT_BOOLEAN('m', "module", &module, "Indicates the object will be part of a kernel module"),
 	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;
+extern bool no_fp, no_unreachable, retpoline, module;
 
 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
@@ -1920,6 +1920,15 @@ static int validate_retpoline(struct obj
 		if (insn->retpoline_safe)
 			continue;
 
+		/*
+		 * .init.text code is ran before userspace and thus doesn't
+		 * strictly need retpolines, except for modules which are
+		 * loaded late, they very much do need retpoline in their
+		 * .init.text
+		 */
+		if (!strcmp(insn->sec->name, ".init.text") && !module)
+			continue;
+
 		WARN_FUNC("indirect %s found in RETPOLINE build",
 			  insn->sec, insn->offset,
 			  insn->type == INSN_JUMP_DYNAMIC ? "jump" : "call");

[PATCH 4/7] x86,nospec: Annotate indirect calls/jumps

[Peter Zijlstra]

[-- Attachment #1: peterz-retpoline-annotate-nospec.patch --]
[-- Type: text/plain, Size: 2358 bytes --]

Annotate the indirect calls/jumps in the CALL_NOSPEC/JUMP_NOSPEC
alternatives.

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/nospec-branch.h |   27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -67,6 +67,18 @@
 .endm
 
 /*
+ * This should be used immediately before an indirect jump/call. It tells
+ * objtool the subsequent indirect jump/call is vouched safe for retpoline
+ * builds.
+ */
+.macro ANNOTATE_RETPOLINE_SAFE
+	.Lannotate_\@:
+	.pushsection .discard.retpoline_safe
+	_ASM_PTR .Lannotate_\@
+	.popsection
+.endm
+
+/*
  * These are the bare retpoline primitives for indirect jmp and call.
  * Do not use these directly; they only exist to make the ALTERNATIVE
  * invocation below less ugly.
@@ -102,9 +114,9 @@
 .macro JMP_NOSPEC reg:req
 #ifdef CONFIG_RETPOLINE
 	ANNOTATE_NOSPEC_ALTERNATIVE
-	ALTERNATIVE_2 __stringify(jmp *\reg),				\
+	ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *\reg),	\
 		__stringify(RETPOLINE_JMP \reg), X86_FEATURE_RETPOLINE,	\
-		__stringify(lfence; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
+		__stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
 #else
 	jmp	*\reg
 #endif
@@ -113,9 +125,9 @@
 .macro CALL_NOSPEC reg:req
 #ifdef CONFIG_RETPOLINE
 	ANNOTATE_NOSPEC_ALTERNATIVE
-	ALTERNATIVE_2 __stringify(call *\reg),				\
+	ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; call *\reg),	\
 		__stringify(RETPOLINE_CALL \reg), X86_FEATURE_RETPOLINE,\
-		__stringify(lfence; call *\reg), X86_FEATURE_RETPOLINE_AMD
+		__stringify(lfence; ANNOTATE_RETPOLINE_SAFE; call *\reg), X86_FEATURE_RETPOLINE_AMD
 #else
 	call	*\reg
 #endif
@@ -143,6 +155,12 @@
 	".long 999b - .\n\t"					\
 	".popsection\n\t"
 
+#define ANNOTATE_RETPOLINE_SAFE					\
+	"999:\n\t"						\
+	".pushsection .discard.retpoline_safe\n\t"		\
+	_ASM_PTR " 999b\n\t"					\
+	".popsection\n\t"
+
 #if defined(CONFIG_X86_64) && defined(RETPOLINE)
 
 /*
@@ -152,6 +170,7 @@
 # define CALL_NOSPEC						\
 	ANNOTATE_NOSPEC_ALTERNATIVE				\
 	ALTERNATIVE(						\
+	ANNOTATE_RETPOLINE_SAFE					\
 	"call *%[thunk_target]\n",				\
 	"call __x86_indirect_thunk_%V[thunk_target]\n",		\
 	X86_FEATURE_RETPOLINE)

Re: [PATCH 4/7] x86,nospec: Annotate indirect calls/jumps

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1044 bytes --]



On Thu, 2018-02-01 at 15:34 +0100, Peter Zijlstra wrote:
> 
>   * These are the bare retpoline primitives for indirect jmp and call.
>   * Do not use these directly; they only exist to make the ALTERNATIVE
>   * invocation below less ugly.
> @@ -102,9 +114,9 @@
>  .macro JMP_NOSPEC reg:req
>  #ifdef CONFIG_RETPOLINE
>         ANNOTATE_NOSPEC_ALTERNATIVE
> -       ALTERNATIVE_2 __stringify(jmp *\reg),                           \
> +       ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *\reg),  \
>                 __stringify(RETPOLINE_JMP \reg), X86_FEATURE_RETPOLINE, \
> -               __stringify(lfence; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
> +               __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
>  #else
>         jmp     *\reg
>  #endif

The first one, yes. But the second one for the AMD retpoline is
redundant, isn't it? Objtool isn't going to look there.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]

Re: [PATCH 4/7] x86,nospec: Annotate indirect calls/jumps

[Peter Zijlstra]

On Thu, Feb 01, 2018 at 02:55:26PM +0000, David Woodhouse wrote:
> 
> 
> On Thu, 2018-02-01 at 15:34 +0100, Peter Zijlstra wrote:
> > 
> >   * These are the bare retpoline primitives for indirect jmp and call.
> >   * Do not use these directly; they only exist to make the ALTERNATIVE
> >   * invocation below less ugly.
> > @@ -102,9 +114,9 @@
> >  .macro JMP_NOSPEC reg:req
> >  #ifdef CONFIG_RETPOLINE
> >         ANNOTATE_NOSPEC_ALTERNATIVE
> > -       ALTERNATIVE_2 __stringify(jmp *\reg),                           \
> > +       ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *\reg),  \
> >                 __stringify(RETPOLINE_JMP \reg), X86_FEATURE_RETPOLINE, \
> > -               __stringify(lfence; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
> > +               __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
> >  #else
> >         jmp     *\reg
> >  #endif
> 
> The first one, yes. But the second one for the AMD retpoline is
> redundant, isn't it? Objtool isn't going to look there.

It was when I wrote it.. lemme try again.

Re: [PATCH 4/7] x86,nospec: Annotate indirect calls/jumps

[Peter Zijlstra]

On Thu, Feb 01, 2018 at 04:11:36PM +0100, Peter Zijlstra wrote:
> On Thu, Feb 01, 2018 at 02:55:26PM +0000, David Woodhouse wrote:
> > 
> > 
> > On Thu, 2018-02-01 at 15:34 +0100, Peter Zijlstra wrote:
> > > 
> > >   * These are the bare retpoline primitives for indirect jmp and call.
> > >   * Do not use these directly; they only exist to make the ALTERNATIVE
> > >   * invocation below less ugly.
> > > @@ -102,9 +114,9 @@
> > >  .macro JMP_NOSPEC reg:req
> > >  #ifdef CONFIG_RETPOLINE
> > >         ANNOTATE_NOSPEC_ALTERNATIVE
> > > -       ALTERNATIVE_2 __stringify(jmp *\reg),                           \
> > > +       ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *\reg),  \
> > >                 __stringify(RETPOLINE_JMP \reg), X86_FEATURE_RETPOLINE, \
> > > -               __stringify(lfence; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
> > > +               __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
> > >  #else
> > >         jmp     *\reg
> > >  #endif
> > 
> > The first one, yes. But the second one for the AMD retpoline is
> > redundant, isn't it? Objtool isn't going to look there.
> 
> It was when I wrote it.. lemme try again.

Insta complaint:

arch/x86/entry/.tmp_entry_64.o: warning: objtool: .altinstr_replacement+0x19: indirect jump found in RETPOLINE build

Re: [PATCH 4/7] x86,nospec: Annotate indirect calls/jumps

[Josh Poimboeuf]

On Thu, Feb 01, 2018 at 04:13:48PM +0100, Peter Zijlstra wrote:
> On Thu, Feb 01, 2018 at 04:11:36PM +0100, Peter Zijlstra wrote:
> > On Thu, Feb 01, 2018 at 02:55:26PM +0000, David Woodhouse wrote:
> > > 
> > > 
> > > On Thu, 2018-02-01 at 15:34 +0100, Peter Zijlstra wrote:
> > > > 
> > > >   * These are the bare retpoline primitives for indirect jmp and call.
> > > >   * Do not use these directly; they only exist to make the ALTERNATIVE
> > > >   * invocation below less ugly.
> > > > @@ -102,9 +114,9 @@
> > > >  .macro JMP_NOSPEC reg:req
> > > >  #ifdef CONFIG_RETPOLINE
> > > >         ANNOTATE_NOSPEC_ALTERNATIVE
> > > > -       ALTERNATIVE_2 __stringify(jmp *\reg),                           \
> > > > +       ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *\reg),  \
> > > >                 __stringify(RETPOLINE_JMP \reg), X86_FEATURE_RETPOLINE, \
> > > > -               __stringify(lfence; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
> > > > +               __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
> > > >  #else
> > > >         jmp     *\reg
> > > >  #endif
> > > 
> > > The first one, yes. But the second one for the AMD retpoline is
> > > redundant, isn't it? Objtool isn't going to look there.
> > 
> > It was when I wrote it.. lemme try again.
> 
> Insta complaint:
> 
> arch/x86/entry/.tmp_entry_64.o: warning: objtool: .altinstr_replacement+0x19: indirect jump found in RETPOLINE build

Right, objtool was recently made smarter, such that it actually decodes
the ignored alternatives.  The check for that warning needs to also
check that insn->ignore isn't set.

-- 
Josh

Re: [PATCH 4/7] x86,nospec: Annotate indirect calls/jumps

[Peter Zijlstra]

On Thu, Feb 01, 2018 at 09:21:34AM -0600, Josh Poimboeuf wrote:
> On Thu, Feb 01, 2018 at 04:13:48PM +0100, Peter Zijlstra wrote:

> > arch/x86/entry/.tmp_entry_64.o: warning: objtool: .altinstr_replacement+0x19: indirect jump found in RETPOLINE build
> 
> Right, objtool was recently made smarter, such that it actually decodes
> the ignored alternatives.

I think it always did, you just ignored the alternatives for the code
flow stuff.

> The check for that warning needs to also check that insn->ignore isn't
> set.

So I tried to keep the two annotations independent, thinking the code
flow ignore would eventually go away when we got smarter about it. It
even has a comment about that:

/*
 * 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.
 * But it at least allows objtool to understand the control flow *around* the
 * retpoline.
 */

So I'm not seeing how making retpoline_safe depend on nospec_ignores is
a good thing.

[PATCH 5/7] x86/paravirt: Annotate indirect calls

[Peter Zijlstra]

[-- Attachment #1: peterz-retpoline-annotate-pv.patch --]
[-- Type: text/plain, Size: 3813 bytes --]

Paravirt emits indirect calls which get flagged by objtool retpoline
checks, annotate it away because all these indirect calls will be
patched out before we start userspace.

This patching happens through alternative_instructions() ->
apply_paravirt() -> pv_init_ops.patch() which will eventually end up
in paravirt_patch_default(). This function _will_ write direct
alternatives.

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/paravirt.h       |   17 +++++++++++++----
 arch/x86/include/asm/paravirt_types.h |    5 ++++-
 2 files changed, 17 insertions(+), 5 deletions(-)

--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -7,6 +7,7 @@
 #ifdef CONFIG_PARAVIRT
 #include <asm/pgtable_types.h>
 #include <asm/asm.h>
+#include <asm/nospec-branch.h>
 
 #include <asm/paravirt_types.h>
 
@@ -879,23 +880,27 @@ extern void default_banner(void);
 
 #define INTERRUPT_RETURN						\
 	PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_iret), CLBR_NONE,	\
-		  jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_iret))
+		  ANNOTATE_RETPOLINE_SAFE;					\
+		  jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_iret);)
 
 #define DISABLE_INTERRUPTS(clobbers)					\
 	PARA_SITE(PARA_PATCH(pv_irq_ops, PV_IRQ_irq_disable), clobbers, \
 		  PV_SAVE_REGS(clobbers | CLBR_CALLEE_SAVE);		\
+		  ANNOTATE_RETPOLINE_SAFE;					\
 		  call PARA_INDIRECT(pv_irq_ops+PV_IRQ_irq_disable);	\
 		  PV_RESTORE_REGS(clobbers | CLBR_CALLEE_SAVE);)
 
 #define ENABLE_INTERRUPTS(clobbers)					\
 	PARA_SITE(PARA_PATCH(pv_irq_ops, PV_IRQ_irq_enable), clobbers,	\
 		  PV_SAVE_REGS(clobbers | CLBR_CALLEE_SAVE);		\
+		  ANNOTATE_RETPOLINE_SAFE;					\
 		  call PARA_INDIRECT(pv_irq_ops+PV_IRQ_irq_enable);	\
 		  PV_RESTORE_REGS(clobbers | CLBR_CALLEE_SAVE);)
 
 #ifdef CONFIG_X86_32
 #define GET_CR0_INTO_EAX				\
 	push %ecx; push %edx;				\
+	ANNOTATE_RETPOLINE_SAFE;				\
 	call PARA_INDIRECT(pv_cpu_ops+PV_CPU_read_cr0);	\
 	pop %edx; pop %ecx
 #else	/* !CONFIG_X86_32 */
@@ -917,21 +922,25 @@ extern void default_banner(void);
  */
 #define SWAPGS								\
 	PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_swapgs), CLBR_NONE,	\
-		  call PARA_INDIRECT(pv_cpu_ops+PV_CPU_swapgs)		\
+		  ANNOTATE_RETPOLINE_SAFE;					\
+		  call PARA_INDIRECT(pv_cpu_ops+PV_CPU_swapgs);		\
 		 )
 
 #define GET_CR2_INTO_RAX				\
-	call PARA_INDIRECT(pv_mmu_ops+PV_MMU_read_cr2)
+	ANNOTATE_RETPOLINE_SAFE;				\
+	call PARA_INDIRECT(pv_mmu_ops+PV_MMU_read_cr2);
 
 #define USERGS_SYSRET64							\
 	PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_usergs_sysret64),	\
 		  CLBR_NONE,						\
-		  jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_usergs_sysret64))
+		  ANNOTATE_RETPOLINE_SAFE;					\
+		  jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_usergs_sysret64);)
 
 #ifdef CONFIG_DEBUG_ENTRY
 #define SAVE_FLAGS(clobbers)                                        \
 	PARA_SITE(PARA_PATCH(pv_irq_ops, PV_IRQ_save_fl), clobbers, \
 		  PV_SAVE_REGS(clobbers | CLBR_CALLEE_SAVE);        \
+		  ANNOTATE_RETPOLINE_SAFE;				    \
 		  call PARA_INDIRECT(pv_irq_ops+PV_IRQ_save_fl);    \
 		  PV_RESTORE_REGS(clobbers | CLBR_CALLEE_SAVE);)
 #endif
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -43,6 +43,7 @@
 #include <asm/desc_defs.h>
 #include <asm/kmap_types.h>
 #include <asm/pgtable_types.h>
+#include <asm/nospec-branch.h>
 
 struct page;
 struct thread_struct;
@@ -392,7 +393,9 @@ int paravirt_disable_iospace(void);
  * offset into the paravirt_patch_template structure, and can therefore be
  * freely converted back into a structure offset.
  */
-#define PARAVIRT_CALL	"call *%c[paravirt_opptr];"
+#define PARAVIRT_CALL					\
+	ANNOTATE_RETPOLINE_SAFE				\
+	"call *%c[paravirt_opptr];"
 
 /*
  * These macros are intended to wrap calls through one of the paravirt

[PATCH 6/7] x86: Annotate indirect jump in head_64.S

[Peter Zijlstra]

[-- Attachment #1: peterz-objtool-retpoline-annotate.patch --]
[-- Type: text/plain, Size: 763 bytes --]

The objtool retpoline validation found this indirect jump. Seeing how
it's on CPU bringup before we run userspace it should be safe, annotate
it.

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/kernel/head_64.S |    2 ++
 1 file changed, 2 insertions(+)

--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -23,6 +23,7 @@
 #include <asm/nops.h>
 #include "../entry/calling.h"
 #include <asm/export.h>
+#include <asm/nospec-branch.h>
 
 #ifdef CONFIG_PARAVIRT
 #include <asm/asm-offsets.h>
@@ -134,6 +135,7 @@ ENTRY(secondary_startup_64)
 
 	/* Ensure I am executing from virtual addresses */
 	movq	$1f, %rax
+	ANNOTATE_RETPOLINE_SAFE
 	jmp	*%rax
 1:
 	UNWIND_HINT_EMPTY

[PATCH 7/7] x86,sme: Annotate indirect call

[Peter Zijlstra]

[-- Attachment #1: peterz-retpoline-annotate-sme.patch --]
[-- Type: text/plain, Size: 796 bytes --]

This is boot code, we run this _way_ before userspace comes along to
poison our branch predictor.

Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Borislav Petkov <bp@suse.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/mm/mem_encrypt_boot.S |    2 ++
 1 file changed, 2 insertions(+)

--- a/arch/x86/mm/mem_encrypt_boot.S
+++ b/arch/x86/mm/mem_encrypt_boot.S
@@ -15,6 +15,7 @@
 #include <asm/page.h>
 #include <asm/processor-flags.h>
 #include <asm/msr-index.h>
+#include <asm/nospec-branch.h>
 
 	.text
 	.code64
@@ -59,6 +60,7 @@ ENTRY(sme_encrypt_execute)
 	movq	%rax, %r8		/* Workarea encryption routine */
 	addq	$PAGE_SIZE, %r8		/* Workarea intermediate copy buffer */
 
+	ANNOTATE_RETPOLINE_SAFE
 	call	*%rax			/* Call the encryption routine */
 
 	pop	%r12

Re: [PATCH 0/7] objtool: retpoline validation

[Josh Poimboeuf]

On Thu, Feb 01, 2018 at 03:34:21PM +0100, Peter Zijlstra wrote:
> There are the retpoline validation patches; they work with the __noretpoline
> thing from David.

Have you run this through 0-day bot yet?  A manual awk/sed found another
one, which objtool confirms:

  drivers/watchdog/.tmp_hpwdt.o: warning: objtool: .text+0x24: indirect call found in RETPOLINE build

And my search wasn't exhaustive so it would be good to sic 0-day bot on
it.

-- 
Josh

Re: [PATCH 0/7] objtool: retpoline validation

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 751 bytes --]

On Thu, 2018-02-01 at 09:28 -0600, Josh Poimboeuf wrote:
> On Thu, Feb 01, 2018 at 03:34:21PM +0100, Peter Zijlstra wrote:
> > 
> > There are the retpoline validation patches; they work with the
> > __noretpoline
> > thing from David.
> Have you run this through 0-day bot yet?  A manual awk/sed found
> another
> one, which objtool confirms:
> 
>   drivers/watchdog/.tmp_hpwdt.o: warning: objtool: .text+0x24:
> indirect call found in RETPOLINE build
> 
> And my search wasn't exhaustive so it would be good to sic 0-day bot on
> it.

We discussed that one. It's correct; we're calling into firmware so
there's *no* point in retpolining that one. We need to set IBRS before
any runtime calls into firmware, if we want to be safe.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]

Re: [PATCH 0/7] objtool: retpoline validation

[Peter Zijlstra]

On Thu, Feb 01, 2018 at 03:32:11PM +0000, David Woodhouse wrote:
> On Thu, 2018-02-01 at 09:28 -0600, Josh Poimboeuf wrote:
> > On Thu, Feb 01, 2018 at 03:34:21PM +0100, Peter Zijlstra wrote:
> > > 
> > > There are the retpoline validation patches; they work with the
> > > __noretpoline
> > > thing from David.
> > Have you run this through 0-day bot yet?  A manual awk/sed found
> > another
> > one, which objtool confirms:
> > 
> >   drivers/watchdog/.tmp_hpwdt.o: warning: objtool: .text+0x24:
> > indirect call found in RETPOLINE build
> > 
> > And my search wasn't exhaustive so it would be good to sic 0-day bot on
> > it.
> 
> We discussed that one. It's correct; we're calling into firmware so
> there's *no* point in retpolining that one. We need to set IBRS before
> any runtime calls into firmware, if we want to be safe.

Ideally we'd have a way to mark the module 'unsafe' or something.

Re: [PATCH 0/7] objtool: retpoline validation

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1244 bytes --]

On Thu, 2018-02-01 at 16:40 +0100, Peter Zijlstra wrote:
> On Thu, Feb 01, 2018 at 03:32:11PM +0000, David Woodhouse wrote:
> > 
> > On Thu, 2018-02-01 at 09:28 -0600, Josh Poimboeuf wrote:
> > > 
> > > On Thu, Feb 01, 2018 at 03:34:21PM +0100, Peter Zijlstra wrote:
> > > > 
> > > > 
> > > > There are the retpoline validation patches; they work with the
> > > > __noretpoline
> > > > thing from David.
> > > Have you run this through 0-day bot yet?  A manual awk/sed found
> > > another
> > > one, which objtool confirms:
> > > 
> > >   drivers/watchdog/.tmp_hpwdt.o: warning: objtool: .text+0x24:
> > > indirect call found in RETPOLINE build
> > > 
> > > And my search wasn't exhaustive so it would be good to sic 0-day bot on
> > > it.
> > We discussed that one. It's correct; we're calling into firmware so
> > there's *no* point in retpolining that one. We need to set IBRS before
> > any runtime calls into firmware, if we want to be safe.
>
> Ideally we'd have a way to mark the module 'unsafe' or something.

No, we just need to set IBRS before doing it. The same applies to any
EFI runtime calls, APM and all kinds of other random crap that calls
into firmware. I'm not sure why those aren't showing up.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]

Re: [PATCH 0/7] objtool: retpoline validation

[Peter Zijlstra]

On Thu, Feb 01, 2018 at 04:51:35PM +0000, David Woodhouse wrote:
> > Ideally we'd have a way to mark the module 'unsafe' or something.
> 
> No, we just need to set IBRS before doing it. 

That would work, assuming IBRS is available to begin with of course. Do
we WARN if we hit this code and don't have IBRS available?

> The same applies to any
> EFI runtime calls, APM and all kinds of other random crap that calls
> into firmware. I'm not sure why those aren't showing up.

arch/x86/platform/efi/Makefile:OBJECT_FILES_NON_STANDARD_efi_thunk_$(BITS).o := y
arch/x86/platform/efi/Makefile:OBJECT_FILES_NON_STANDARD_efi_stub_$(BITS).o := y

And similar things tell objtool to please not look..

Re: [PATCH 0/7] objtool: retpoline validation

[Josh Poimboeuf]

On Thu, Feb 01, 2018 at 06:14:27PM +0100, Peter Zijlstra wrote:
> On Thu, Feb 01, 2018 at 04:51:35PM +0000, David Woodhouse wrote:
> > > Ideally we'd have a way to mark the module 'unsafe' or something.
> > 
> > No, we just need to set IBRS before doing it. 
> 
> That would work, assuming IBRS is available to begin with of course. Do
> we WARN if we hit this code and don't have IBRS available?

Perhaps it should just be reported in the spectre_v2 sysfs file.

So

  "Mitigation: Full generic retpoline"

would instead be something like

  "Vulnerable: Retpoline without IBRS"

?

> > The same applies to any
> > EFI runtime calls, APM and all kinds of other random crap that calls
> > into firmware. I'm not sure why those aren't showing up.
> 
> arch/x86/platform/efi/Makefile:OBJECT_FILES_NON_STANDARD_efi_thunk_$(BITS).o := y
> arch/x86/platform/efi/Makefile:OBJECT_FILES_NON_STANDARD_efi_stub_$(BITS).o := y
> 
> And similar things tell objtool to please not look..

Right, some of the corner cases like efi, vdso, and bpf tend to be
ignored by objtool right now.

-- 
Josh

Re: [PATCH 0/7] objtool: retpoline validation

[Tim Chen]

On 02/01/2018 08:51 AM, David Woodhouse wrote:

> 
> No, we just need to set IBRS before doing it. The same applies to any
> EFI runtime calls, APM and all kinds of other random crap that calls
> into firmware. I'm not sure why those aren't showing up.
> 

Dave,

Are you planning to update your IBRS for firmware call patches
and repost those?

Tim

Re: [PATCH 0/7] objtool: retpoline validation

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 533 bytes --]

On Thu, 2018-02-01 at 10:16 -0800, Tim Chen wrote:
> On 02/01/2018 08:51 AM, David Woodhouse wrote:
> > No, we just need to set IBRS before doing it. The same applies to any
> > EFI runtime calls, APM and all kinds of other random crap that calls
> > into firmware. I'm not sure why those aren't showing up.
> > 
> Dave,
> 
> Are you planning to update your IBRS for firmware call patches
> and repost those?

Yep. In the tree now...

http://git.infradead.org/users/dwmw2/linux-retpoline.git/shortlog/refs/heads/ibpb


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]

Re: [PATCH 0/7] objtool: retpoline validation

[Peter Zijlstra]

On Thu, Feb 01, 2018 at 09:28:56AM -0600, Josh Poimboeuf wrote:
> On Thu, Feb 01, 2018 at 03:34:21PM +0100, Peter Zijlstra wrote:
> > There are the retpoline validation patches; they work with the __noretpoline
> > thing from David.
> 
> Have you run this through 0-day bot yet? 

Yes, it complains a _lot_ because no retpoline supported compiler.

> A manual awk/sed found another
> one, which objtool confirms:
> 
>   drivers/watchdog/.tmp_hpwdt.o: warning: objtool: .text+0x24: indirect call found in RETPOLINE build

That is the only known one left. It calls into BIOS code, so its not
safe and using a retpoline for it is pointless since nobody audited the
BIOS code.

Re: [PATCH 0/7] objtool: retpoline validation

[Peter Zijlstra]

On Thu, Feb 01, 2018 at 04:32:35PM +0100, Peter Zijlstra wrote:
> On Thu, Feb 01, 2018 at 09:28:56AM -0600, Josh Poimboeuf wrote:
> > On Thu, Feb 01, 2018 at 03:34:21PM +0100, Peter Zijlstra wrote:
> > > There are the retpoline validation patches; they work with the __noretpoline
> > > thing from David.
> > 
> > Have you run this through 0-day bot yet? 
> 
> Yes, it complains a _lot_ because no retpoline supported compiler.

I think I finally got something that works there... Damn I hate
makefiles..


--- a/Makefile
+++ b/Makefile
@@ -486,6 +486,11 @@ KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG
 KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC)
 endif
 
+ifneq ($(call cc-option,-mindirect-branch=thunk-extern -mindirect-branch-register),)
+  CC_HAS_RETPOLINE := 1
+endif
+export CC_HAS_RETPOLINE
+
 ifeq ($(config-targets),1)
 # ===========================================================================
 # *config targets only - make sure prerequisites are updated, and descend
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -267,8 +267,10 @@ else
 objtool_args += $(call cc-ifversion, -lt, 0405, --no-unreachable)
 endif
 ifdef CONFIG_RETPOLINE
+ifdef CC_HAS_RETPOLINE
   objtool_args += --retpoline
 endif
+endif
 
 
 ifdef CONFIG_MODVERSIONS

Re: [PATCH 0/7] objtool: retpoline validation

[Josh Poimboeuf]

On Thu, Feb 01, 2018 at 03:34:21PM +0100, Peter Zijlstra wrote:
> There are the retpoline validation patches; they work with the __noretpoline
> thing from David.

For the series:

  Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh