All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/24] objtool: retpoline and asm-goto validation
@ 2018-01-23 15:25 Peter Zijlstra
  2018-01-23 15:25 ` [PATCH 01/24] objtool: Use existing global variables for options Peter Zijlstra
                   ` (24 more replies)
  0 siblings, 25 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

This series adds two features to objtool:

 - retpoline validation; it verifies no indirect jumps/calls are left in
   CONFIG_RETPOLINE=y builds.

   includes fixups for kvm and others

 - asm-goto validation; allows asserting that a block of code is not reachable
   through a dynamic branch.

After this it forces an asm-goto capable compiler to build arch/x86 and removes
the fallback code for static_cpu_has(). After which we can use static_cpu_has()
to guarantee a lack of dynamic branches, which is a requested feature for
various spectre related things.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 01/24] objtool: Use existing global variables for options
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-23 15:25 ` [PATCH 02/24] objtool: Add retpoline validation Peter Zijlstra
                   ` (23 subsequent siblings)
  24 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

[-- Attachment #1: peterz-objtool-unclutter.patch --]
[-- Type: text/plain, Size: 2770 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;
@@ -51,7 +47,7 @@ int cmd_orc(int argc, const char **argv)
 
 		objname = argv[0];
 
-		return check(objname, no_fp, no_unreachable, true);
+		return check(objname, true);
 
 	}
 
--- 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,
@@ -1973,13 +1973,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);

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 02/24] objtool: Add retpoline validation
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
  2018-01-23 15:25 ` [PATCH 01/24] objtool: Use existing global variables for options Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-23 18:21   ` Borislav Petkov
  2018-01-26  9:54   ` David Woodhouse
  2018-01-23 15:25 ` [PATCH 03/24] x86/paravirt: Annotate indirect calls Peter Zijlstra
                   ` (22 subsequent siblings)
  24 siblings, 2 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

[-- Attachment #1: peterz-objtool-indirect.patch --]
[-- Type: text/plain, Size: 5514 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.

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         |   87 ++++++++++++++++++++++++++++++++++++++++--
 tools/objtool/check.h         |    1 
 5 files changed, 92 insertions(+), 5 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 */
@@ -548,13 +549,11 @@ static int add_call_destinations(struct
 			 * normal for a function to call within itself.  So
 			 * disable this warning for now.
 			 */
-#if 0
-			if (!insn->call_dest) {
+			if (!retpoline && !insn->call_dest) {
 				WARN_FUNC("can't find call dest symbol at offset 0x%lx",
 					  insn->sec, insn->offset, dest_off);
 				return -1;
 			}
-#endif
 		} else if (rela->sym->type == STT_SECTION) {
 			insn->call_dest = find_symbol_by_offset(rela->sym->sec,
 								rela->addend+4);
@@ -1067,6 +1066,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;
@@ -1105,6 +1152,10 @@ static int decode_sections(struct objtoo
 	if (ret)
 		return ret;
 
+	ret = read_retpoline_hints(file);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
@@ -1848,6 +1899,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 &&
@@ -2002,6 +2076,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;

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 03/24] x86/paravirt: Annotate indirect calls
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
  2018-01-23 15:25 ` [PATCH 01/24] objtool: Use existing global variables for options Peter Zijlstra
  2018-01-23 15:25 ` [PATCH 02/24] objtool: Add retpoline validation Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-25 10:02   ` David Woodhouse
                     ` (2 more replies)
  2018-01-23 15:25 ` [PATCH 04/24] x86,nospec: Annotate indirect calls/jumps Peter Zijlstra
                   ` (21 subsequent siblings)
  24 siblings, 3 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

[-- Attachment #1: peterz-retpoline-annotate-pv.patch --]
[-- Type: text/plain, Size: 3584 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.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/paravirt.h       |   22 ++++++++++++++++++----
 arch/x86/include/asm/paravirt_types.h |    7 ++++++-
 2 files changed, 24 insertions(+), 5 deletions(-)

--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -828,6 +828,12 @@ extern void default_banner(void);
 	 .short clobbers;			\
 	.popsection
 
+#define PARA_RETPOLINE_SAFE				\
+	773:;						\
+	.pushsection .discard.retpoline_safe;		\
+	_ASM_PTR 773b;					\
+	.popsection
+
 
 #define COND_PUSH(set, mask, reg)			\
 	.if ((~(set)) & mask); push %reg; .endif
@@ -879,23 +885,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))
+		  PARA_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);		\
+		  PARA_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);		\
+		  PARA_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;				\
+	PARA_RETPOLINE_SAFE;				\
 	call PARA_INDIRECT(pv_cpu_ops+PV_CPU_read_cr0);	\
 	pop %edx; pop %ecx
 #else	/* !CONFIG_X86_32 */
@@ -917,21 +927,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)		\
+		  PARA_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)
+	PARA_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))
+		  PARA_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);        \
+		  PARA_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
@@ -392,7 +392,12 @@ 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					\
+	"773:;\n"					\
+	".pushsection .discard.retpoline_safe\n"	\
+	_ASM_PTR " 773b\n"				\
+	".popsection\n"					\
+	"call *%c[paravirt_opptr];"
 
 /*
  * These macros are intended to wrap calls through one of the paravirt

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 04/24] x86,nospec: Annotate indirect calls/jumps
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (2 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 03/24] x86/paravirt: Annotate indirect calls Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-26 10:19   ` David Woodhouse
  2018-01-23 15:25 ` [PATCH 05/24] x86: Annotate indirect jump in head_64.S Peter Zijlstra
                   ` (20 subsequent siblings)
  24 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

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

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


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)

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 05/24] x86: Annotate indirect jump in head_64.S
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (3 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 04/24] x86,nospec: Annotate indirect calls/jumps Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-26 10:24   ` David Woodhouse
  2018-01-23 15:25 ` [PATCH 06/24] x86,kvm: Fix indirect calls in emulator Peter Zijlstra
                   ` (19 subsequent siblings)
  24 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

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

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

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

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 06/24] x86,kvm: Fix indirect calls in emulator
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (4 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 05/24] x86: Annotate indirect jump in head_64.S Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-23 20:28   ` Borislav Petkov
  2018-01-23 15:25 ` [PATCH 07/24] x86,vmx: Fix indirect call Peter Zijlstra
                   ` (18 subsequent siblings)
  24 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

[-- Attachment #1: peterz-kvm-retpoline.patch --]
[-- Type: text/plain, Size: 1429 bytes --]

Replace the indirect calls with CALL_NOSPEC.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/kvm/emulate.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -25,6 +25,7 @@
 #include <asm/kvm_emulate.h>
 #include <linux/stringify.h>
 #include <asm/debugreg.h>
+#include <asm/nospec-branch.h>
 
 #include "x86.h"
 #include "tss.h"
@@ -1021,8 +1022,8 @@ static __always_inline u8 test_cc(unsign
 	void (*fop)(void) = (void *)em_setcc + 4 * (condition & 0xf);
 
 	flags = (flags & EFLAGS_MASK) | X86_EFLAGS_IF;
-	asm("push %[flags]; popf; call *%[fastop]"
-	    : "=a"(rc) : [fastop]"r"(fop), [flags]"r"(flags));
+	asm("push %[flags]; popf; " CALL_NOSPEC
+	    : "=a"(rc) : [thunk_target]"r"(fop), [flags]"r"(flags));
 	return rc;
 }
 
@@ -5305,9 +5306,9 @@ static int fastop(struct x86_emulate_ctx
 	if (!(ctxt->d & ByteOp))
 		fop += __ffs(ctxt->dst.bytes) * FASTOP_SIZE;
 
-	asm("push %[flags]; popf; call *%[fastop]; pushf; pop %[flags]\n"
+	asm("push %[flags]; popf; " CALL_NOSPEC " ; pushf; pop %[flags]\n"
 	    : "+a"(ctxt->dst.val), "+d"(ctxt->src.val), [flags]"+D"(flags),
-	      [fastop]"+S"(fop), ASM_CALL_CONSTRAINT
+	      [thunk_target]"+S"(fop), ASM_CALL_CONSTRAINT
 	    : "c"(ctxt->src2.val));
 
 	ctxt->eflags = (ctxt->eflags & ~EFLAGS_MASK) | (flags & EFLAGS_MASK);

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 07/24] x86,vmx: Fix indirect call
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (5 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 06/24] x86,kvm: Fix indirect calls in emulator Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-25  9:36   ` Peter Zijlstra
  2018-01-23 15:25 ` [PATCH 08/24] x86,sme: Annotate " Peter Zijlstra
                   ` (17 subsequent siblings)
  24 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

[-- Attachment #1: peterz-kvm-vmx-retpoline.patch --]
[-- Type: text/plain, Size: 641 bytes --]

Replace indirect call with CALL_NOSPEC.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/kvm/vmx.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -9064,14 +9064,14 @@ static void vmx_handle_external_intr(str
 #endif
 			"pushf\n\t"
 			__ASM_SIZE(push) " $%c[cs]\n\t"
-			"call *%[entry]\n\t"
+			CALL_NOSPEC
 			:
 #ifdef CONFIG_X86_64
 			[sp]"=&r"(tmp),
 #endif
 			ASM_CALL_CONSTRAINT
 			:
-			[entry]"r"(entry),
+			[thunk_target]"r"(entry),
 			[ss]"i"(__KERNEL_DS),
 			[cs]"i"(__KERNEL_CS)
 			);

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 08/24] x86,sme: Annotate indirect call
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (6 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 07/24] x86,vmx: Fix indirect call Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-26 10:37   ` David Woodhouse
  2018-01-23 15:25 ` [PATCH 09/24] jump_label: Add branch hints to static_branch_{un,}likely() Peter Zijlstra
                   ` (16 subsequent siblings)
  24 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra, Tom Lendacky,
	Borislav Petkov

[-- 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

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 09/24] jump_label: Add branch hints to static_branch_{un,}likely()
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (7 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 08/24] x86,sme: Annotate " Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-24 18:46   ` Borislav Petkov
  2018-01-23 15:25 ` [PATCH 10/24] sched: Optimize ttwu_stat() Peter Zijlstra
                   ` (15 subsequent siblings)
  24 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

[-- Attachment #1: peterz-jump_label-fix.patch --]
[-- Type: text/plain, Size: 935 bytes --]

For some reason these were missing, I've not observed this patch
making a difference in the few code locations I checked, but this
makes sense.

Cc: Jason Baron <jbaron@akamai.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/jump_label.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -388,7 +388,7 @@ extern bool ____wrong_branch_error(void)
 		branch = !arch_static_branch_jump(&(x)->key, true);		\
 	else									\
 		branch = ____wrong_branch_error();				\
-	branch;									\
+	likely(branch);								\
 })
 
 #define static_branch_unlikely(x)						\
@@ -400,7 +400,7 @@ extern bool ____wrong_branch_error(void)
 		branch = arch_static_branch(&(x)->key, false);			\
 	else									\
 		branch = ____wrong_branch_error();				\
-	branch;									\
+	unlikely(branch);							\
 })
 
 #else /* !HAVE_JUMP_LABEL */

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 10/24] sched: Optimize ttwu_stat()
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (8 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 09/24] jump_label: Add branch hints to static_branch_{un,}likely() Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-23 15:25 ` [PATCH 11/24] x86: Reindent _static_cpu_has Peter Zijlstra
                   ` (14 subsequent siblings)
  24 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

[-- Attachment #1: peterz-sched-opt-ttwu_stat.patch --]
[-- Type: text/plain, Size: 2601 bytes --]

The whole of ttwu_stat() is guarded by a single schedstat_enabled(),
there is absolutely no point in then issuing another static_branch for
every single schedstat_inc() in there.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/core.c  |   16 ++++++++--------
 kernel/sched/stats.h |    2 ++
 2 files changed, 10 insertions(+), 8 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1630,16 +1630,16 @@ ttwu_stat(struct task_struct *p, int cpu
 
 #ifdef CONFIG_SMP
 	if (cpu == rq->cpu) {
-		schedstat_inc(rq->ttwu_local);
-		schedstat_inc(p->se.statistics.nr_wakeups_local);
+		__schedstat_inc(rq->ttwu_local);
+		__schedstat_inc(p->se.statistics.nr_wakeups_local);
 	} else {
 		struct sched_domain *sd;
 
-		schedstat_inc(p->se.statistics.nr_wakeups_remote);
+		__schedstat_inc(p->se.statistics.nr_wakeups_remote);
 		rcu_read_lock();
 		for_each_domain(rq->cpu, sd) {
 			if (cpumask_test_cpu(cpu, sched_domain_span(sd))) {
-				schedstat_inc(sd->ttwu_wake_remote);
+				__schedstat_inc(sd->ttwu_wake_remote);
 				break;
 			}
 		}
@@ -1647,14 +1647,14 @@ ttwu_stat(struct task_struct *p, int cpu
 	}
 
 	if (wake_flags & WF_MIGRATED)
-		schedstat_inc(p->se.statistics.nr_wakeups_migrate);
+		__schedstat_inc(p->se.statistics.nr_wakeups_migrate);
 #endif /* CONFIG_SMP */
 
-	schedstat_inc(rq->ttwu_count);
-	schedstat_inc(p->se.statistics.nr_wakeups);
+	__schedstat_inc(rq->ttwu_count);
+	__schedstat_inc(p->se.statistics.nr_wakeups);
 
 	if (wake_flags & WF_SYNC)
-		schedstat_inc(p->se.statistics.nr_wakeups_sync);
+		__schedstat_inc(p->se.statistics.nr_wakeups_sync);
 }
 
 static inline void ttwu_activate(struct rq *rq, struct task_struct *p, int en_flags)
--- a/kernel/sched/stats.h
+++ b/kernel/sched/stats.h
@@ -31,6 +31,7 @@ rq_sched_info_dequeued(struct rq *rq, un
 		rq->rq_sched_info.run_delay += delta;
 }
 #define schedstat_enabled()		static_branch_unlikely(&sched_schedstats)
+#define __schedstat_inc(var)		do { var++; } while (0)
 #define schedstat_inc(var)		do { if (schedstat_enabled()) { var++; } } while (0)
 #define schedstat_add(var, amt)		do { if (schedstat_enabled()) { var += (amt); } } while (0)
 #define schedstat_set(var, val)		do { if (schedstat_enabled()) { var = (val); } } while (0)
@@ -48,6 +49,7 @@ static inline void
 rq_sched_info_depart(struct rq *rq, unsigned long long delta)
 {}
 #define schedstat_enabled()		0
+#define __schedstat_inc(var)		do { } while (0)
 #define schedstat_inc(var)		do { } while (0)
 #define schedstat_add(var, amt)		do { } while (0)
 #define schedstat_set(var, val)		do { } while (0)

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 11/24] x86: Reindent _static_cpu_has
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (9 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 10/24] sched: Optimize ttwu_stat() Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-23 15:25 ` [PATCH 12/24] x86: Update _static_cpu_has to use all named variables Peter Zijlstra
                   ` (13 subsequent siblings)
  24 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra, Borislav Petkov

[-- Attachment #1: peterz-fix-static_cpu_has.patch --]
[-- Type: text/plain, Size: 3020 bytes --]

Because its daft..

Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/cpufeature.h |   78 +++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 39 deletions(-)

--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -145,45 +145,45 @@ extern void clear_cpu_cap(struct cpuinfo
  */
 static __always_inline __pure bool _static_cpu_has(u16 bit)
 {
-		asm_volatile_goto("1: jmp 6f\n"
-			 "2:\n"
-			 ".skip -(((5f-4f) - (2b-1b)) > 0) * "
-			         "((5f-4f) - (2b-1b)),0x90\n"
-			 "3:\n"
-			 ".section .altinstructions,\"a\"\n"
-			 " .long 1b - .\n"		/* src offset */
-			 " .long 4f - .\n"		/* repl offset */
-			 " .word %P1\n"			/* always replace */
-			 " .byte 3b - 1b\n"		/* src len */
-			 " .byte 5f - 4f\n"		/* repl len */
-			 " .byte 3b - 2b\n"		/* pad len */
-			 ".previous\n"
-			 ".section .altinstr_replacement,\"ax\"\n"
-			 "4: jmp %l[t_no]\n"
-			 "5:\n"
-			 ".previous\n"
-			 ".section .altinstructions,\"a\"\n"
-			 " .long 1b - .\n"		/* src offset */
-			 " .long 0\n"			/* no replacement */
-			 " .word %P0\n"			/* feature bit */
-			 " .byte 3b - 1b\n"		/* src len */
-			 " .byte 0\n"			/* repl len */
-			 " .byte 0\n"			/* pad len */
-			 ".previous\n"
-			 ".section .altinstr_aux,\"ax\"\n"
-			 "6:\n"
-			 " testb %[bitnum],%[cap_byte]\n"
-			 " jnz %l[t_yes]\n"
-			 " jmp %l[t_no]\n"
-			 ".previous\n"
-			 : : "i" (bit), "i" (X86_FEATURE_ALWAYS),
-			     [bitnum] "i" (1 << (bit & 7)),
-			     [cap_byte] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3])
-			 : : t_yes, t_no);
-	t_yes:
-		return true;
-	t_no:
-		return false;
+	asm_volatile_goto("1: jmp 6f\n"
+		 "2:\n"
+		 ".skip -(((5f-4f) - (2b-1b)) > 0) * "
+			 "((5f-4f) - (2b-1b)),0x90\n"
+		 "3:\n"
+		 ".section .altinstructions,\"a\"\n"
+		 " .long 1b - .\n"		/* src offset */
+		 " .long 4f - .\n"		/* repl offset */
+		 " .word %P1\n"			/* always replace */
+		 " .byte 3b - 1b\n"		/* src len */
+		 " .byte 5f - 4f\n"		/* repl len */
+		 " .byte 3b - 2b\n"		/* pad len */
+		 ".previous\n"
+		 ".section .altinstr_replacement,\"ax\"\n"
+		 "4: jmp %l[t_no]\n"
+		 "5:\n"
+		 ".previous\n"
+		 ".section .altinstructions,\"a\"\n"
+		 " .long 1b - .\n"		/* src offset */
+		 " .long 0\n"			/* no replacement */
+		 " .word %P0\n"			/* feature bit */
+		 " .byte 3b - 1b\n"		/* src len */
+		 " .byte 0\n"			/* repl len */
+		 " .byte 0\n"			/* pad len */
+		 ".previous\n"
+		 ".section .altinstr_aux,\"ax\"\n"
+		 "6:\n"
+		 " testb %[bitnum],%[cap_byte]\n"
+		 " jnz %l[t_yes]\n"
+		 " jmp %l[t_no]\n"
+		 ".previous\n"
+		 : : "i" (bit), "i" (X86_FEATURE_ALWAYS),
+		     [bitnum] "i" (1 << (bit & 7)),
+		     [cap_byte] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3])
+		 : : t_yes, t_no);
+t_yes:
+	return true;
+t_no:
+	return false;
 }
 
 #define static_cpu_has(bit)					\

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 12/24] x86: Update _static_cpu_has to use all named variables
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (10 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 11/24] x86: Reindent _static_cpu_has Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-25 19:31   ` Borislav Petkov
  2018-01-23 15:25 ` [PATCH 13/24] objtool: Implement base jump_assert support Peter Zijlstra
                   ` (12 subsequent siblings)
  24 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

[-- Attachment #1: peterz-fix-static_cpu_has_names.patch --]
[-- Type: text/plain, Size: 1491 bytes --]


Requested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/cpufeature.h |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -153,7 +153,7 @@ static __always_inline __pure bool _stat
 		 ".section .altinstructions,\"a\"\n"
 		 " .long 1b - .\n"		/* src offset */
 		 " .long 4f - .\n"		/* repl offset */
-		 " .word %P1\n"			/* always replace */
+		 " .word %P[always]\n"		/* always replace */
 		 " .byte 3b - 1b\n"		/* src len */
 		 " .byte 5f - 4f\n"		/* repl len */
 		 " .byte 3b - 2b\n"		/* pad len */
@@ -165,7 +165,7 @@ static __always_inline __pure bool _stat
 		 ".section .altinstructions,\"a\"\n"
 		 " .long 1b - .\n"		/* src offset */
 		 " .long 0\n"			/* no replacement */
-		 " .word %P0\n"			/* feature bit */
+		 " .word %P[feature]\n"		/* feature bit */
 		 " .byte 3b - 1b\n"		/* src len */
 		 " .byte 0\n"			/* repl len */
 		 " .byte 0\n"			/* pad len */
@@ -176,8 +176,9 @@ static __always_inline __pure bool _stat
 		 " jnz %l[t_yes]\n"
 		 " jmp %l[t_no]\n"
 		 ".previous\n"
-		 : : "i" (bit), "i" (X86_FEATURE_ALWAYS),
-		     [bitnum] "i" (1 << (bit & 7)),
+		 : : [feature]  "i" (bit),
+		     [always]   "i" (X86_FEATURE_ALWAYS),
+		     [bitnum]   "i" (1 << (bit & 7)),
 		     [cap_byte] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3])
 		 : : t_yes, t_no);
 t_yes:

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 13/24] objtool: Implement base jump_assert support
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (11 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 12/24] x86: Update _static_cpu_has to use all named variables Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-26 10:45   ` David Woodhouse
  2018-01-23 15:25 ` [PATCH 14/24] x86: Add a type field to alt_instr Peter Zijlstra
                   ` (11 subsequent siblings)
  24 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra, Borislav Petkov

[-- Attachment #1: peter_zijlstra-q-objtool_vs_jump_label.patch --]
[-- Type: text/plain, Size: 4058 bytes --]

Implement a jump_label assertion that asserts that the code location
is indeed only reachable through a static_branch. Because if GCC is
absolutely retaded it could generate code like:

	xor rax,rax
	NOP/JMP 1f
	mov $1, rax
1:
	test rax,rax
	jz 2f
	<do-code>
2:

instead of the sensible:

	NOP/JMP 1f
	<do-code>
1:

This implements objtool infrastructure for ensuring the code ends up
sane, since we'll rely on that for correctness and security.

We tag the instructions after the static branch with static_jump_dest=true;
that is the instruction after the NOP and the instruction at the
JMP+disp site.

Then, when we read the .discard.jump_assert section, we assert that
each entry points to an instruction that has static_jump_dest set.

With this we can assert that the code emitted for the if statement
ends up at the static jump location and nothing untowards happened.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/check.c |   70 ++++++++++++++++++++++++++++++++++++++++++++++++--
 tools/objtool/check.h |    2 -
 2 files changed, 69 insertions(+), 3 deletions(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -686,8 +686,17 @@ static int handle_jump_alt(struct objtoo
 			   struct instruction *orig_insn,
 			   struct instruction **new_insn)
 {
-	if (orig_insn->type == INSN_NOP)
+	struct instruction *next_insn = list_next_entry(orig_insn, list);
+
+	if (orig_insn->type == INSN_NOP) {
+		/*
+		 * If orig_insn is a NOP, then new_insn is the branch target
+		 * for when it would've been a JMP.
+		 */
+		next_insn->static_jump_dest = true;
+		(*new_insn)->static_jump_dest = true;
 		return 0;
+	}
 
 	if (orig_insn->type != INSN_JUMP_UNCONDITIONAL) {
 		WARN_FUNC("unsupported instruction at jump label",
@@ -695,7 +704,16 @@ static int handle_jump_alt(struct objtoo
 		return -1;
 	}
 
-	*new_insn = list_next_entry(orig_insn, list);
+	/*
+	 * Otherwise, orig_insn is a JMP and it will have orig_insn->jump_dest.
+	 * In this case we'll effectively NOP the alt by pointing new_insn at
+	 * next_insn.
+	 */
+	orig_insn->jump_dest->static_jump_dest = true;
+	next_insn->static_jump_dest = true;
+
+	*new_insn = next_insn;
+
 	return 0;
 }
 
@@ -1114,6 +1132,50 @@ static int read_retpoline_hints(struct o
 	return 0;
 }
 
+static int assert_static_jumps(struct objtool_file *file)
+{
+	struct section *sec, *relasec;
+	struct instruction *insn;
+	struct rela *rela;
+	int i;
+
+	sec = find_section_by_name(file->elf, ".discard.jump_assert");
+	if (!sec)
+		return 0;
+
+	relasec = sec->rela;
+	if (!relasec) {
+		WARN("missing .rela.discard.jump_assert section");
+		return -1;
+	}
+
+	if (sec->len % sizeof(unsigned long)) {
+		WARN("jump_assert 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 jump_assert[%d]", i);
+			return -1;
+		}
+
+		insn = find_insn(file, rela->sym->sec, rela->addend);
+		if (!insn) {
+			WARN("can't find insn for jump_assert[%d]", i);
+			return -1;
+		}
+
+		if (!insn->static_jump_dest) {
+			WARN_FUNC("static assert FAIL", insn->sec, insn->offset);
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
 static int decode_sections(struct objtool_file *file)
 {
 	int ret;
@@ -2073,6 +2135,10 @@ int check(const char *_objname, bool orc
 		goto out;
 	warnings += ret;
 
+	ret = assert_static_jumps(&file);
+	if (ret < 0)
+		return ret;
+
 	if (list_empty(&file.insn_list))
 		goto out;
 
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -45,7 +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;
+	bool retpoline_safe, static_jump_dest;
 	struct symbol *call_dest;
 	struct instruction *jump_dest;
 	struct list_head alts;

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 14/24] x86: Add a type field to alt_instr
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (12 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 13/24] objtool: Implement base jump_assert support Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-23 15:25 ` [PATCH 15/24] x86: Annotate static_cpu_has alternative Peter Zijlstra
                   ` (10 subsequent siblings)
  24 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra, Borislav Petkov,
	Borislav Petkov

[-- Attachment #1: peterz-alternative-type.patch --]
[-- Type: text/plain, Size: 3784 bytes --]

Add a type field to the alternative description. For now this will be
used to annotate static_cpu_has() but possible future uses include
using it to implement alternative alignment and special NOP handling.

Suggested-by: Borislav Petkov <bp@alien8.de>
Reviewed-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/alternative-asm.h |    3 ++-
 arch/x86/include/asm/alternative.h     |    6 +++++-
 arch/x86/include/asm/cpufeature.h      |    2 ++
 arch/x86/kernel/alternative.c          |    4 ++--
 tools/objtool/special.c                |    2 +-
 5 files changed, 12 insertions(+), 5 deletions(-)

--- a/arch/x86/include/asm/alternative-asm.h
+++ b/arch/x86/include/asm/alternative-asm.h
@@ -25,13 +25,14 @@
  * enough information for the alternatives patching code to patch an
  * instruction. See apply_alternatives().
  */
-.macro altinstruction_entry orig alt feature orig_len alt_len pad_len
+.macro altinstruction_entry orig alt feature orig_len alt_len pad_len type=0
 	.long \orig - .
 	.long \alt - .
 	.word \feature
 	.byte \orig_len
 	.byte \alt_len
 	.byte \pad_len
+	.byte \type
 .endm
 
 /*
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -45,6 +45,8 @@
 #define LOCK_PREFIX ""
 #endif
 
+#define ALT_TYPE_DEFAULT	0
+
 struct alt_instr {
 	s32 instr_offset;	/* original instruction */
 	s32 repl_offset;	/* offset to replacement instruction */
@@ -52,6 +54,7 @@ struct alt_instr {
 	u8  instrlen;		/* length of original instruction */
 	u8  replacementlen;	/* length of new instruction */
 	u8  padlen;		/* length of build-time padding */
+	u8  type;		/* type of alternative */
 } __packed;
 
 /*
@@ -127,7 +130,8 @@ static inline int alternatives_text_rese
 	" .word " __stringify(feature) "\n"		/* feature bit     */ \
 	" .byte " alt_total_slen "\n"			/* source len      */ \
 	" .byte " alt_rlen(num) "\n"			/* replacement len */ \
-	" .byte " alt_pad_len "\n"			/* pad len */
+	" .byte " alt_pad_len "\n"			/* pad len */	      \
+	" .byte 0 \n"					/* type */
 
 #define ALTINSTR_REPLACEMENT(newinstr, feature, num)	/* replacement */     \
 	b_replacement(num)":\n\t" newinstr "\n" e_replacement(num) ":\n\t"
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -157,6 +157,7 @@ static __always_inline __pure bool _stat
 		 " .byte 3b - 1b\n"		/* src len */
 		 " .byte 5f - 4f\n"		/* repl len */
 		 " .byte 3b - 2b\n"		/* pad len */
+		 " .byte 0\n"			/* type */
 		 ".previous\n"
 		 ".section .altinstr_replacement,\"ax\"\n"
 		 "4: jmp %l[t_no]\n"
@@ -169,6 +170,7 @@ static __always_inline __pure bool _stat
 		 " .byte 3b - 1b\n"		/* src len */
 		 " .byte 0\n"			/* repl len */
 		 " .byte 0\n"			/* pad len */
+		 " .byte 0\n"			/* type */
 		 ".previous\n"
 		 ".section .altinstr_aux,\"ax\"\n"
 		 "6:\n"
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -400,11 +400,11 @@ void __init_or_module noinline apply_alt
 			continue;
 		}
 
-		DPRINTK("feat: %d*32+%d, old: (%p, len: %d), repl: (%p, len: %d), pad: %d",
+		DPRINTK("feat: %d*32+%d, old: (%p, len: %d), repl: (%p, len: %d), pad: %d, type: %d",
 			a->cpuid >> 5,
 			a->cpuid & 0x1f,
 			instr, a->instrlen,
-			replacement, a->replacementlen, a->padlen);
+			replacement, a->replacementlen, a->padlen, a->type);
 
 		DUMP_BYTES(instr, a->instrlen, "%p: old_insn: ", instr);
 		DUMP_BYTES(replacement, a->replacementlen, "%p: rpl_insn: ", replacement);
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -34,7 +34,7 @@
 #define JUMP_ORIG_OFFSET	0
 #define JUMP_NEW_OFFSET		8
 
-#define ALT_ENTRY_SIZE		13
+#define ALT_ENTRY_SIZE		14
 #define ALT_ORIG_OFFSET		0
 #define ALT_NEW_OFFSET		4
 #define ALT_FEATURE_OFFSET	8

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 15/24] x86: Annotate static_cpu_has alternative
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (13 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 14/24] x86: Add a type field to alt_instr Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-23 15:25 ` [PATCH 16/24] objtool: Implement jump_assert for _static_cpu_has() Peter Zijlstra
                   ` (9 subsequent siblings)
  24 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra, Borislav Petkov

[-- Attachment #1: peterz-x86-static_cpu_has-annotate.patch --]
[-- Type: text/plain, Size: 1938 bytes --]

In order to recognise static_cpu_has() alternatives from any other
alternative without dodgy heuristics, we need to explicitly mark them.
Use the new type field for this.

Reviewed-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/alternative.h |    6 ++++++
 arch/x86/include/asm/cpufeature.h  |    5 +++--
 2 files changed, 9 insertions(+), 2 deletions(-)

--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -47,6 +47,12 @@
 
 #define ALT_TYPE_DEFAULT	0
 
+/*
+ * This alt_instr descriptor is part of a static_cpu_has() construct. Use it to
+ * detect its type when processing with other tools, like objtool, for example.
+ */
+#define ALT_TYPE_STATIC_CPU_HAS	1
+
 struct alt_instr {
 	s32 instr_offset;	/* original instruction */
 	s32 repl_offset;	/* offset to replacement instruction */
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -157,7 +157,7 @@ static __always_inline __pure bool _stat
 		 " .byte 3b - 1b\n"		/* src len */
 		 " .byte 5f - 4f\n"		/* repl len */
 		 " .byte 3b - 2b\n"		/* pad len */
-		 " .byte 0\n"			/* type */
+		 " .byte %P[type]\n"		/* type */
 		 ".previous\n"
 		 ".section .altinstr_replacement,\"ax\"\n"
 		 "4: jmp %l[t_no]\n"
@@ -170,7 +170,7 @@ static __always_inline __pure bool _stat
 		 " .byte 3b - 1b\n"		/* src len */
 		 " .byte 0\n"			/* repl len */
 		 " .byte 0\n"			/* pad len */
-		 " .byte 0\n"			/* type */
+		 " .byte %P[type]\n"		/* type */
 		 ".previous\n"
 		 ".section .altinstr_aux,\"ax\"\n"
 		 "6:\n"
@@ -181,6 +181,7 @@ static __always_inline __pure bool _stat
 		 : : [feature]  "i" (bit),
 		     [always]   "i" (X86_FEATURE_ALWAYS),
 		     [bitnum]   "i" (1 << (bit & 7)),
+		     [type]	"i" (ALT_TYPE_STATIC_CPU_HAS),
 		     [cap_byte] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3])
 		 : : t_yes, t_no);
 t_yes:

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 16/24] objtool: Implement jump_assert for _static_cpu_has()
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (14 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 15/24] x86: Annotate static_cpu_has alternative Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-23 15:25 ` [PATCH 17/24] objtool: Introduce special_type Peter Zijlstra
                   ` (8 subsequent siblings)
  24 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra, Borislav Petkov

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

Unlike the jump_label bits, static_cpu_has is implemented with
alternatives. We use the new type field to distinguish them from any
other alternatives

Like jump_labels, make static_cpu_has set static_jump_dest on the
instructions after the static branch such that we can assert on it.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/check.c   |   21 +++++++++++++++++++++
 tools/objtool/special.c |   26 +++++++++++++++-----------
 tools/objtool/special.h |    1 +
 3 files changed, 37 insertions(+), 11 deletions(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -636,6 +636,12 @@ static int handle_group_alt(struct objto
 	fake_jump->ignore = true;
 
 	if (!special_alt->new_len) {
+		/*
+		 * The NOP case for _static_cpu_has()
+		 */
+		if (special_alt->static_cpu_has)
+			fake_jump->jump_dest->static_jump_dest = true;
+
 		*new_insn = fake_jump;
 		return 0;
 	}
@@ -664,6 +670,21 @@ static int handle_group_alt(struct objto
 				  insn->sec, insn->offset);
 			return -1;
 		}
+
+		if (special_alt->static_cpu_has) {
+			if (insn->type != INSN_JUMP_UNCONDITIONAL) {
+				WARN_FUNC("not an unconditional jump in _static_cpu_has()",
+					  insn->sec, insn->offset);
+			}
+			if (insn->jump_dest == fake_jump) {
+				WARN_FUNC("jump inside alternative for _static_cpu_has()",
+					  insn->sec, insn->offset);
+			}
+			/*
+			 * The JMP+disp case for _static_cpu_has()
+			 */
+			insn->jump_dest->static_jump_dest = true;
+		}
 	}
 
 	if (!last_new_insn) {
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -40,6 +40,10 @@
 #define ALT_FEATURE_OFFSET	8
 #define ALT_ORIG_LEN_OFFSET	10
 #define ALT_NEW_LEN_OFFSET	11
+#define ALT_TYPE_OFFSET		13
+
+#define ALT_TYPE_DEFAULT	0
+#define ALT_TYPE_STATIC_CPU_HAS	1
 
 #define X86_FEATURE_POPCNT (4*32+23)
 
@@ -48,7 +52,6 @@ struct special_entry {
 	bool group, jump_or_nop;
 	unsigned char size, orig, new;
 	unsigned char orig_len, new_len; /* group only */
-	unsigned char feature; /* ALTERNATIVE macro CPU feature */
 };
 
 struct special_entry entries[] = {
@@ -60,7 +63,6 @@ struct special_entry entries[] = {
 		.orig_len = ALT_ORIG_LEN_OFFSET,
 		.new = ALT_NEW_OFFSET,
 		.new_len = ALT_NEW_LEN_OFFSET,
-		.feature = ALT_FEATURE_OFFSET,
 	},
 	{
 		.sec = "__jump_table",
@@ -84,24 +86,23 @@ static int get_alt_entry(struct elf *elf
 {
 	struct rela *orig_rela, *new_rela;
 	unsigned long offset;
+	void *data;
 
 	offset = idx * entry->size;
+	data = sec->data->d_buf + offset;
 
 	alt->group = entry->group;
 	alt->jump_or_nop = entry->jump_or_nop;
 
 	if (alt->group) {
-		alt->orig_len = *(unsigned char *)(sec->data->d_buf + offset +
-						   entry->orig_len);
-		alt->new_len = *(unsigned char *)(sec->data->d_buf + offset +
-						  entry->new_len);
-	}
-
-	if (entry->feature) {
 		unsigned short feature;
+		unsigned char type;
 
-		feature = *(unsigned short *)(sec->data->d_buf + offset +
-					      entry->feature);
+		alt->orig_len = *(unsigned char *)(data + entry->orig_len);
+		alt->new_len = *(unsigned char *)(data + entry->new_len);
+
+		feature = *(unsigned short *)(data + ALT_FEATURE_OFFSET);
+		type = *(unsigned char *)(data + ALT_TYPE_OFFSET);
 
 		/*
 		 * It has been requested that we don't validate the !POPCNT
@@ -110,6 +111,9 @@ static int get_alt_entry(struct elf *elf
 		 */
 		if (feature == X86_FEATURE_POPCNT)
 			alt->skip_orig = true;
+
+		if (type == ALT_TYPE_STATIC_CPU_HAS)
+			alt->static_cpu_has = true;
 	}
 
 	orig_rela = find_rela_by_dest(sec, offset + entry->orig);
--- a/tools/objtool/special.h
+++ b/tools/objtool/special.h
@@ -27,6 +27,7 @@ struct special_alt {
 	bool group;
 	bool skip_orig;
 	bool jump_or_nop;
+	bool static_cpu_has;
 
 	struct section *orig_sec;
 	unsigned long orig_off;

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 17/24] objtool: Introduce special_type
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (15 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 16/24] objtool: Implement jump_assert for _static_cpu_has() Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-23 15:25 ` [PATCH 18/24] objtool: More complex static jump implementation Peter Zijlstra
                   ` (7 subsequent siblings)
  24 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

[-- Attachment #1: peterz-objtool-special-type.patch --]
[-- Type: text/plain, Size: 3157 bytes --]

Use an enum for the special_alt entries instead of a collection of
booleans.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/check.c   |   14 +++++++++++---
 tools/objtool/special.c |   14 +++++++-------
 tools/objtool/special.h |   10 ++++++++--
 3 files changed, 26 insertions(+), 12 deletions(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -773,7 +773,7 @@ static int add_special_section_alts(stru
 			continue;
 
 		new_insn = NULL;
-		if (!special_alt->group || special_alt->new_len) {
+		if (special_alt->type != alternative || special_alt->new_len) {
 			new_insn = find_insn(file, special_alt->new_sec,
 					     special_alt->new_off);
 			if (!new_insn) {
@@ -785,16 +785,24 @@ static int add_special_section_alts(stru
 			}
 		}
 
-		if (special_alt->group) {
+		switch (special_alt->type) {
+		case alternative:
 			ret = handle_group_alt(file, special_alt, orig_insn,
 					       &new_insn);
 			if (ret)
 				goto out;
-		} else if (special_alt->jump_or_nop) {
+			break;
+
+		case jump_label:
 			ret = handle_jump_alt(file, special_alt, orig_insn,
 					      &new_insn);
 			if (ret)
 				goto out;
+
+			break;
+
+		default:
+			break;
 		}
 
 		alt = malloc(sizeof(*alt));
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -49,7 +49,7 @@
 
 struct special_entry {
 	const char *sec;
-	bool group, jump_or_nop;
+	enum special_type type;
 	unsigned char size, orig, new;
 	unsigned char orig_len, new_len; /* group only */
 };
@@ -57,7 +57,7 @@ struct special_entry {
 struct special_entry entries[] = {
 	{
 		.sec = ".altinstructions",
-		.group = true,
+		.type = alternative,
 		.size = ALT_ENTRY_SIZE,
 		.orig = ALT_ORIG_OFFSET,
 		.orig_len = ALT_ORIG_LEN_OFFSET,
@@ -66,13 +66,14 @@ struct special_entry entries[] = {
 	},
 	{
 		.sec = "__jump_table",
-		.jump_or_nop = true,
+		.type = jump_label,
 		.size = JUMP_ENTRY_SIZE,
 		.orig = JUMP_ORIG_OFFSET,
 		.new = JUMP_NEW_OFFSET,
 	},
 	{
 		.sec = "__ex_table",
+		.type = exception,
 		.size = EX_ENTRY_SIZE,
 		.orig = EX_ORIG_OFFSET,
 		.new = EX_NEW_OFFSET,
@@ -91,10 +92,9 @@ static int get_alt_entry(struct elf *elf
 	offset = idx * entry->size;
 	data = sec->data->d_buf + offset;
 
-	alt->group = entry->group;
-	alt->jump_or_nop = entry->jump_or_nop;
+	alt->type = entry->type;
 
-	if (alt->group) {
+	if (alt->type == alternative) {
 		unsigned short feature;
 		unsigned char type;
 
@@ -130,7 +130,7 @@ static int get_alt_entry(struct elf *elf
 	alt->orig_sec = orig_rela->sym->sec;
 	alt->orig_off = orig_rela->addend;
 
-	if (!entry->group || alt->new_len) {
+	if (entry->type != alternative || alt->new_len) {
 		new_rela = find_rela_by_dest(sec, offset + entry->new);
 		if (!new_rela) {
 			WARN_FUNC("can't find new rela",
--- a/tools/objtool/special.h
+++ b/tools/objtool/special.h
@@ -21,12 +21,18 @@
 #include <stdbool.h>
 #include "elf.h"
 
+enum special_type {
+	alternative,
+	jump_label,
+	exception,
+};
+
 struct special_alt {
 	struct list_head list;
 
-	bool group;
+	enum special_type type;
+
 	bool skip_orig;
-	bool jump_or_nop;
 	bool static_cpu_has;
 
 	struct section *orig_sec;

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 18/24] objtool: More complex static jump implementation
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (16 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 17/24] objtool: Introduce special_type Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-23 15:25 ` [PATCH 19/24] objtool: Even more complex static block checks Peter Zijlstra
                   ` (6 subsequent siblings)
  24 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

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

When using something like:

  -#define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]))
  +#define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]) && \
  +			(arch_static_assert(), true))

we get an objtool assertion fail like:

kernel/sched/fair.o: warning: objtool: hrtick_update()+0xd: static assert FAIL

where:

0000000000001140 <hrtick_update>:
    1140:       0f 1f 44 00 00          nopl   0x0(%rax,%rax,1)
    1145:       c3                      retq
    1146:       48 8b b7 30 09 00 00    mov    0x930(%rdi),%rsi
    114d:       8b 87 d8 09 00 00       mov    0x9d8(%rdi),%eax
    1153:       48 0f a3 05 00 00 00    bt     %rax,0x0(%rip)        # 115b <hrtick_update+0x1b>
    115a:       00
                        1157: R_X86_64_PC32     __cpu_active_mask-0x4

and:

RELOCATION RECORDS FOR [__jump_table]:
0000000000000150 R_X86_64_64       .text+0x0000000000001140
0000000000000158 R_X86_64_64       .text+0x0000000000001146

RELOCATION RECORDS FOR [.discard.jump_assert]:
0000000000000028 R_X86_64_64       .text+0x000000000000114d

IOW, GCC managed to place the assertion 1 instruction _after_ the
static jump target (it lifted a load over it).

So while the code generation is fine, the assertion gets placed wrong.
We can 'fix' this by not only considering the immediate static jump
locations but also all the unconditional code after it, terminating
the basic block on any unconditional instruction or branch entry
point.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/check.c |   41 +++++++++++++++++++++++++++++++++++++++++
 tools/objtool/check.h |    1 +
 2 files changed, 42 insertions(+)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -520,6 +520,8 @@ static int add_jump_destinations(struct
 				  dest_off);
 			return -1;
 		}
+
+		insn->jump_dest->branch_target++;
 	}
 
 	return 0;
@@ -1205,6 +1207,41 @@ static int assert_static_jumps(struct ob
 	return 0;
 }
 
+static int grow_static_blocks(struct objtool_file *file)
+{
+	struct instruction *insn;
+	bool static_block = false;
+
+	for_each_insn(file, insn) {
+		if (!static_block && !insn->static_jump_dest)
+			continue;
+
+		if (insn->static_jump_dest) {
+			static_block = true;
+			continue;
+		}
+
+		if (insn->branch_target) {
+			static_block = false;
+			continue;
+		} else switch (insn->type) {
+		case INSN_JUMP_CONDITIONAL:
+		case INSN_JUMP_UNCONDITIONAL:
+		case INSN_JUMP_DYNAMIC:
+		case INSN_CALL:
+		case INSN_CALL_DYNAMIC:
+		case INSN_RETURN:
+		case INSN_BUG:
+			static_block = false;
+			continue;
+		}
+
+		insn->static_jump_dest = static_block;
+	}
+
+	return 0;
+}
+
 static int decode_sections(struct objtool_file *file)
 {
 	int ret;
@@ -1247,6 +1284,10 @@ static int decode_sections(struct objtoo
 	if (ret)
 		return ret;
 
+	ret = grow_static_blocks(file);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -46,6 +46,7 @@ struct instruction {
 	unsigned long immediate;
 	bool alt_group, visited, dead_end, ignore, hint, save, restore, ignore_alts;
 	bool retpoline_safe, static_jump_dest;
+	unsigned int branch_target;
 	struct symbol *call_dest;
 	struct instruction *jump_dest;
 	struct list_head alts;

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 19/24] objtool: Even more complex static block checks
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (17 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 18/24] objtool: More complex static jump implementation Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-23 15:25 ` [PATCH 20/24] objtool: Another static block fail Peter Zijlstra
                   ` (5 subsequent siblings)
  24 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

[-- Attachment #1: peterz-objtool-more-clever.patch --]
[-- Type: text/plain, Size: 3559 bytes --]

I've observed GCC transform:

  f()
  {
	  if (!static_branch_unlikely())
		  return;

	  static_assert();
	  A;
  }

  g()
  {
	  f();
  }

Into:

  f()
  {
	  static_assert();
	  A;
  }

  g()
  {
	  if (static_branch_unlikely())
		  f();
  }

Which results in the assertion landing at f+0. The transformation is
valid and useful; it avoids a pointless CALL+RET sequence, so we'll
have to teach objtool how to deal with this.

Do this by marking all CALL destinations with static_call when called
from a static_block and non_static_call when called outside a
static_block. This allows us to identify functions called exclusively
from a static_block and start them with a static_block.

Limit to static functions and do not apply recursive.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/check.c |   84 +++++++++++++++++++++++++++++++++++++-------------
 tools/objtool/elf.h   |    1 
 2 files changed, 64 insertions(+), 21 deletions(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1207,36 +1207,78 @@ static int assert_static_jumps(struct ob
 	return 0;
 }
 
+static bool __grow_static_block(struct objtool_file *file,
+				struct instruction *insn)
+{
+	/* if a !static jump can come in here, terminate */
+	if (insn->branch_target && !insn->static_jump_dest)
+		return false;
+
+	switch (insn->type) {
+	case INSN_JUMP_UNCONDITIONAL:
+		/* mark this instruction, terminate this section */
+		insn->static_jump_dest = true;
+		return false;
+
+	/* these disturb unconditional code flow, terminate */
+	case INSN_JUMP_CONDITIONAL:
+	case INSN_JUMP_DYNAMIC:
+	case INSN_RETURN:
+	case INSN_BUG:
+		return false;
+
+	/* these return right back and don't disturb the code flow */
+	case INSN_CALL:
+	case INSN_CALL_DYNAMIC:
+		break;
+	}
+
+	/* mark this insn, and continue the section */
+	insn->static_jump_dest = true;
+	return true;
+}
+
 static int grow_static_blocks(struct objtool_file *file)
 {
-	struct instruction *insn;
 	bool static_block = false;
+	struct symbol *func, *tmp;
+	struct instruction *insn;
+	struct section *sec;
 
 	for_each_insn(file, insn) {
-		if (!static_block && !insn->static_jump_dest)
-			continue;
+		if (static_block || insn->static_jump_dest)
+			static_block = __grow_static_block(file, insn);
 
-		if (insn->static_jump_dest) {
-			static_block = true;
-			continue;
+		if (insn->type == INSN_CALL) {
+			func = insn->call_dest;
+			if (!func)
+				continue;
+
+			if (static_block)
+				func->static_call = true;
+			else
+				func->non_static_call = true;
 		}
+	}
 
-		if (insn->branch_target) {
-			static_block = false;
-			continue;
-		} else switch (insn->type) {
-		case INSN_JUMP_CONDITIONAL:
-		case INSN_JUMP_UNCONDITIONAL:
-		case INSN_JUMP_DYNAMIC:
-		case INSN_CALL:
-		case INSN_CALL_DYNAMIC:
-		case INSN_RETURN:
-		case INSN_BUG:
-			static_block = false;
-			continue;
+	for_each_sec(file, sec) {
+		list_for_each_entry_safe(func, tmp, &sec->symbol_list, list) {
+			if (func->bind != STB_LOCAL)
+				continue;
+
+			if (!func->static_call)
+				continue;
+
+			if (func->non_static_call)
+				continue;
+
+			/* static && !non_static -- only static callers */
+
+			func_for_each_insn(file, func, insn) {
+				if (!__grow_static_block(file, insn))
+					break;
+			}
 		}
-
-		insn->static_jump_dest = static_block;
 	}
 
 	return 0;
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -61,6 +61,7 @@ struct symbol {
 	unsigned char bind, type;
 	unsigned long offset;
 	unsigned int len;
+	bool static_call, non_static_call;
 };
 
 struct rela {

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 20/24] objtool: Another static block fail
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (18 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 19/24] objtool: Even more complex static block checks Peter Zijlstra
@ 2018-01-23 15:25 ` Peter Zijlstra
  2018-01-29 22:52   ` Josh Poimboeuf
  2018-01-23 15:26 ` [PATCH 21/24] objtool: Skip static assert when KCOV/KASAN Peter Zijlstra
                   ` (4 subsequent siblings)
  24 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:25 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

[-- Attachment #1: peterz-objtool-more-clever-3.patch --]
[-- Type: text/plain, Size: 3086 bytes --]

I've observed GCC generate:

  sym:
     NOP/JMP 1f	(static_branch)
     JMP 2f
  1: /* crud */
     JMP 3f
  2: /* other crud */

  3: RETQ


This means we need to follow unconditional jumps; be conservative and
only follow if its a unique jump.

(I've not yet figured out which CONFIG option is responsible for this,
 a normal defconfig build does not generate crap like this)

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/check.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++---
 tools/objtool/elf.h   |    2 -
 2 files changed, 51 insertions(+), 4 deletions(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1207,6 +1207,8 @@ static int assert_static_jumps(struct ob
 	return 0;
 }
 
+static void jmp_grow_static_block(struct objtool_file *file, struct instruction *insn);
+
 static bool __grow_static_block(struct objtool_file *file,
 				struct instruction *insn)
 {
@@ -1216,7 +1218,8 @@ static bool __grow_static_block(struct o
 
 	switch (insn->type) {
 	case INSN_JUMP_UNCONDITIONAL:
-		/* mark this instruction, terminate this section */
+		/* follow the jump, mark this instruction, terminate this section */
+		jmp_grow_static_block(file, insn);
 		insn->static_jump_dest = true;
 		return false;
 
@@ -1238,6 +1241,50 @@ static bool __grow_static_block(struct o
 	return true;
 }
 
+static void jmp_grow_static_block(struct objtool_file *file, struct instruction *insn)
+{
+	struct instruction *dest = insn->jump_dest;
+	bool old_sjd, static_block;
+
+	if (!dest)
+		return;
+
+	/* more than a single site jumps here, can't be certain */
+	if (dest->branch_target > 1)
+		return;
+
+	/* mark the destination, so we can continue there */
+	old_sjd = dest->static_jump_dest;
+	dest->static_jump_dest = true;
+
+	if (dest->offset > insn->offset) {
+		/* fwd jump, the main iteration will still get there. */
+		return;
+	}
+
+	/* backwards jump, we need to revisit the instructions */
+	do {
+		static_block = __grow_static_block(file, dest);
+
+		if (insn->type == INSN_CALL && !old_sjd && dest->static_jump_dest) {
+			struct symbol *func = insn->call_dest;
+			if (!func || func->bind != STB_LOCAL)
+				goto next;
+
+			/*
+			 * we flipped the call to static, update the stats.
+			 */
+			if (insn->static_jump_dest) {
+				func->non_static_call--;
+				func->static_call++;
+			}
+		}
+next:
+		dest = list_next_entry(dest, list);
+		old_sjd = dest->static_jump_dest;
+	} while (static_block && dest != insn);
+}
+
 static int grow_static_blocks(struct objtool_file *file)
 {
 	bool static_block = false;
@@ -1255,9 +1302,9 @@ static int grow_static_blocks(struct obj
 				continue;
 
 			if (static_block)
-				func->static_call = true;
+				func->static_call++;
 			else
-				func->non_static_call = true;
+				func->non_static_call++;
 		}
 	}
 
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -61,7 +61,7 @@ struct symbol {
 	unsigned char bind, type;
 	unsigned long offset;
 	unsigned int len;
-	bool static_call, non_static_call;
+	unsigned int static_call, non_static_call;
 };
 
 struct rela {

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 21/24] objtool: Skip static assert when KCOV/KASAN
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (19 preceding siblings ...)
  2018-01-23 15:25 ` [PATCH 20/24] objtool: Another static block fail Peter Zijlstra
@ 2018-01-23 15:26 ` Peter Zijlstra
  2018-01-23 15:26 ` [PATCH 22/24] x86/jump_label: Implement arch_static_assert() Peter Zijlstra
                   ` (3 subsequent siblings)
  24 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:26 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

[-- Attachment #1: peterz-objtool-no-assert.patch --]
[-- Type: text/plain, Size: 2051 bytes --]

They make an absolutely horrid mess of things.

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

--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -272,6 +272,15 @@ endif
 ifdef CONFIG_RETPOLINE
   objtool_args += --retpoline
 endif
+ifdef CONFIG_KCOV
+  objtool_no_assert := 1
+endif
+ifdef CONFIG_KASAN
+  objtool_no_assert := 1
+endif
+ifdef objtool_no_assert
+  objtool_args += --no-static-assert
+endif
 
 
 ifdef CONFIG_MODVERSIONS
--- 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, no_assert;
 
 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('s', "no-static-assert", &no_assert, "Skip static branch validation"),
 	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, no_assert;
 
 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
@@ -1122,6 +1122,9 @@ static int assert_static_jumps(struct ob
 	struct rela *rela;
 	int i;
 
+	if (no_assert)
+		return 0;
+
 	sec = find_section_by_name(file->elf, ".discard.jump_assert");
 	if (!sec)
 		return 0;

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 22/24] x86/jump_label: Implement arch_static_assert()
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (20 preceding siblings ...)
  2018-01-23 15:26 ` [PATCH 21/24] objtool: Skip static assert when KCOV/KASAN Peter Zijlstra
@ 2018-01-23 15:26 ` Peter Zijlstra
  2018-01-23 15:26 ` [PATCH 23/24] x86: Force asm-goto Peter Zijlstra
                   ` (2 subsequent siblings)
  24 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:26 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra, Borislav Petkov

[-- Attachment #1: peterz-x86-asm-goto-assert.patch --]
[-- Type: text/plain, Size: 1422 bytes --]

Implement the static (branch) assertion. It simply emits the address
of the next instruction into a special section which objtool will read
and validate against either __jump_table or .altinstructions.

Use like:

	if (static_branch_likely(_key)) {
		arch_static_assert();
		/* do stuff */
	}

Or

	if (static_cpu_has(_feat)) {
		arch_static_assert();
		/* do stuff */
	}

Cc: Borislav Petkov <bp@alien8.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/jump_label.h |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -62,6 +62,29 @@ static __always_inline bool arch_static_
 	return true;
 }
 
+/*
+ * Annotation for objtool; asserts that the previous instruction is the
+ * jump_label patch site. Or rather, that the next instruction is a static
+ * branch target.
+ *
+ * Use like:
+ *
+ *	if (static_branch_likely(key)) {
+ *		arch_static_assert();
+ *		do_code();
+ *	}
+ *
+ * Also works with static_cpu_has().
+ */
+static __always_inline void arch_static_assert(void)
+{
+	asm volatile ("1:\n\t"
+		      ".pushsection .discard.jump_assert \n\t"
+		      _ASM_ALIGN  "\n\t"
+		      _ASM_PTR "1b \n\t"
+		      ".popsection \n\t");
+}
+
 #ifdef CONFIG_X86_64
 typedef u64 jump_label_t;
 #else

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 23/24] x86: Force asm-goto
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (21 preceding siblings ...)
  2018-01-23 15:26 ` [PATCH 22/24] x86/jump_label: Implement arch_static_assert() Peter Zijlstra
@ 2018-01-23 15:26 ` Peter Zijlstra
  2018-01-23 15:26 ` [PATCH 24/24] x86: Remove FAST_FEATURE_TESTS Peter Zijlstra
  2018-01-23 15:42 ` [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
  24 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:26 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

[-- Attachment #1: peterz-x86-force-asm-goto.patch --]
[-- Type: text/plain, Size: 1867 bytes --]

Now that we have objtool to validate the correctness of asm-goto
constructs we can start using it to guarantee the absence of dynamic
branches (and thus speculation).

A primary prerequisite for this is of course that the compiler
supports asm-goto. This effecively lifts the minimum GCC version to
build an x86 kernel to gcc-4.5.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 Makefile          |   13 +++++++------
 arch/x86/Makefile |    4 ++++
 2 files changed, 11 insertions(+), 6 deletions(-)

--- a/Makefile
+++ b/Makefile
@@ -513,6 +513,13 @@ ifneq ($(filter install,$(MAKECMDGOALS))
         endif
 endif
 
+# check for 'asm goto'
+ifeq ($(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC) $(KBUILD_CFLAGS)), y)
+  CC_HAVE_ASM_GOTO := 1
+  KBUILD_CFLAGS += -DCC_HAVE_ASM_GOTO
+  KBUILD_AFLAGS += -DCC_HAVE_ASM_GOTO
+endif
+
 ifeq ($(mixed-targets),1)
 # ===========================================================================
 # We're called with mixed targets (*config and build targets).
@@ -652,12 +659,6 @@ KBUILD_CFLAGS += $(call cc-ifversion, -l
 # Tell gcc to never replace conditional load with a non-conditional one
 KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
 
-# check for 'asm goto'
-ifeq ($(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC) $(KBUILD_CFLAGS)), y)
-	KBUILD_CFLAGS += -DCC_HAVE_ASM_GOTO
-	KBUILD_AFLAGS += -DCC_HAVE_ASM_GOTO
-endif
-
 include scripts/Makefile.gcc-plugins
 
 ifdef CONFIG_READABLE_ASM
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -186,6 +186,10 @@ ifdef CONFIG_FUNCTION_GRAPH_TRACER
   endif
 endif
 
+ifndef CC_HAVE_ASM_GOTO
+  $(error Compiler lacks asm-goto support.)
+endif
+
 #
 # Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent a
 # GCC bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226).  There's no way

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH 24/24] x86: Remove FAST_FEATURE_TESTS
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (22 preceding siblings ...)
  2018-01-23 15:26 ` [PATCH 23/24] x86: Force asm-goto Peter Zijlstra
@ 2018-01-23 15:26 ` Peter Zijlstra
  2018-01-23 15:42 ` [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
  24 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:26 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Peter Zijlstra

[-- Attachment #1: peterz-x86-remove-fast-feature.patch --]
[-- Type: text/plain, Size: 1861 bytes --]

Since we want to rely on static branches to avoid speculation, remove
any possible fallback code for static_cpu_has.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/Kconfig                  |   11 -----------
 arch/x86/include/asm/cpufeature.h |    8 --------
 2 files changed, 19 deletions(-)

--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -389,17 +389,6 @@ config X86_FEATURE_NAMES
 
 	  If in doubt, say Y.
 
-config X86_FAST_FEATURE_TESTS
-	bool "Fast CPU feature tests" if EMBEDDED
-	default y
-	---help---
-	  Some fast-paths in the kernel depend on the capabilities of the CPU.
-	  Say Y here for the kernel to patch in the appropriate code at runtime
-	  based on the capabilities of the CPU. The infrastructure for patching
-	  code at runtime takes up some additional space; space-constrained
-	  embedded systems may wish to say N here to produce smaller, slightly
-	  slower code.
-
 config X86_X2APIC
 	bool "Support x2apic"
 	depends on X86_LOCAL_APIC && X86_64 && (IRQ_REMAP || HYPERVISOR_GUEST)
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -137,7 +137,6 @@ extern void clear_cpu_cap(struct cpuinfo
 
 #define setup_force_cpu_bug(bit) setup_force_cpu_cap(bit)
 
-#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_X86_FAST_FEATURE_TESTS)
 /*
  * Static testing of CPU features.  Used the same as boot_cpu_has().
  * These will statically patch the target code for additional
@@ -196,13 +195,6 @@ static __always_inline __pure bool _stat
 		boot_cpu_has(bit) :				\
 		_static_cpu_has(bit)				\
 )
-#else
-/*
- * Fall back to dynamic for gcc versions which don't support asm goto. Should be
- * a minority now anyway.
- */
-#define static_cpu_has(bit)		boot_cpu_has(bit)
-#endif
 
 #define cpu_has_bug(c, bit)		cpu_has(c, (bit))
 #define set_cpu_bug(c, bit)		set_cpu_cap(c, (bit))

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 00/24] objtool: retpoline and asm-goto validation
  2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
                   ` (23 preceding siblings ...)
  2018-01-23 15:26 ` [PATCH 24/24] x86: Remove FAST_FEATURE_TESTS Peter Zijlstra
@ 2018-01-23 15:42 ` Peter Zijlstra
  2018-01-23 15:57   ` David Woodhouse
  24 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 15:42 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron

On Tue, Jan 23, 2018 at 04:25:39PM +0100, Peter Zijlstra wrote:
> This series adds two features to objtool:
> 
>  - retpoline validation; it verifies no indirect jumps/calls are left in
>    CONFIG_RETPOLINE=y builds.
> 
>    includes fixups for kvm and others
> 
>  - asm-goto validation; allows asserting that a block of code is not reachable
>    through a dynamic branch.
> 
> After this it forces an asm-goto capable compiler to build arch/x86 and removes
> the fallback code for static_cpu_has(). After which we can use static_cpu_has()
> to guarantee a lack of dynamic branches, which is a requested feature for
> various spectre related things.

x86_64-allmodconfig -KCOV -KASAN +patch gives:

drivers/infiniband/hw/bnxt_re/.tmp_qplib_fp.o: warning: objtool: bnxt_qplib_poll_cq()+0xf8: sibling call from callable instruction with modified stack frame
drivers/watchdog/.tmp_hpwdt.o: warning: objtool: .text+0x24: indirect call found in RETPOLINE build


---
Subject: jump_label: Add static assertion to every static_branch
From: Peter Zijlstra <peterz@infradead.org>
Date: Tue Jan 16 15:27:36 CET 2018

For testing only.. can't use in general because people do:

  static_branch() || more-cond

which confuses the living daylight out of objtool validation and
really isn't fixable.

DO NOT MERGE

Not-signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/jump_label.h |    1 +
 include/linux/cgroup.h            |    2 +-
 include/linux/jump_label.h        |   15 +++++++++++++--
 3 files changed, 15 insertions(+), 3 deletions(-)

--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -76,6 +76,7 @@ static __always_inline bool arch_static_
  *
  * Also works with static_cpu_has().
  */
+#define arch_static_assert arch_static_assert
 static __always_inline void arch_static_assert(void)
 {
 	asm volatile ("1:\n\t"
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -88,7 +88,7 @@ extern struct css_set init_css_set;
  * @ss: subsystem in question
  */
 #define cgroup_subsys_on_dfl(ss)						\
-	static_branch_likely(&ss ## _on_dfl_key)
+	__static_branch_likely(&ss ## _on_dfl_key)
 
 bool css_has_online_children(struct cgroup_subsys_state *css);
 struct cgroup_subsys_state *css_from_id(int id, struct cgroup_subsys *ss);
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -323,6 +323,10 @@ extern bool ____wrong_branch_error(void)
 
 #ifdef HAVE_JUMP_LABEL
 
+#ifndef arch_static_assert
+#define arch_static_assert (void)
+#endif
+
 /*
  * Combine the right initial value (type) with the right branch order
  * to generate the desired result.
@@ -379,7 +383,7 @@ extern bool ____wrong_branch_error(void)
  * See jump_label_type() / jump_label_init_type().
  */
 
-#define static_branch_likely(x)							\
+#define __static_branch_likely(x)						\
 ({										\
 	bool branch;								\
 	if (__builtin_types_compatible_p(typeof(*x), struct static_key_true))	\
@@ -391,7 +395,9 @@ extern bool ____wrong_branch_error(void)
 	likely(branch);								\
 })
 
-#define static_branch_unlikely(x)						\
+#define static_branch_likely(x) (__static_branch_likely(x) && (arch_static_assert(), true))
+
+#define __static_branch_unlikely(x)						\
 ({										\
 	bool branch;								\
 	if (__builtin_types_compatible_p(typeof(*x), struct static_key_true))	\
@@ -403,11 +409,16 @@ extern bool ____wrong_branch_error(void)
 	unlikely(branch);							\
 })
 
+#define static_branch_unlikely(x) (__static_branch_unlikely(x) && (arch_static_assert(), true))
+
 #else /* !HAVE_JUMP_LABEL */
 
 #define static_branch_likely(x)		likely(static_key_enabled(&(x)->key))
 #define static_branch_unlikely(x)	unlikely(static_key_enabled(&(x)->key))
 
+#define __static_branch_likely(x)		likely(static_key_enabled(&(x)->key))
+#define __static_branch_unlikely(x)	unlikely(static_key_enabled(&(x)->key))
+
 #endif /* HAVE_JUMP_LABEL */
 
 /*

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 00/24] objtool: retpoline and asm-goto validation
  2018-01-23 15:42 ` [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
@ 2018-01-23 15:57   ` David Woodhouse
  2018-01-23 16:03     ` Peter Zijlstra
  0 siblings, 1 reply; 71+ messages in thread
From: David Woodhouse @ 2018-01-23 15:57 UTC (permalink / raw)
  To: Peter Zijlstra, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron

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

On Tue, 2018-01-23 at 16:42 +0100, Peter Zijlstra wrote:
> drivers/watchdog/.tmp_hpwdt.o: warning: objtool: .text+0x24: indirect call found in RETPOLINE build

You're calling into BIOS there. Not a lot of point in protecting that
one. This is one of the cases where we were saying you needed to turn
on IBRS to have complete protection, even with retpoline.


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

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 00/24] objtool: retpoline and asm-goto validation
  2018-01-23 15:57   ` David Woodhouse
@ 2018-01-23 16:03     ` Peter Zijlstra
  0 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-23 16:03 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Thomas Gleixner, Josh Poimboeuf, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

On Tue, Jan 23, 2018 at 03:57:39PM +0000, David Woodhouse wrote:
> On Tue, 2018-01-23 at 16:42 +0100, Peter Zijlstra wrote:
> > drivers/watchdog/.tmp_hpwdt.o: warning: objtool: .text+0x24: indirect call found in RETPOLINE build
> 
> You're calling into BIOS there. Not a lot of point in protecting that
> one. This is one of the cases where we were saying you needed to turn
> on IBRS to have complete protection, even with retpoline.
> 

That code looked entirely dodgy, which is why I left it. I can easily
add the ANNOTATE_RETPOLINE_SAFE thing to shut it up, but given it is an
actual problem, maybe we should just keep it?

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 02/24] objtool: Add retpoline validation
  2018-01-23 15:25 ` [PATCH 02/24] objtool: Add retpoline validation Peter Zijlstra
@ 2018-01-23 18:21   ` Borislav Petkov
  2018-01-26  9:54   ` David Woodhouse
  1 sibling, 0 replies; 71+ messages in thread
From: Borislav Petkov @ 2018-01-23 18:21 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: David Woodhouse, Thomas Gleixner, Josh Poimboeuf, linux-kernel,
	Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron

On Tue, Jan 23, 2018 at 04:25:41PM +0100, Peter Zijlstra wrote:
> 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.
> 
> 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         |   87 ++++++++++++++++++++++++++++++++++++++++--
>  tools/objtool/check.h         |    1 
>  5 files changed, 92 insertions(+), 5 deletions(-)

Yap, very noisy:

$ grep indirect build.log | wc -l
12904

Let's see how many will remain at the end of the patchset. :)

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 06/24] x86,kvm: Fix indirect calls in emulator
  2018-01-23 15:25 ` [PATCH 06/24] x86,kvm: Fix indirect calls in emulator Peter Zijlstra
@ 2018-01-23 20:28   ` Borislav Petkov
  2018-01-23 20:48     ` David Woodhouse
  0 siblings, 1 reply; 71+ messages in thread
From: Borislav Petkov @ 2018-01-23 20:28 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: David Woodhouse, Thomas Gleixner, Josh Poimboeuf, linux-kernel,
	Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron

On Tue, Jan 23, 2018 at 04:25:45PM +0100, Peter Zijlstra wrote:
> Replace the indirect calls with CALL_NOSPEC.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/x86/kvm/emulate.c |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -25,6 +25,7 @@
>  #include <asm/kvm_emulate.h>
>  #include <linux/stringify.h>
>  #include <asm/debugreg.h>
> +#include <asm/nospec-branch.h>
>  
>  #include "x86.h"
>  #include "tss.h"
> @@ -1021,8 +1022,8 @@ static __always_inline u8 test_cc(unsign
>  	void (*fop)(void) = (void *)em_setcc + 4 * (condition & 0xf);
>  
>  	flags = (flags & EFLAGS_MASK) | X86_EFLAGS_IF;
> -	asm("push %[flags]; popf; call *%[fastop]"
> -	    : "=a"(rc) : [fastop]"r"(fop), [flags]"r"(flags));
> +	asm("push %[flags]; popf; " CALL_NOSPEC
> +	    : "=a"(rc) : [thunk_target]"r"(fop), [flags]"r"(flags));

Oh, "thunk_target" is magical.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 06/24] x86,kvm: Fix indirect calls in emulator
  2018-01-23 20:28   ` Borislav Petkov
@ 2018-01-23 20:48     ` David Woodhouse
  2018-01-24 10:35       ` Peter Zijlstra
  0 siblings, 1 reply; 71+ messages in thread
From: David Woodhouse @ 2018-01-23 20:48 UTC (permalink / raw)
  To: Borislav Petkov, Peter Zijlstra
  Cc: Thomas Gleixner, Josh Poimboeuf, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

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

On Tue, 2018-01-23 at 21:28 +0100, Borislav Petkov wrote:
> 
> >       flags = (flags & EFLAGS_MASK) | X86_EFLAGS_IF;
> > -     asm("push %[flags]; popf; call *%[fastop]"
> > -         : "=a"(rc) : [fastop]"r"(fop), [flags]"r"(flags));
> > +     asm("push %[flags]; popf; " CALL_NOSPEC
> > +         : "=a"(rc) : [thunk_target]"r"(fop), [flags]"r"(flags));
> 
> Oh, "thunk_target" is magical.

You can use THUNK_TARGET(fop), which will be "rm" on 32-bit and avoids
register starvation in some cases (I don't think the hyperv calls
worked until I did that).

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

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 06/24] x86,kvm: Fix indirect calls in emulator
  2018-01-23 20:48     ` David Woodhouse
@ 2018-01-24 10:35       ` Peter Zijlstra
  2018-01-24 10:43         ` Paolo Bonzini
  0 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-24 10:35 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Borislav Petkov, Thomas Gleixner, Josh Poimboeuf, linux-kernel,
	Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron

On Tue, Jan 23, 2018 at 08:48:13PM +0000, David Woodhouse wrote:
> On Tue, 2018-01-23 at 21:28 +0100, Borislav Petkov wrote:
> > 
> > >       flags = (flags & EFLAGS_MASK) | X86_EFLAGS_IF;
> > > -     asm("push %[flags]; popf; call *%[fastop]"
> > > -         : "=a"(rc) : [fastop]"r"(fop), [flags]"r"(flags));
> > > +     asm("push %[flags]; popf; " CALL_NOSPEC
> > > +         : "=a"(rc) : [thunk_target]"r"(fop), [flags]"r"(flags));
> > 
> > Oh, "thunk_target" is magical.
> 
> You can use THUNK_TARGET(fop), which will be "rm" on 32-bit and avoids
> register starvation in some cases (I don't think the hyperv calls
> worked until I did that).

The reason I didn't use THUNK_TARGET() was exactly because it used "rm"
and the current code did "r" only. I'm happy to change if people can
agree on something ;-)

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 06/24] x86,kvm: Fix indirect calls in emulator
  2018-01-24 10:35       ` Peter Zijlstra
@ 2018-01-24 10:43         ` Paolo Bonzini
  2018-01-25  9:34           ` Peter Zijlstra
  0 siblings, 1 reply; 71+ messages in thread
From: Paolo Bonzini @ 2018-01-24 10:43 UTC (permalink / raw)
  To: Peter Zijlstra, David Woodhouse
  Cc: Borislav Petkov, Thomas Gleixner, Josh Poimboeuf, linux-kernel,
	Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Jun Nakajima, Asit Mallick,
	Jason Baron

On 24/01/2018 11:35, Peter Zijlstra wrote:
> On Tue, Jan 23, 2018 at 08:48:13PM +0000, David Woodhouse wrote:
>> On Tue, 2018-01-23 at 21:28 +0100, Borislav Petkov wrote:
>>>
>>>>        flags = (flags & EFLAGS_MASK) | X86_EFLAGS_IF;
>>>> -     asm("push %[flags]; popf; call *%[fastop]"
>>>> -         : "=a"(rc) : [fastop]"r"(fop), [flags]"r"(flags));
>>>> +     asm("push %[flags]; popf; " CALL_NOSPEC
>>>> +         : "=a"(rc) : [thunk_target]"r"(fop), [flags]"r"(flags));
>>>
>>> Oh, "thunk_target" is magical.
>>
>> You can use THUNK_TARGET(fop), which will be "rm" on 32-bit and avoids
>> register starvation in some cases (I don't think the hyperv calls
>> worked until I did that).
> 
> The reason I didn't use THUNK_TARGET() was exactly because it used "rm"
> and the current code did "r" only. I'm happy to change if people can
> agree on something ;-)

In practice, "fastop" is going to be in a register because of how it's
computed, but "rm" is okay.

Thanks,

Paolo

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 09/24] jump_label: Add branch hints to static_branch_{un,}likely()
  2018-01-23 15:25 ` [PATCH 09/24] jump_label: Add branch hints to static_branch_{un,}likely() Peter Zijlstra
@ 2018-01-24 18:46   ` Borislav Petkov
  0 siblings, 0 replies; 71+ messages in thread
From: Borislav Petkov @ 2018-01-24 18:46 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: David Woodhouse, Thomas Gleixner, Josh Poimboeuf, linux-kernel,
	Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron

On Tue, Jan 23, 2018 at 04:25:48PM +0100, Peter Zijlstra wrote:
> For some reason these were missing, I've not observed this patch
> making a difference in the few code locations I checked, but this
> makes sense.
> 
> Cc: Jason Baron <jbaron@akamai.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  include/linux/jump_label.h |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- a/include/linux/jump_label.h
> +++ b/include/linux/jump_label.h
> @@ -388,7 +388,7 @@ extern bool ____wrong_branch_error(void)
>  		branch = !arch_static_branch_jump(&(x)->key, true);		\
>  	else									\
>  		branch = ____wrong_branch_error();				\
> -	branch;									\
> +	likely(branch);								\
>  })
>  
>  #define static_branch_unlikely(x)						\
> @@ -400,7 +400,7 @@ extern bool ____wrong_branch_error(void)
>  		branch = arch_static_branch(&(x)->key, false);			\
>  	else									\
>  		branch = ____wrong_branch_error();				\
> -	branch;									\
> +	unlikely(branch);							\
>  })

LOL, it is practically in the name already. :-)

Reviewed-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 06/24] x86,kvm: Fix indirect calls in emulator
  2018-01-24 10:43         ` Paolo Bonzini
@ 2018-01-25  9:34           ` Peter Zijlstra
  2018-01-25  9:49             ` David Woodhouse
  2018-01-26 10:57             ` Paolo Bonzini
  0 siblings, 2 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-25  9:34 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: David Woodhouse, Borislav Petkov, Thomas Gleixner,
	Josh Poimboeuf, linux-kernel, Dave Hansen, Ashok Raj, Tim Chen,
	Andy Lutomirski, Linus Torvalds, Greg KH, Andrea Arcangeli,
	Andi Kleen, Arjan Van De Ven, Dan Williams, Jun Nakajima,
	Asit Mallick, Jason Baron

On Wed, Jan 24, 2018 at 11:43:05AM +0100, Paolo Bonzini wrote:
> On 24/01/2018 11:35, Peter Zijlstra wrote:
> > On Tue, Jan 23, 2018 at 08:48:13PM +0000, David Woodhouse wrote:
> >> On Tue, 2018-01-23 at 21:28 +0100, Borislav Petkov wrote:
> >>>
> >>>>        flags = (flags & EFLAGS_MASK) | X86_EFLAGS_IF;
> >>>> -     asm("push %[flags]; popf; call *%[fastop]"
> >>>> -         : "=a"(rc) : [fastop]"r"(fop), [flags]"r"(flags));
> >>>> +     asm("push %[flags]; popf; " CALL_NOSPEC
> >>>> +         : "=a"(rc) : [thunk_target]"r"(fop), [flags]"r"(flags));
> >>>
> >>> Oh, "thunk_target" is magical.
> >>
> >> You can use THUNK_TARGET(fop), which will be "rm" on 32-bit and avoids
> >> register starvation in some cases (I don't think the hyperv calls
> >> worked until I did that).
> > 
> > The reason I didn't use THUNK_TARGET() was exactly because it used "rm"
> > and the current code did "r" only. I'm happy to change if people can
> > agree on something ;-)
> 
> In practice, "fastop" is going to be in a register because of how it's
> computed, but "rm" is okay.

OK, so the other occurence in that file uses "+S", which is the SI
register. That cannot use THUNK_TARGET(), right?

So do you want one THUNK_TARGET and one open coded, or keep the patch as
is (both open coded) ?

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 07/24] x86,vmx: Fix indirect call
  2018-01-23 15:25 ` [PATCH 07/24] x86,vmx: Fix indirect call Peter Zijlstra
@ 2018-01-25  9:36   ` Peter Zijlstra
  0 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-25  9:36 UTC (permalink / raw)
  To: David Woodhouse, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron

Subject: x86,vmx: Fix indirect call
From: Peter Zijlstra <peterz@infradead.org>
Date: Tue Jan 23 14:10:19 CET 2018

Replace indirect call with CALL_NOSPEC.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 - now with THUNK_TARGET()

 arch/x86/kvm/vmx.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -9129,14 +9129,14 @@ static void vmx_handle_external_intr(str
 #endif
 			"pushf\n\t"
 			__ASM_SIZE(push) " $%c[cs]\n\t"
-			"call *%[entry]\n\t"
+			CALL_NOSPEC
 			:
 #ifdef CONFIG_X86_64
 			[sp]"=&r"(tmp),
 #endif
 			ASM_CALL_CONSTRAINT
 			:
-			[entry]"r"(entry),
+			THUNK_TARGET(entry),
 			[ss]"i"(__KERNEL_DS),
 			[cs]"i"(__KERNEL_CS)
 			);

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 06/24] x86,kvm: Fix indirect calls in emulator
  2018-01-25  9:34           ` Peter Zijlstra
@ 2018-01-25  9:49             ` David Woodhouse
  2018-01-26 10:57             ` Paolo Bonzini
  1 sibling, 0 replies; 71+ messages in thread
From: David Woodhouse @ 2018-01-25  9:49 UTC (permalink / raw)
  To: Peter Zijlstra, Paolo Bonzini
  Cc: Borislav Petkov, Thomas Gleixner, Josh Poimboeuf, linux-kernel,
	Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Jun Nakajima, Asit Mallick,
	Jason Baron

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

On Thu, 2018-01-25 at 10:34 +0100, Peter Zijlstra wrote:
> On Wed, Jan 24, 2018 at 11:43:05AM +0100, Paolo Bonzini wrote:
> > 
> > On 24/01/2018 11:35, Peter Zijlstra wrote:
> > > 
> > > On Tue, Jan 23, 2018 at 08:48:13PM +0000, David Woodhouse wrote:
> > > > 
> > > > On Tue, 2018-01-23 at 21:28 +0100, Borislav Petkov wrote:
> > > > > 
> > > > > 
> > > > > > 
> > > > > >        flags = (flags & EFLAGS_MASK) | X86_EFLAGS_IF;
> > > > > > -     asm("push %[flags]; popf; call *%[fastop]"
> > > > > > -         : "=a"(rc) : [fastop]"r"(fop), [flags]"r"(flags));
> > > > > > +     asm("push %[flags]; popf; " CALL_NOSPEC
> > > > > > +         : "=a"(rc) : [thunk_target]"r"(fop), [flags]"r"(flags));
> > > > > Oh, "thunk_target" is magical.
> > > > You can use THUNK_TARGET(fop), which will be "rm" on 32-bit and avoids
> > > > register starvation in some cases (I don't think the hyperv calls
> > > > worked until I did that).
> > > The reason I didn't use THUNK_TARGET() was exactly because it used "rm"
> > > and the current code did "r" only. I'm happy to change if people can
> > > agree on something ;-)
> > In practice, "fastop" is going to be in a register because of how it's
> > computed, but "rm" is okay.
> OK, so the other occurence in that file uses "+S", which is the SI
> register. That cannot use THUNK_TARGET(), right?
> 
> So do you want one THUNK_TARGET and one open coded, or keep the patch as
> is (both open coded) ?

As long as it builds for i386, you might as well keep them both open-
coded. The "rm" was there mostly for the hyperv call, which ran out of
registers completely when it was just "r".

This patch *really* wants to be going to Linus urgently as a retpoline
fix, and not buried as patch 6/24 though, right? Likewise 7/24? 

Both of them,

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

... unless you want me to send them on, but it makes most sense for
Thomas to round them I suspect?



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

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 03/24] x86/paravirt: Annotate indirect calls
  2018-01-23 15:25 ` [PATCH 03/24] x86/paravirt: Annotate indirect calls Peter Zijlstra
@ 2018-01-25 10:02   ` David Woodhouse
  2018-01-25 10:22     ` Peter Zijlstra
  2018-01-29 17:58   ` Josh Poimboeuf
  2018-01-29 18:38   ` Josh Poimboeuf
  2 siblings, 1 reply; 71+ messages in thread
From: David Woodhouse @ 2018-01-25 10:02 UTC (permalink / raw)
  To: Peter Zijlstra, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron

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

On Tue, 2018-01-23 at 16:25 +0100, Peter Zijlstra wrote:
> 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.

I've seen this asserted repeatedly but I've never truly convinced
myself of it. Is this absolutely unconditionally true in every case,
even when we're running as a guest and there are *actual* calls to be
made? We turn them into direct calls, never leave them indirect?

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

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 03/24] x86/paravirt: Annotate indirect calls
  2018-01-25 10:02   ` David Woodhouse
@ 2018-01-25 10:22     ` Peter Zijlstra
  2018-01-25 10:26       ` Juergen Gross
  0 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-25 10:22 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Thomas Gleixner, Josh Poimboeuf, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

On Thu, Jan 25, 2018 at 10:02:05AM +0000, David Woodhouse wrote:
> On Tue, 2018-01-23 at 16:25 +0100, Peter Zijlstra wrote:
> > 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.
> 
> I've seen this asserted repeatedly but I've never truly convinced
> myself of it. Is this absolutely unconditionally true in every case,
> even when we're running as a guest and there are *actual* calls to be
> made? We turn them into direct calls, never leave them indirect?

That is my understanding; and when I worked on the paravirt spinlock
code and disassembled live guest code this seemed to have happend.

But let me go read the paravirt code again to make a stronger argument
in favour.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 03/24] x86/paravirt: Annotate indirect calls
  2018-01-25 10:22     ` Peter Zijlstra
@ 2018-01-25 10:26       ` Juergen Gross
  2018-01-25 10:52         ` David Woodhouse
  0 siblings, 1 reply; 71+ messages in thread
From: Juergen Gross @ 2018-01-25 10:26 UTC (permalink / raw)
  To: Peter Zijlstra, David Woodhouse
  Cc: Thomas Gleixner, Josh Poimboeuf, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

On 25/01/18 11:22, Peter Zijlstra wrote:
> On Thu, Jan 25, 2018 at 10:02:05AM +0000, David Woodhouse wrote:
>> On Tue, 2018-01-23 at 16:25 +0100, Peter Zijlstra wrote:
>>> 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.
>>
>> I've seen this asserted repeatedly but I've never truly convinced
>> myself of it. Is this absolutely unconditionally true in every case,
>> even when we're running as a guest and there are *actual* calls to be
>> made? We turn them into direct calls, never leave them indirect?
> 
> That is my understanding; and when I worked on the paravirt spinlock
> code and disassembled live guest code this seemed to have happend.
> 
> But let me go read the paravirt code again to make a stronger argument
> in favour.
> 

paravirt_patch_default() is the function you want to look at: it either
replaces the indirect call by some cutom code (which is never an
indirect call) or by a call of the target function.


Juergen

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 03/24] x86/paravirt: Annotate indirect calls
  2018-01-25 10:26       ` Juergen Gross
@ 2018-01-25 10:52         ` David Woodhouse
  2018-01-25 11:35           ` Peter Zijlstra
  0 siblings, 1 reply; 71+ messages in thread
From: David Woodhouse @ 2018-01-25 10:52 UTC (permalink / raw)
  To: Juergen Gross, Peter Zijlstra
  Cc: Thomas Gleixner, Josh Poimboeuf, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

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

On Thu, 2018-01-25 at 11:26 +0100, Juergen Gross wrote:
> On 25/01/18 11:22, Peter Zijlstra wrote:
> > 
> > On Thu, Jan 25, 2018 at 10:02:05AM +0000, David Woodhouse wrote:
> > > 
> > > On Tue, 2018-01-23 at 16:25 +0100, Peter Zijlstra wrote:
> > > > 
> > > > 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.
> > > I've seen this asserted repeatedly but I've never truly convinced
> > > myself of it. Is this absolutely unconditionally true in every case,
> > > even when we're running as a guest and there are *actual* calls to be
> > > made? We turn them into direct calls, never leave them indirect?
> > That is my understanding; and when I worked on the paravirt spinlock
> > code and disassembled live guest code this seemed to have happend.
> > 
> > But let me go read the paravirt code again to make a stronger argument
> > in favour.
> > 
> paravirt_patch_default() is the function you want to look at: it either
> replaces the indirect call by some cutom code (which is never an
> indirect call) or by a call of the target function.

OK, my brain hurts a bit but I'm happy now. Thank you.

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

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 03/24] x86/paravirt: Annotate indirect calls
  2018-01-25 10:52         ` David Woodhouse
@ 2018-01-25 11:35           ` Peter Zijlstra
  2018-01-26  9:57             ` David Woodhouse
  0 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-25 11:35 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Juergen Gross, Thomas Gleixner, Josh Poimboeuf, linux-kernel,
	Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron

On Thu, Jan 25, 2018 at 10:52:53AM +0000, David Woodhouse wrote:
> OK, my brain hurts a bit but I'm happy now. Thank you.

OK, I've updated the Changelog thusly. Is this satisfactory?

---
Subject: x86/paravirt: Annotate indirect calls
From: Peter Zijlstra <peterz@infradead.org>
Date: Wed Jan 17 16:58:11 CET 2018

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.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 12/24] x86: Update _static_cpu_has to use all named variables
  2018-01-23 15:25 ` [PATCH 12/24] x86: Update _static_cpu_has to use all named variables Peter Zijlstra
@ 2018-01-25 19:31   ` Borislav Petkov
  0 siblings, 0 replies; 71+ messages in thread
From: Borislav Petkov @ 2018-01-25 19:31 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: David Woodhouse, Thomas Gleixner, Josh Poimboeuf, linux-kernel,
	Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron

On Tue, Jan 23, 2018 at 04:25:51PM +0100, Peter Zijlstra wrote:
> 
> Requested-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/x86/include/asm/cpufeature.h |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -153,7 +153,7 @@ static __always_inline __pure bool _stat
>  		 ".section .altinstructions,\"a\"\n"
>  		 " .long 1b - .\n"		/* src offset */
>  		 " .long 4f - .\n"		/* repl offset */
> -		 " .word %P1\n"			/* always replace */
> +		 " .word %P[always]\n"		/* always replace */
>  		 " .byte 3b - 1b\n"		/* src len */
>  		 " .byte 5f - 4f\n"		/* repl len */
>  		 " .byte 3b - 2b\n"		/* pad len */
> @@ -165,7 +165,7 @@ static __always_inline __pure bool _stat
>  		 ".section .altinstructions,\"a\"\n"
>  		 " .long 1b - .\n"		/* src offset */
>  		 " .long 0\n"			/* no replacement */
> -		 " .word %P0\n"			/* feature bit */
> +		 " .word %P[feature]\n"		/* feature bit */
>  		 " .byte 3b - 1b\n"		/* src len */
>  		 " .byte 0\n"			/* repl len */
>  		 " .byte 0\n"			/* pad len */
> @@ -176,8 +176,9 @@ static __always_inline __pure bool _stat
>  		 " jnz %l[t_yes]\n"
>  		 " jmp %l[t_no]\n"
>  		 ".previous\n"
> -		 : : "i" (bit), "i" (X86_FEATURE_ALWAYS),
> -		     [bitnum] "i" (1 << (bit & 7)),
> +		 : : [feature]  "i" (bit),
> +		     [always]   "i" (X86_FEATURE_ALWAYS),
> +		     [bitnum]   "i" (1 << (bit & 7)),
>  		     [cap_byte] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3])
>  		 : : t_yes, t_no);
>  t_yes:

Reviewed-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 02/24] objtool: Add retpoline validation
  2018-01-23 15:25 ` [PATCH 02/24] objtool: Add retpoline validation Peter Zijlstra
  2018-01-23 18:21   ` Borislav Petkov
@ 2018-01-26  9:54   ` David Woodhouse
  1 sibling, 0 replies; 71+ messages in thread
From: David Woodhouse @ 2018-01-26  9:54 UTC (permalink / raw)
  To: Peter Zijlstra, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron

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

On Tue, 2018-01-23 at 16:25 +0100, Peter Zijlstra wrote:
> 
> +               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;


...

        case 0xff:
                if (modrm_reg == 2 || modrm_reg == 3)

                        *type = INSN_CALL_DYNAMIC;

                else if (modrm_reg == 4)

                        *type = INSN_JUMP_DYNAMIC;

                else if (modrm_reg == 5)

                        /* jmpf */
                        *type = INSN_CONTEXT_SWITCH;


I *think* your check includes far calls (FF/3), although not far jumps?
It shouldn't, because I don't believe far calls are subject to the same
speculation?

Other than that, which you can probably ignore if you didn't have to
explicitly annotate [m]any safe far calls anyway,

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

Thanks for doing this.

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

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 03/24] x86/paravirt: Annotate indirect calls
  2018-01-25 11:35           ` Peter Zijlstra
@ 2018-01-26  9:57             ` David Woodhouse
  0 siblings, 0 replies; 71+ messages in thread
From: David Woodhouse @ 2018-01-26  9:57 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Juergen Gross, Thomas Gleixner, Josh Poimboeuf, linux-kernel,
	Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron

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

On Thu, 2018-01-25 at 12:35 +0100, Peter Zijlstra wrote:
> On Thu, Jan 25, 2018 at 10:52:53AM +0000, David Woodhouse wrote:
> > 
> > OK, my brain hurts a bit but I'm happy now. Thank you.
> OK, I've updated the Changelog thusly. Is this satisfactory?
> 
> ---
> Subject: x86/paravirt: Annotate indirect calls
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Wed Jan 17 16:58:11 CET 2018
> 
> 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.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

I love you, Peter.

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

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 04/24] x86,nospec: Annotate indirect calls/jumps
  2018-01-23 15:25 ` [PATCH 04/24] x86,nospec: Annotate indirect calls/jumps Peter Zijlstra
@ 2018-01-26 10:19   ` David Woodhouse
  2018-01-29 17:44     ` Peter Zijlstra
  0 siblings, 1 reply; 71+ messages in thread
From: David Woodhouse @ 2018-01-26 10:19 UTC (permalink / raw)
  To: Peter Zijlstra, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron

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

On Tue, 2018-01-23 at 16:25 +0100, Peter Zijlstra wrote:
> Annotate the indirect calls/jumps in the CALL_NOSPEC/JUMP_NOSPEC
> alternatives.
> 
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

However...


>  /*
> + * 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

Didn't I just see one of those in patch 3? So this makes two...



> @@ -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)

... three.

Now, I did briefly toy with the idea of using a .macro from both
__ASSEMBLY__ and inline asm, making the latter work by means of 
asm(".include \"asm/nospec-branch.h\");

In the end I just ended up with the __FILL_RETURN_BUFFER CPP macro
which is used from both by other tricks.

Can we look at doing something like that, please?


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

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 05/24] x86: Annotate indirect jump in head_64.S
  2018-01-23 15:25 ` [PATCH 05/24] x86: Annotate indirect jump in head_64.S Peter Zijlstra
@ 2018-01-26 10:24   ` David Woodhouse
  0 siblings, 0 replies; 71+ messages in thread
From: David Woodhouse @ 2018-01-26 10:24 UTC (permalink / raw)
  To: Peter Zijlstra, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron

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

On Tue, 2018-01-23 at 16:25 +0100, Peter Zijlstra wrote:
> The objtool retpoline validation found this indirect jump. Seeing how
> its on CPU bringup before we run userspace it should be safe, annotate

http://angryflower.com/itsits.gif

> it.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>


> ---
>  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 
>  #include "../entry/calling.h"
>  #include 
> +#include 
>  
>  #ifdef CONFIG_PARAVIRT
>  #include 
> @@ -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
> 
> 

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

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 08/24] x86,sme: Annotate indirect call
  2018-01-23 15:25 ` [PATCH 08/24] x86,sme: Annotate " Peter Zijlstra
@ 2018-01-26 10:37   ` David Woodhouse
  2018-01-29 17:49     ` Peter Zijlstra
  2018-01-31  9:29     ` Peter Zijlstra
  0 siblings, 2 replies; 71+ messages in thread
From: David Woodhouse @ 2018-01-26 10:37 UTC (permalink / raw)
  To: Peter Zijlstra, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Tom Lendacky, Borislav Petkov

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

On Tue, 2018-01-23 at 16:25 +0100, Peter Zijlstra wrote:
> This is boot code, we run this _way_ before userspace comes along to
> poison our branch predictor.

Hm, objtool knows about sections, doesn't it? Why it is whining about
indirect jumps in inittext anyway?

In fact, why are we even *doing* retpolines in inittext? Not that we
are; since we flipped the ALTERNATIVE logic around, at that point we
still have the 'oldinstr' which is a bare jmp anyway. We might as well
do this:

--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -37,10 +37,15 @@
  * as gcc otherwise puts the data into the bss section and not into the init
  * section.
  */
+#if defined(RETPOLINE) && !defined(MODULE)
+#define __noretpoline __attribute__((indirect_branch("keep")))
+#else
+#define __noretpoline
+#endif
 
 /* These are for everybody (although not all archs will actually
    discard it in modules) */
-#define __init         __section(.init.text) __cold __inittrace __latent_entropy
+#define __init         __section(.init.text) __cold __inittrace __latent_entropy __noretpoline
 #define __initdata     __section(.init.data)
 #define __initconst    __section(.init.rodata)
 #define __exitdata     __section(.exit.data)


I had that once and dropped it because of concerns about VM guests
being "vulnerable" at boot time. But really, do they even have any
interesting data to purloin at that point? And shouldn't the hypervisor
be protecting them with STIBP if they have nasty HT siblings? 

(And if hypervisors do start doing that, it might be nice for a guest
to have a way to say "you can stop now; I'm safe")

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

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 13/24] objtool: Implement base jump_assert support
  2018-01-23 15:25 ` [PATCH 13/24] objtool: Implement base jump_assert support Peter Zijlstra
@ 2018-01-26 10:45   ` David Woodhouse
  0 siblings, 0 replies; 71+ messages in thread
From: David Woodhouse @ 2018-01-26 10:45 UTC (permalink / raw)
  To: Peter Zijlstra, Thomas Gleixner, Josh Poimboeuf
  Cc: linux-kernel, Dave Hansen, Ashok Raj, Tim Chen, Andy Lutomirski,
	Linus Torvalds, Greg KH, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, Dan Williams, Paolo Bonzini, Jun Nakajima,
	Asit Mallick, Jason Baron, Borislav Petkov

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

On Tue, 2018-01-23 at 16:25 +0100, Peter Zijlstra wrote:
> Implement a jump_label assertion that asserts that the code location
> is indeed only reachable through a static_branch. Because if GCC is
> absolutely retaded it could generate code like:
> 
>         xor rax,rax
>         NOP/JMP 1f
>         mov $1, rax
> 1:
>         test rax,rax
>         jz 2f
>         
> 2:
> 
> instead of the sensible:
> 
>         NOP/JMP 1f
>         
> 1:
> 
> This implements objtool infrastructure for ensuring the code ends up
> sane, since we'll rely on that for correctness and security.
> 
> We tag the instructions after the static branch with static_jump_dest=true;
> that is the instruction after the NOP and the instruction at the
> JMP+disp site.
> 
> Then, when we read the .discard.jump_assert section, we assert that
> each entry points to an instruction that has static_jump_dest set.
> 
> With this we can assert that the code emitted for the if statement
> ends up at the static jump location and nothing untowards happened.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Thank you for pandering to my paranoia. I suspect that misspelling the
word 'retarded' isn't going to be sufficient to stop people from
objecting to the use of that word, but other than that,

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

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

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 06/24] x86,kvm: Fix indirect calls in emulator
  2018-01-25  9:34           ` Peter Zijlstra
  2018-01-25  9:49             ` David Woodhouse
@ 2018-01-26 10:57             ` Paolo Bonzini
  1 sibling, 0 replies; 71+ messages in thread
From: Paolo Bonzini @ 2018-01-26 10:57 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: David Woodhouse, Borislav Petkov, Thomas Gleixner,
	Josh Poimboeuf, linux-kernel, Dave Hansen, Ashok Raj, Tim Chen,
	Andy Lutomirski, Linus Torvalds, Greg KH, Andrea Arcangeli,
	Andi Kleen, Arjan Van De Ven, Dan Williams, Jun Nakajima,
	Asit Mallick, Jason Baron

On 25/01/2018 10:34, Peter Zijlstra wrote:
> On Wed, Jan 24, 2018 at 11:43:05AM +0100, Paolo Bonzini wrote:
>> On 24/01/2018 11:35, Peter Zijlstra wrote:
>>> On Tue, Jan 23, 2018 at 08:48:13PM +0000, David Woodhouse wrote:
>>>> On Tue, 2018-01-23 at 21:28 +0100, Borislav Petkov wrote:
>>>>>
>>>>>>        flags = (flags & EFLAGS_MASK) | X86_EFLAGS_IF;
>>>>>> -     asm("push %[flags]; popf; call *%[fastop]"
>>>>>> -         : "=a"(rc) : [fastop]"r"(fop), [flags]"r"(flags));
>>>>>> +     asm("push %[flags]; popf; " CALL_NOSPEC
>>>>>> +         : "=a"(rc) : [thunk_target]"r"(fop), [flags]"r"(flags));
>>>>>
>>>>> Oh, "thunk_target" is magical.
>>>>
>>>> You can use THUNK_TARGET(fop), which will be "rm" on 32-bit and avoids
>>>> register starvation in some cases (I don't think the hyperv calls
>>>> worked until I did that).
>>>
>>> The reason I didn't use THUNK_TARGET() was exactly because it used "rm"
>>> and the current code did "r" only. I'm happy to change if people can
>>> agree on something ;-)
>>
>> In practice, "fastop" is going to be in a register because of how it's
>> computed, but "rm" is okay.
> 
> OK, so the other occurence in that file uses "+S", which is the SI
> register. That cannot use THUNK_TARGET(), right?

Nope, it reads the output in %esi too.

> So do you want one THUNK_TARGET and one open coded, or keep the patch as
> is (both open coded) ?

Open coded is okay.

Paolo

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 04/24] x86,nospec: Annotate indirect calls/jumps
  2018-01-26 10:19   ` David Woodhouse
@ 2018-01-29 17:44     ` Peter Zijlstra
  0 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-29 17:44 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Thomas Gleixner, Josh Poimboeuf, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

On Fri, Jan 26, 2018 at 10:19:47AM +0000, David Woodhouse wrote:
> On Tue, 2018-01-23 at 16:25 +0100, Peter Zijlstra wrote:
> > Annotate the indirect calls/jumps in the CALL_NOSPEC/JUMP_NOSPEC
> > alternatives.
> > 
> > 
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> 
> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
> 
> However...
> 
> 
> >  /*
> > + * 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
> 
> Didn't I just see one of those in patch 3? So this makes two...
> 
> 
> 
> > @@ -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)
> 
> ... three.
> 
> Now, I did briefly toy with the idea of using a .macro from both
> __ASSEMBLY__ and inline asm, making the latter work by means of 
> asm(".include \"asm/nospec-branch.h\");
> 
> In the end I just ended up with the __FILL_RETURN_BUFFER CPP macro
> which is used from both by other tricks.
> 
> Can we look at doing something like that, please?


I'll try. The paravirt one might be tricky, I always end in header-hell
with that thing.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 08/24] x86,sme: Annotate indirect call
  2018-01-26 10:37   ` David Woodhouse
@ 2018-01-29 17:49     ` Peter Zijlstra
  2018-01-29 17:50       ` Peter Zijlstra
  2018-01-31  9:29     ` Peter Zijlstra
  1 sibling, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-29 17:49 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Thomas Gleixner, Josh Poimboeuf, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron,
	Tom Lendacky, Borislav Petkov

On Fri, Jan 26, 2018 at 10:37:30AM +0000, David Woodhouse wrote:
> On Tue, 2018-01-23 at 16:25 +0100, Peter Zijlstra wrote:
> > This is boot code, we run this _way_ before userspace comes along to
> > poison our branch predictor.
> 
> Hm, objtool knows about sections, doesn't it? Why it is whining about
> indirect jumps in inittext anyway?
> 
> In fact, why are we even *doing* retpolines in inittext? Not that we
> are; since we flipped the ALTERNATIVE logic around, at that point we
> still have the 'oldinstr' which is a bare jmp anyway. We might as well
> do this:
> 
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -37,10 +37,15 @@
>   * as gcc otherwise puts the data into the bss section and not into the init
>   * section.
>   */
> +#if defined(RETPOLINE) && !defined(MODULE)
> +#define __noretpoline __attribute__((indirect_branch("keep")))
> +#else
> +#define __noretpoline
> +#endif
>  
>  /* These are for everybody (although not all archs will actually
>     discard it in modules) */
> -#define __init         __section(.init.text) __cold __inittrace __latent_entropy
> +#define __init         __section(.init.text) __cold __inittrace __latent_entropy __noretpoline

We run module __init text concurrently with userspace.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 08/24] x86,sme: Annotate indirect call
  2018-01-29 17:49     ` Peter Zijlstra
@ 2018-01-29 17:50       ` Peter Zijlstra
  0 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-29 17:50 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Thomas Gleixner, Josh Poimboeuf, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron,
	Tom Lendacky, Borislav Petkov

On Mon, Jan 29, 2018 at 06:49:52PM +0100, Peter Zijlstra wrote:
> On Fri, Jan 26, 2018 at 10:37:30AM +0000, David Woodhouse wrote:
> > On Tue, 2018-01-23 at 16:25 +0100, Peter Zijlstra wrote:
> > > This is boot code, we run this _way_ before userspace comes along to
> > > poison our branch predictor.
> > 
> > Hm, objtool knows about sections, doesn't it? Why it is whining about
> > indirect jumps in inittext anyway?
> > 
> > In fact, why are we even *doing* retpolines in inittext? Not that we
> > are; since we flipped the ALTERNATIVE logic around, at that point we
> > still have the 'oldinstr' which is a bare jmp anyway. We might as well
> > do this:
> > 
> > --- a/include/linux/init.h
> > +++ b/include/linux/init.h
> > @@ -37,10 +37,15 @@
> >   * as gcc otherwise puts the data into the bss section and not into the init
> >   * section.
> >   */
> > +#if defined(RETPOLINE) && !defined(MODULE)
> > +#define __noretpoline __attribute__((indirect_branch("keep")))
> > +#else
> > +#define __noretpoline
> > +#endif

Clearly I cannot read...

> >  /* These are for everybody (although not all archs will actually
> >     discard it in modules) */
> > -#define __init         __section(.init.text) __cold __inittrace __latent_entropy
> > +#define __init         __section(.init.text) __cold __inittrace __latent_entropy __noretpoline
> 
> We run module __init text concurrently with userspace.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 03/24] x86/paravirt: Annotate indirect calls
  2018-01-23 15:25 ` [PATCH 03/24] x86/paravirt: Annotate indirect calls Peter Zijlstra
  2018-01-25 10:02   ` David Woodhouse
@ 2018-01-29 17:58   ` Josh Poimboeuf
  2018-01-29 18:09     ` David Woodhouse
  2018-01-29 18:17     ` Peter Zijlstra
  2018-01-29 18:38   ` Josh Poimboeuf
  2 siblings, 2 replies; 71+ messages in thread
From: Josh Poimboeuf @ 2018-01-29 17:58 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: David Woodhouse, Thomas Gleixner, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

On Tue, Jan 23, 2018 at 04:25:42PM +0100, Peter Zijlstra wrote:
> 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.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/x86/include/asm/paravirt.h       |   22 ++++++++++++++++++----
>  arch/x86/include/asm/paravirt_types.h |    7 ++++++-
>  2 files changed, 24 insertions(+), 5 deletions(-)
> 
> --- a/arch/x86/include/asm/paravirt.h
> +++ b/arch/x86/include/asm/paravirt.h
> @@ -828,6 +828,12 @@ extern void default_banner(void);
>  	 .short clobbers;			\
>  	.popsection
>  
> +#define PARA_RETPOLINE_SAFE				\
> +	773:;						\
> +	.pushsection .discard.retpoline_safe;		\
> +	_ASM_PTR 773b;					\
> +	.popsection

Why does paravirt have its own version of this macro?

-- 
Josh

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 03/24] x86/paravirt: Annotate indirect calls
  2018-01-29 17:58   ` Josh Poimboeuf
@ 2018-01-29 18:09     ` David Woodhouse
  2018-01-29 18:17     ` Peter Zijlstra
  1 sibling, 0 replies; 71+ messages in thread
From: David Woodhouse @ 2018-01-29 18:09 UTC (permalink / raw)
  To: Josh Poimboeuf, Peter Zijlstra
  Cc: Thomas Gleixner, linux-kernel, Dave Hansen, Ashok Raj, Tim Chen,
	Andy Lutomirski, Linus Torvalds, Greg KH, Andrea Arcangeli,
	Andi Kleen, Arjan Van De Ven, Dan Williams, Paolo Bonzini,
	Jun Nakajima, Asit Mallick, Jason Baron

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

On Mon, 2018-01-29 at 11:58 -0600, Josh Poimboeuf wrote:
> On Tue, Jan 23, 2018 at 04:25:42PM +0100, Peter Zijlstra wrote:
> > 
> > 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.
> > 
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > ---
> >  arch/x86/include/asm/paravirt.h       |   22 ++++++++++++++++++---
> > -
> >  arch/x86/include/asm/paravirt_types.h |    7 ++++++-
> >  2 files changed, 24 insertions(+), 5 deletions(-)
> > 
> > --- a/arch/x86/include/asm/paravirt.h
> > +++ b/arch/x86/include/asm/paravirt.h
> > @@ -828,6 +828,12 @@ extern void default_banner(void);
> >  	 .short clobbers;			\
> >  	.popsection
> >  
> > +#define PARA_RETPOLINE_SAFE				\
> > +	773:;						\
> > +	.pushsection .discard.retpoline_safe;		\
> > +	_ASM_PTR 773b;					\
> > +	.popsection
>
> Why does paravirt have its own version of this macro?

Because otherwise we'd only have two, and they might get lonely?

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

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 03/24] x86/paravirt: Annotate indirect calls
  2018-01-29 17:58   ` Josh Poimboeuf
  2018-01-29 18:09     ` David Woodhouse
@ 2018-01-29 18:17     ` Peter Zijlstra
  1 sibling, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-29 18:17 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: David Woodhouse, Thomas Gleixner, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

On Mon, Jan 29, 2018 at 11:58:58AM -0600, Josh Poimboeuf wrote:
> On Tue, Jan 23, 2018 at 04:25:42PM +0100, Peter Zijlstra wrote:
> > 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.
> > 
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > ---
> >  arch/x86/include/asm/paravirt.h       |   22 ++++++++++++++++++----
> >  arch/x86/include/asm/paravirt_types.h |    7 ++++++-
> >  2 files changed, 24 insertions(+), 5 deletions(-)
> > 
> > --- a/arch/x86/include/asm/paravirt.h
> > +++ b/arch/x86/include/asm/paravirt.h
> > @@ -828,6 +828,12 @@ extern void default_banner(void);
> >  	 .short clobbers;			\
> >  	.popsection
> >  
> > +#define PARA_RETPOLINE_SAFE				\
> > +	773:;						\
> > +	.pushsection .discard.retpoline_safe;		\
> > +	_ASM_PTR 773b;					\
> > +	.popsection
> 
> Why does paravirt have its own version of this macro?

Every time I touch paravirt.h I land in header hell :/ Maybe it can be
made to work, but I was too scared to try this time.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 03/24] x86/paravirt: Annotate indirect calls
  2018-01-23 15:25 ` [PATCH 03/24] x86/paravirt: Annotate indirect calls Peter Zijlstra
  2018-01-25 10:02   ` David Woodhouse
  2018-01-29 17:58   ` Josh Poimboeuf
@ 2018-01-29 18:38   ` Josh Poimboeuf
  2018-01-29 19:21     ` Peter Zijlstra
  2 siblings, 1 reply; 71+ messages in thread
From: Josh Poimboeuf @ 2018-01-29 18:38 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: David Woodhouse, Thomas Gleixner, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

On Tue, Jan 23, 2018 at 04:25:42PM +0100, Peter Zijlstra wrote:
> 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.

There's one condition where this isn't true: the 'noreplace-paravirt'
cmdline option.  I have no idea why that option exists but we should
consider just removing it.

-- 
Josh

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 03/24] x86/paravirt: Annotate indirect calls
  2018-01-29 18:38   ` Josh Poimboeuf
@ 2018-01-29 19:21     ` Peter Zijlstra
  2018-01-30 16:02       ` Josh Poimboeuf
  0 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-29 19:21 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: David Woodhouse, Thomas Gleixner, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

On Mon, Jan 29, 2018 at 12:38:50PM -0600, Josh Poimboeuf wrote:
> On Tue, Jan 23, 2018 at 04:25:42PM +0100, Peter Zijlstra wrote:
> > 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.
> 
> There's one condition where this isn't true: the 'noreplace-paravirt'
> cmdline option.  I have no idea why that option exists but we should
> consider just removing it.

Smells like a debug remnant from back when all this patching crap was
new. And yes, that sounds like something that should maybe go away. Esp.
for retpoline builds.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 20/24] objtool: Another static block fail
  2018-01-23 15:25 ` [PATCH 20/24] objtool: Another static block fail Peter Zijlstra
@ 2018-01-29 22:52   ` Josh Poimboeuf
  2018-01-30  9:56     ` Peter Zijlstra
  0 siblings, 1 reply; 71+ messages in thread
From: Josh Poimboeuf @ 2018-01-29 22:52 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: David Woodhouse, Thomas Gleixner, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

On Tue, Jan 23, 2018 at 04:25:59PM +0100, Peter Zijlstra wrote:
> I've observed GCC generate:
> 
>   sym:
>      NOP/JMP 1f	(static_branch)
>      JMP 2f
>   1: /* crud */
>      JMP 3f
>   2: /* other crud */
> 
>   3: RETQ
> 
> 
> This means we need to follow unconditional jumps; be conservative and
> only follow if its a unique jump.
> 
> (I've not yet figured out which CONFIG option is responsible for this,
>  a normal defconfig build does not generate crap like this)
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Any chance we can just add a compiler barrier to the assertion macro and
avoid all this grow_static_blocks() mess?  It seems a bit... fragile.

-- 
Josh

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 20/24] objtool: Another static block fail
  2018-01-29 22:52   ` Josh Poimboeuf
@ 2018-01-30  9:56     ` Peter Zijlstra
  2018-01-31  3:12       ` Josh Poimboeuf
  0 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-30  9:56 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: David Woodhouse, Thomas Gleixner, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

On Mon, Jan 29, 2018 at 04:52:53PM -0600, Josh Poimboeuf wrote:
> On Tue, Jan 23, 2018 at 04:25:59PM +0100, Peter Zijlstra wrote:
> > I've observed GCC generate:
> > 
> >   sym:
> >      NOP/JMP 1f	(static_branch)
> >      JMP 2f
> >   1: /* crud */
> >      JMP 3f
> >   2: /* other crud */
> > 
> >   3: RETQ
> > 
> > 
> > This means we need to follow unconditional jumps; be conservative and
> > only follow if its a unique jump.
> > 
> > (I've not yet figured out which CONFIG option is responsible for this,
> >  a normal defconfig build does not generate crap like this)
> > 
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> 
> Any chance we can just add a compiler barrier to the assertion macro and
> avoid all this grow_static_blocks() mess?  It seems a bit... fragile.

It is all rather unfortunate yes.. :/ I've tried to keep the grow stuff
as conservative as possible while still covering all the weirdness I
found. And while it was great fun, I do agree it would be much better to
not have to do this.


You're thinking of something like this?

 static __always_inline void arch_static_assert(void)
 {
        asm volatile ("1:\n\t"
                      ".pushsection .discard.jump_assert \n\t"
                      _ASM_ALIGN  "\n\t"
                      _ASM_PTR "1b \n\t"
-                     ".popsection \n\t");
+                     ".popsection \n\t" ::: "memory");
 }

That doesn't seem to matter much; see here:

static void
ttwu_stat(struct task_struct *p, int cpu, int wake_flags)
{
        struct rq *rq;

        if (!schedstat_enabled())
                return;

        rq = this_rq();


$ objdump -dr build/kernel/sched/core.o

0000000000001910 <ttwu_stat>:
    1910:       e8 00 00 00 00          callq  1915 <ttwu_stat+0x5>
                        1911: R_X86_64_PC32     __fentry__-0x4
    1915:       41 57                   push   %r15
    1917:       41 56                   push   %r14
    1919:       41 55                   push   %r13
    191b:       41 54                   push   %r12
    191d:       55                      push   %rbp
    191e:       53                      push   %rbx
    191f:       0f 1f 44 00 00          nopl   0x0(%rax,%rax,1)
    1924:       eb 25                   jmp    194b <ttwu_stat+0x3b>
    1926:       41 89 d5                mov    %edx,%r13d
    1929:       41 89 f4                mov    %esi,%r12d
    192c:       48 89 fb                mov    %rdi,%rbx
    192f:       49 c7 c6 00 00 00 00    mov    $0x0,%r14
                        1932: R_X86_64_32S      runqueues


$ objdump -j __jump_table -sr build/kernel/sched.o

0000000000000048 R_X86_64_64       .text+0x000000000000191f
0000000000000050 R_X86_64_64       .text+0x0000000000001926
0000000000000058 R_X86_64_64       sched_schedstats


$ objdump -j .discard.jump_assert -dr build/kernel/sched.o

0000000000000000 R_X86_64_64       .text+0x000000000000192f


It still lifts random crud over that first initial statement (the rq
load).

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 03/24] x86/paravirt: Annotate indirect calls
  2018-01-29 19:21     ` Peter Zijlstra
@ 2018-01-30 16:02       ` Josh Poimboeuf
  2018-01-31  4:13         ` [PATCH] x86/paravirt: Remove 'noreplace-paravirt' cmdline option Josh Poimboeuf
  0 siblings, 1 reply; 71+ messages in thread
From: Josh Poimboeuf @ 2018-01-30 16:02 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: David Woodhouse, Thomas Gleixner, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

On Mon, Jan 29, 2018 at 08:21:08PM +0100, Peter Zijlstra wrote:
> On Mon, Jan 29, 2018 at 12:38:50PM -0600, Josh Poimboeuf wrote:
> > On Tue, Jan 23, 2018 at 04:25:42PM +0100, Peter Zijlstra wrote:
> > > 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.
> > 
> > There's one condition where this isn't true: the 'noreplace-paravirt'
> > cmdline option.  I have no idea why that option exists but we should
> > consider just removing it.
> 
> Smells like a debug remnant from back when all this patching crap was
> new. And yes, that sounds like something that should maybe go away. Esp.
> for retpoline builds.

Ok, I'll whip up a patch to remove it and see if anybody complains.

-- 
Josh

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 20/24] objtool: Another static block fail
  2018-01-30  9:56     ` Peter Zijlstra
@ 2018-01-31  3:12       ` Josh Poimboeuf
  2018-01-31 10:01         ` Peter Zijlstra
  0 siblings, 1 reply; 71+ messages in thread
From: Josh Poimboeuf @ 2018-01-31  3:12 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: David Woodhouse, Thomas Gleixner, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

On Tue, Jan 30, 2018 at 10:56:53AM +0100, Peter Zijlstra wrote:
> On Mon, Jan 29, 2018 at 04:52:53PM -0600, Josh Poimboeuf wrote:
> > On Tue, Jan 23, 2018 at 04:25:59PM +0100, Peter Zijlstra wrote:
> > > I've observed GCC generate:
> > > 
> > >   sym:
> > >      NOP/JMP 1f	(static_branch)
> > >      JMP 2f
> > >   1: /* crud */
> > >      JMP 3f
> > >   2: /* other crud */
> > > 
> > >   3: RETQ
> > > 
> > > 
> > > This means we need to follow unconditional jumps; be conservative and
> > > only follow if its a unique jump.
> > > 
> > > (I've not yet figured out which CONFIG option is responsible for this,
> > >  a normal defconfig build does not generate crap like this)
> > > 
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > 
> > Any chance we can just add a compiler barrier to the assertion macro and
> > avoid all this grow_static_blocks() mess?  It seems a bit... fragile.
> 
> It is all rather unfortunate yes.. :/ I've tried to keep the grow stuff
> as conservative as possible while still covering all the weirdness I
> found. And while it was great fun, I do agree it would be much better to
> not have to do this.

The more I look at this patch, and the previous one, the less sure I am
about this idea.  (This isn't a comment about the code, just some
general uneasiness about the unpredictable GCC behavior we're trying to
constrain, and whether it's worth the pain.)

Covering all the corner cases is complicated, and it might get even more
complicated with future compiler optimizations.

What if we *only* use the assertions in places that really need them,
like the IBRS checks?  Would that allow us to simplify and drop these
last two (or three) patches? 

Or, maybe we should just forget the whole thing and just stick with the
dynamic IBRS checks with lfence.  Yes, it's less ideal for the kernel,
but adding these acrobatics to objtool also has a cost.

-- 
Josh

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH] x86/paravirt: Remove 'noreplace-paravirt' cmdline option
  2018-01-30 16:02       ` Josh Poimboeuf
@ 2018-01-31  4:13         ` Josh Poimboeuf
  2018-01-31  5:59           ` Juergen Gross
  2018-01-31  9:42           ` [tip:x86/pti] " tip-bot for Josh Poimboeuf
  0 siblings, 2 replies; 71+ messages in thread
From: Josh Poimboeuf @ 2018-01-31  4:13 UTC (permalink / raw)
  To: x86, Juergen Gross, Alok Kataria, Rusty Russell
  Cc: David Woodhouse, Thomas Gleixner, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron,
	Peter Zijlstra

The 'noreplace-paravirt' option disables paravirt patching, leaving the
original pv indirect calls in place.

That's highly incompatible with retpolines, unless we want to uglify
paravirt even further and convert the paravirt calls to retpolines.

As far as I can tell, the option doesn't seem to be useful for much
other than introducing surprising corner cases and making the kernel
vulnerable to Spectre v2.  It was probably a debug option from the early
paravirt days.  So just remove it.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  2 --
 arch/x86/kernel/alternative.c                   | 14 --------------
 2 files changed, 16 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a205567f537f..f6741a5d89e7 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2739,8 +2739,6 @@
 	norandmaps	Don't use address space randomization.  Equivalent to
 			echo 0 > /proc/sys/kernel/randomize_va_space
 
-	noreplace-paravirt	[X86,IA-64,PV_OPS] Don't patch paravirt_ops
-
 	noreplace-smp	[X86-32,SMP] Don't replace SMP instructions
 			with UP alternatives
 
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 30571fdaaf6f..a481763a3776 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -46,17 +46,6 @@ static int __init setup_noreplace_smp(char *str)
 }
 __setup("noreplace-smp", setup_noreplace_smp);
 
-#ifdef CONFIG_PARAVIRT
-static int __initdata_or_module noreplace_paravirt = 0;
-
-static int __init setup_noreplace_paravirt(char *str)
-{
-	noreplace_paravirt = 1;
-	return 1;
-}
-__setup("noreplace-paravirt", setup_noreplace_paravirt);
-#endif
-
 #define DPRINTK(fmt, args...)						\
 do {									\
 	if (debug_alternative)						\
@@ -599,9 +588,6 @@ void __init_or_module apply_paravirt(struct paravirt_patch_site *start,
 	struct paravirt_patch_site *p;
 	char insnbuf[MAX_PATCH_LEN];
 
-	if (noreplace_paravirt)
-		return;
-
 	for (p = start; p < end; p++) {
 		unsigned int used;
 
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* Re: [PATCH] x86/paravirt: Remove 'noreplace-paravirt' cmdline option
  2018-01-31  4:13         ` [PATCH] x86/paravirt: Remove 'noreplace-paravirt' cmdline option Josh Poimboeuf
@ 2018-01-31  5:59           ` Juergen Gross
  2018-01-31  9:42           ` [tip:x86/pti] " tip-bot for Josh Poimboeuf
  1 sibling, 0 replies; 71+ messages in thread
From: Juergen Gross @ 2018-01-31  5:59 UTC (permalink / raw)
  To: Josh Poimboeuf, x86, Alok Kataria, Rusty Russell
  Cc: David Woodhouse, Thomas Gleixner, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron,
	Peter Zijlstra

On 31/01/18 05:13, Josh Poimboeuf wrote:
> The 'noreplace-paravirt' option disables paravirt patching, leaving the
> original pv indirect calls in place.
> 
> That's highly incompatible with retpolines, unless we want to uglify
> paravirt even further and convert the paravirt calls to retpolines.
> 
> As far as I can tell, the option doesn't seem to be useful for much
> other than introducing surprising corner cases and making the kernel
> vulnerable to Spectre v2.  It was probably a debug option from the early
> paravirt days.  So just remove it.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 08/24] x86,sme: Annotate indirect call
  2018-01-26 10:37   ` David Woodhouse
  2018-01-29 17:49     ` Peter Zijlstra
@ 2018-01-31  9:29     ` Peter Zijlstra
  2018-01-31 15:04       ` Josh Poimboeuf
  1 sibling, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-31  9:29 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Thomas Gleixner, Josh Poimboeuf, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron,
	Tom Lendacky, Borislav Petkov

On Fri, Jan 26, 2018 at 10:37:30AM +0000, David Woodhouse wrote:
> On Tue, 2018-01-23 at 16:25 +0100, Peter Zijlstra wrote:
> > This is boot code, we run this _way_ before userspace comes along to
> > poison our branch predictor.
> 
> Hm, objtool knows about sections, doesn't it? Why it is whining about
> indirect jumps in inittext anyway?
> 
> In fact, why are we even *doing* retpolines in inittext? Not that we
> are; since we flipped the ALTERNATIVE logic around, at that point we
> still have the 'oldinstr' which is a bare jmp anyway. We might as well
> do this:
> 
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -37,10 +37,15 @@
>   * as gcc otherwise puts the data into the bss section and not into the init
>   * section.
>   */
> +#if defined(RETPOLINE) && !defined(MODULE)
> +#define __noretpoline __attribute__((indirect_branch("keep")))
> +#else
> +#define __noretpoline
> +#endif
>  
>  /* These are for everybody (although not all archs will actually
>     discard it in modules) */
> -#define __init         __section(.init.text) __cold __inittrace __latent_entropy
> +#define __init         __section(.init.text) __cold __inittrace __latent_entropy __noretpoline
>  #define __initdata     __section(.init.data)
>  #define __initconst    __section(.init.rodata)
>  #define __exitdata     __section(.exit.data)


Something like so then?

---

Subject: objtool: Add module specific retpoline rules
From: Peter Zijlstra <peterz@infradead.org>
Date: Wed Jan 31 10:18:28 CET 2018

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");

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [tip:x86/pti] x86/paravirt: Remove 'noreplace-paravirt' cmdline option
  2018-01-31  4:13         ` [PATCH] x86/paravirt: Remove 'noreplace-paravirt' cmdline option Josh Poimboeuf
  2018-01-31  5:59           ` Juergen Gross
@ 2018-01-31  9:42           ` tip-bot for Josh Poimboeuf
  1 sibling, 0 replies; 71+ messages in thread
From: tip-bot for Josh Poimboeuf @ 2018-01-31  9:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: torvalds, jun.nakajima, pbonzini, tglx, jbaron, mingo,
	asit.k.mallick, gregkh, tim.c.chen, arjan.van.de.ven,
	linux-kernel, jpoimboe, dan.j.williams, hpa, dave.hansen, rusty,
	jgross, akataria, ashok.raj, aarcange, ak, luto, dwmw2, peterz

Commit-ID:  12c69f1e94c89d40696e83804dd2f0965b5250cd
Gitweb:     https://git.kernel.org/tip/12c69f1e94c89d40696e83804dd2f0965b5250cd
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Tue, 30 Jan 2018 22:13:33 -0600
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 31 Jan 2018 10:37:45 +0100

x86/paravirt: Remove 'noreplace-paravirt' cmdline option

The 'noreplace-paravirt' option disables paravirt patching, leaving the
original pv indirect calls in place.

That's highly incompatible with retpolines, unless we want to uglify
paravirt even further and convert the paravirt calls to retpolines.

As far as I can tell, the option doesn't seem to be useful for much
other than introducing surprising corner cases and making the kernel
vulnerable to Spectre v2.  It was probably a debug option from the early
paravirt days.  So just remove it.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Juergen Gross <jgross@suse.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Asit Mallick <asit.k.mallick@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jason Baron <jbaron@akamai.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Alok Kataria <akataria@vmware.com>
Cc: Arjan Van De Ven <arjan.van.de.ven@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Dan Williams <dan.j.williams@intel.com>
Link: https://lkml.kernel.org/r/20180131041333.2x6blhxirc2kclrq@treble

---
 Documentation/admin-guide/kernel-parameters.txt |  2 --
 arch/x86/kernel/alternative.c                   | 14 --------------
 2 files changed, 16 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 46b26bf..1e762c2 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2742,8 +2742,6 @@
 	norandmaps	Don't use address space randomization.  Equivalent to
 			echo 0 > /proc/sys/kernel/randomize_va_space
 
-	noreplace-paravirt	[X86,IA-64,PV_OPS] Don't patch paravirt_ops
-
 	noreplace-smp	[X86-32,SMP] Don't replace SMP instructions
 			with UP alternatives
 
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 30571fd..a481763 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -46,17 +46,6 @@ static int __init setup_noreplace_smp(char *str)
 }
 __setup("noreplace-smp", setup_noreplace_smp);
 
-#ifdef CONFIG_PARAVIRT
-static int __initdata_or_module noreplace_paravirt = 0;
-
-static int __init setup_noreplace_paravirt(char *str)
-{
-	noreplace_paravirt = 1;
-	return 1;
-}
-__setup("noreplace-paravirt", setup_noreplace_paravirt);
-#endif
-
 #define DPRINTK(fmt, args...)						\
 do {									\
 	if (debug_alternative)						\
@@ -599,9 +588,6 @@ void __init_or_module apply_paravirt(struct paravirt_patch_site *start,
 	struct paravirt_patch_site *p;
 	char insnbuf[MAX_PATCH_LEN];
 
-	if (noreplace_paravirt)
-		return;
-
 	for (p = start; p < end; p++) {
 		unsigned int used;
 

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* Re: [PATCH 20/24] objtool: Another static block fail
  2018-01-31  3:12       ` Josh Poimboeuf
@ 2018-01-31 10:01         ` Peter Zijlstra
  2018-01-31 10:07           ` David Woodhouse
  0 siblings, 1 reply; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-31 10:01 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: David Woodhouse, Thomas Gleixner, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

On Tue, Jan 30, 2018 at 09:12:21PM -0600, Josh Poimboeuf wrote:
> 
> Or, maybe we should just forget the whole thing and just stick with the
> dynamic IBRS checks with lfence.  Yes, it's less ideal for the kernel,
> but adding these acrobatics to objtool also has a cost.

For now, IBRS seems off the table entirely. But no, I really don't want
to have to unconditionally eat the LFENCE cost in all those sites.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 20/24] objtool: Another static block fail
  2018-01-31 10:01         ` Peter Zijlstra
@ 2018-01-31 10:07           ` David Woodhouse
  2018-01-31 10:27             ` Peter Zijlstra
  0 siblings, 1 reply; 71+ messages in thread
From: David Woodhouse @ 2018-01-31 10:07 UTC (permalink / raw)
  To: Peter Zijlstra, Josh Poimboeuf
  Cc: Thomas Gleixner, linux-kernel, Dave Hansen, Ashok Raj, Tim Chen,
	Andy Lutomirski, Linus Torvalds, Greg KH, Andrea Arcangeli,
	Andi Kleen, Arjan Van De Ven, Dan Williams, Paolo Bonzini,
	Jun Nakajima, Asit Mallick, Jason Baron

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



On Wed, 2018-01-31 at 11:01 +0100, Peter Zijlstra wrote:
> On Tue, Jan 30, 2018 at 09:12:21PM -0600, Josh Poimboeuf wrote:
> > 
> > Or, maybe we should just forget the whole thing and just stick with the
> > dynamic IBRS checks with lfence.  Yes, it's less ideal for the kernel,
> > but adding these acrobatics to objtool also has a cost.
> 
> For now, IBRS seems off the table entirely. But no, I really don't want
> to have to unconditionally eat the LFENCE cost in all those sites.

There's also alternatives. And without the IBRS-on-kernel-entry bits
there aren't that many call sites that really need this anyway and
don't have *other* conditionals that really are runtime-only (like
dumpable etc.).

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

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 20/24] objtool: Another static block fail
  2018-01-31 10:07           ` David Woodhouse
@ 2018-01-31 10:27             ` Peter Zijlstra
  0 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-31 10:27 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Josh Poimboeuf, Thomas Gleixner, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron

On Wed, Jan 31, 2018 at 10:07:06AM +0000, David Woodhouse wrote:
> On Wed, 2018-01-31 at 11:01 +0100, Peter Zijlstra wrote:
> > On Tue, Jan 30, 2018 at 09:12:21PM -0600, Josh Poimboeuf wrote:
> > > 
> > > Or, maybe we should just forget the whole thing and just stick with the
> > > dynamic IBRS checks with lfence.  Yes, it's less ideal for the kernel,
> > > but adding these acrobatics to objtool also has a cost.
> > 
> > For now, IBRS seems off the table entirely. But no, I really don't want
> > to have to unconditionally eat the LFENCE cost in all those sites.
> 
> There's also alternatives. And without the IBRS-on-kernel-entry bits
> there aren't that many call sites that really need this anyway and
> don't have *other* conditionals that really are runtime-only (like
> dumpable etc.).

There is that.. So I think people wanted to use jump_labels for IBRS so
that we could runtime enable it when people loaded dodgy modules or
something. But even that we could all write in inline asm if needed I
suppose, it'll be ugly, but it should work.

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 08/24] x86,sme: Annotate indirect call
  2018-01-31  9:29     ` Peter Zijlstra
@ 2018-01-31 15:04       ` Josh Poimboeuf
  2018-01-31 16:00         ` Peter Zijlstra
  0 siblings, 1 reply; 71+ messages in thread
From: Josh Poimboeuf @ 2018-01-31 15:04 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: David Woodhouse, Thomas Gleixner, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron,
	Tom Lendacky, Borislav Petkov

On Wed, Jan 31, 2018 at 10:29:21AM +0100, Peter Zijlstra wrote:
> On Fri, Jan 26, 2018 at 10:37:30AM +0000, David Woodhouse wrote:
> > On Tue, 2018-01-23 at 16:25 +0100, Peter Zijlstra wrote:
> > > This is boot code, we run this _way_ before userspace comes along to
> > > poison our branch predictor.
> > 
> > Hm, objtool knows about sections, doesn't it? Why it is whining about
> > indirect jumps in inittext anyway?
> > 
> > In fact, why are we even *doing* retpolines in inittext? Not that we
> > are; since we flipped the ALTERNATIVE logic around, at that point we
> > still have the 'oldinstr' which is a bare jmp anyway. We might as well
> > do this:

Ont the other hand, is there any harm in doing retpolines in .init.text?

I also had a similar question about the ANNOTATE_RETPOLINE_SAFE thing.

If there's no harm, it would be simpler and more robust to just do
retpolines everywhere and not worry about special cases.

(Forgetting paravirt for the moment, which is the eternal "special
case".)

I was also thinking about adding a debug option for _runtime_ retpoline
verification that decodes all kernel text and reports any indirect
branches it finds (yes, kind of like an in-kernel objtool).  That would
be a lot more straightforward without special cases.  Obviously
.init.text wouldn't be a problem there, but the other annotated safe
locations would.

-- 
Josh

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH 08/24] x86,sme: Annotate indirect call
  2018-01-31 15:04       ` Josh Poimboeuf
@ 2018-01-31 16:00         ` Peter Zijlstra
  0 siblings, 0 replies; 71+ messages in thread
From: Peter Zijlstra @ 2018-01-31 16:00 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: David Woodhouse, Thomas Gleixner, linux-kernel, Dave Hansen,
	Ashok Raj, Tim Chen, Andy Lutomirski, Linus Torvalds, Greg KH,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
	Paolo Bonzini, Jun Nakajima, Asit Mallick, Jason Baron,
	Tom Lendacky, Borislav Petkov

On Wed, Jan 31, 2018 at 09:04:51AM -0600, Josh Poimboeuf wrote:
> If there's no harm, it would be simpler and more robust to just do
> retpolines everywhere and not worry about special cases.
> 
> (Forgetting paravirt for the moment, which is the eternal "special
> case".)
> 
> I was also thinking about adding a debug option for _runtime_ retpoline
> verification that decodes all kernel text and reports any indirect
> branches it finds (yes, kind of like an in-kernel objtool).  That would
> be a lot more straightforward without special cases.  Obviously
> .init.text wouldn't be a problem there, but the other annotated safe
> locations would.

Like said, even retpolines themselves need annotation. We could simply
keep the section and not .discard. it.

^ permalink raw reply	[flat|nested] 71+ messages in thread

end of thread, other threads:[~2018-01-31 16:00 UTC | newest]

Thread overview: 71+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-23 15:25 [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
2018-01-23 15:25 ` [PATCH 01/24] objtool: Use existing global variables for options Peter Zijlstra
2018-01-23 15:25 ` [PATCH 02/24] objtool: Add retpoline validation Peter Zijlstra
2018-01-23 18:21   ` Borislav Petkov
2018-01-26  9:54   ` David Woodhouse
2018-01-23 15:25 ` [PATCH 03/24] x86/paravirt: Annotate indirect calls Peter Zijlstra
2018-01-25 10:02   ` David Woodhouse
2018-01-25 10:22     ` Peter Zijlstra
2018-01-25 10:26       ` Juergen Gross
2018-01-25 10:52         ` David Woodhouse
2018-01-25 11:35           ` Peter Zijlstra
2018-01-26  9:57             ` David Woodhouse
2018-01-29 17:58   ` Josh Poimboeuf
2018-01-29 18:09     ` David Woodhouse
2018-01-29 18:17     ` Peter Zijlstra
2018-01-29 18:38   ` Josh Poimboeuf
2018-01-29 19:21     ` Peter Zijlstra
2018-01-30 16:02       ` Josh Poimboeuf
2018-01-31  4:13         ` [PATCH] x86/paravirt: Remove 'noreplace-paravirt' cmdline option Josh Poimboeuf
2018-01-31  5:59           ` Juergen Gross
2018-01-31  9:42           ` [tip:x86/pti] " tip-bot for Josh Poimboeuf
2018-01-23 15:25 ` [PATCH 04/24] x86,nospec: Annotate indirect calls/jumps Peter Zijlstra
2018-01-26 10:19   ` David Woodhouse
2018-01-29 17:44     ` Peter Zijlstra
2018-01-23 15:25 ` [PATCH 05/24] x86: Annotate indirect jump in head_64.S Peter Zijlstra
2018-01-26 10:24   ` David Woodhouse
2018-01-23 15:25 ` [PATCH 06/24] x86,kvm: Fix indirect calls in emulator Peter Zijlstra
2018-01-23 20:28   ` Borislav Petkov
2018-01-23 20:48     ` David Woodhouse
2018-01-24 10:35       ` Peter Zijlstra
2018-01-24 10:43         ` Paolo Bonzini
2018-01-25  9:34           ` Peter Zijlstra
2018-01-25  9:49             ` David Woodhouse
2018-01-26 10:57             ` Paolo Bonzini
2018-01-23 15:25 ` [PATCH 07/24] x86,vmx: Fix indirect call Peter Zijlstra
2018-01-25  9:36   ` Peter Zijlstra
2018-01-23 15:25 ` [PATCH 08/24] x86,sme: Annotate " Peter Zijlstra
2018-01-26 10:37   ` David Woodhouse
2018-01-29 17:49     ` Peter Zijlstra
2018-01-29 17:50       ` Peter Zijlstra
2018-01-31  9:29     ` Peter Zijlstra
2018-01-31 15:04       ` Josh Poimboeuf
2018-01-31 16:00         ` Peter Zijlstra
2018-01-23 15:25 ` [PATCH 09/24] jump_label: Add branch hints to static_branch_{un,}likely() Peter Zijlstra
2018-01-24 18:46   ` Borislav Petkov
2018-01-23 15:25 ` [PATCH 10/24] sched: Optimize ttwu_stat() Peter Zijlstra
2018-01-23 15:25 ` [PATCH 11/24] x86: Reindent _static_cpu_has Peter Zijlstra
2018-01-23 15:25 ` [PATCH 12/24] x86: Update _static_cpu_has to use all named variables Peter Zijlstra
2018-01-25 19:31   ` Borislav Petkov
2018-01-23 15:25 ` [PATCH 13/24] objtool: Implement base jump_assert support Peter Zijlstra
2018-01-26 10:45   ` David Woodhouse
2018-01-23 15:25 ` [PATCH 14/24] x86: Add a type field to alt_instr Peter Zijlstra
2018-01-23 15:25 ` [PATCH 15/24] x86: Annotate static_cpu_has alternative Peter Zijlstra
2018-01-23 15:25 ` [PATCH 16/24] objtool: Implement jump_assert for _static_cpu_has() Peter Zijlstra
2018-01-23 15:25 ` [PATCH 17/24] objtool: Introduce special_type Peter Zijlstra
2018-01-23 15:25 ` [PATCH 18/24] objtool: More complex static jump implementation Peter Zijlstra
2018-01-23 15:25 ` [PATCH 19/24] objtool: Even more complex static block checks Peter Zijlstra
2018-01-23 15:25 ` [PATCH 20/24] objtool: Another static block fail Peter Zijlstra
2018-01-29 22:52   ` Josh Poimboeuf
2018-01-30  9:56     ` Peter Zijlstra
2018-01-31  3:12       ` Josh Poimboeuf
2018-01-31 10:01         ` Peter Zijlstra
2018-01-31 10:07           ` David Woodhouse
2018-01-31 10:27             ` Peter Zijlstra
2018-01-23 15:26 ` [PATCH 21/24] objtool: Skip static assert when KCOV/KASAN Peter Zijlstra
2018-01-23 15:26 ` [PATCH 22/24] x86/jump_label: Implement arch_static_assert() Peter Zijlstra
2018-01-23 15:26 ` [PATCH 23/24] x86: Force asm-goto Peter Zijlstra
2018-01-23 15:26 ` [PATCH 24/24] x86: Remove FAST_FEATURE_TESTS Peter Zijlstra
2018-01-23 15:42 ` [PATCH 00/24] objtool: retpoline and asm-goto validation Peter Zijlstra
2018-01-23 15:57   ` David Woodhouse
2018-01-23 16:03     ` Peter Zijlstra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.