* [PATCH 1/7] objtool: Use existing global variables for options
2018-02-01 14:34 [PATCH 0/7] objtool: retpoline validation Peter Zijlstra
@ 2018-02-01 14:34 ` Peter Zijlstra
2018-02-01 14:34 ` [PATCH 2/7] objtool: Add retpoline validation Peter Zijlstra
` (7 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Peter Zijlstra @ 2018-02-01 14:34 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, Peter Zijlstra
[-- Attachment #1: peterz-objtool-unclutter.patch --]
[-- Type: text/plain, Size: 2802 bytes --]
Use the existing global variables instead of passing them around and
creating duplicate global variables.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
tools/objtool/builtin-check.c | 2 +-
tools/objtool/builtin-orc.c | 6 +-----
tools/objtool/builtin.h | 5 +++++
tools/objtool/check.c | 5 ++---
tools/objtool/check.h | 2 +-
5 files changed, 10 insertions(+), 10 deletions(-)
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -53,5 +53,5 @@ int cmd_check(int argc, const char **arg
objname = argv[0];
- return check(objname, no_fp, no_unreachable, false);
+ return check(objname, false);
}
--- a/tools/objtool/builtin-orc.c
+++ b/tools/objtool/builtin-orc.c
@@ -25,7 +25,6 @@
*/
#include <string.h>
-#include <subcmd/parse-options.h>
#include "builtin.h"
#include "check.h"
@@ -36,9 +35,6 @@ static const char *orc_usage[] = {
NULL,
};
-extern const struct option check_options[];
-extern bool no_fp, no_unreachable;
-
int cmd_orc(int argc, const char **argv)
{
const char *objname;
@@ -54,7 +50,7 @@ int cmd_orc(int argc, const char **argv)
objname = argv[0];
- return check(objname, no_fp, no_unreachable, true);
+ return check(objname, true);
}
if (!strcmp(argv[0], "dump")) {
--- a/tools/objtool/builtin.h
+++ b/tools/objtool/builtin.h
@@ -17,6 +17,11 @@
#ifndef _BUILTIN_H
#define _BUILTIN_H
+#include <subcmd/parse-options.h>
+
+extern const struct option check_options[];
+extern bool no_fp, no_unreachable;
+
extern int cmd_check(int argc, const char **argv);
extern int cmd_orc(int argc, const char **argv);
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -18,6 +18,7 @@
#include <string.h>
#include <stdlib.h>
+#include "builtin.h"
#include "check.h"
#include "elf.h"
#include "special.h"
@@ -33,7 +34,6 @@ struct alternative {
};
const char *objname;
-static bool no_fp;
struct cfi_state initial_func_cfi;
struct instruction *find_insn(struct objtool_file *file,
@@ -1978,13 +1978,12 @@ static void cleanup(struct objtool_file
elf_close(file->elf);
}
-int check(const char *_objname, bool _no_fp, bool no_unreachable, bool orc)
+int check(const char *_objname, bool orc)
{
struct objtool_file file;
int ret, warnings = 0;
objname = _objname;
- no_fp = _no_fp;
file.elf = elf_open(objname, orc ? O_RDWR : O_RDONLY);
if (!file.elf)
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -62,7 +62,7 @@ struct objtool_file {
bool ignore_unreachables, c_file, hints;
};
-int check(const char *objname, bool no_fp, bool no_unreachable, bool orc);
+int check(const char *objname, bool orc);
struct instruction *find_insn(struct objtool_file *file,
struct section *sec, unsigned long offset);
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 2/7] objtool: Add retpoline validation
2018-02-01 14:34 [PATCH 0/7] objtool: retpoline validation Peter Zijlstra
2018-02-01 14:34 ` [PATCH 1/7] objtool: Use existing global variables for options Peter Zijlstra
@ 2018-02-01 14:34 ` Peter Zijlstra
2018-02-01 14:34 ` [PATCH 3/7] objtool: Add module specific retpoline rules Peter Zijlstra
` (6 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Peter Zijlstra @ 2018-02-01 14:34 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, Peter Zijlstra, David Woodhouse
[-- Attachment #1: peterz-objtool-indirect.patch --]
[-- Type: text/plain, Size: 5537 bytes --]
David requested a objtool validation pass for RETPOLINE enabled
builds, where it validates no unannotated indirect jumps or calls are
left.
Add an additional .discard.retpoline_safe section to allow annotating
the few indirect sites that are required and safe.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Requested-by: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
scripts/Makefile.build | 4 +
tools/objtool/builtin-check.c | 3 -
tools/objtool/builtin.h | 2
tools/objtool/check.c | 86 +++++++++++++++++++++++++++++++++++++++++-
tools/objtool/check.h | 1
5 files changed, 93 insertions(+), 3 deletions(-)
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -269,6 +269,10 @@ objtool_args += --no-unreachable
else
objtool_args += $(call cc-ifversion, -lt, 0405, --no-unreachable)
endif
+ifdef CONFIG_RETPOLINE
+ objtool_args += --retpoline
+endif
+
ifdef CONFIG_MODVERSIONS
objtool_o = $(@D)/.tmp_$(@F)
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -29,7 +29,7 @@
#include "builtin.h"
#include "check.h"
-bool no_fp, no_unreachable;
+bool no_fp, no_unreachable, retpoline;
static const char * const check_usage[] = {
"objtool check [<options>] file.o",
@@ -39,6 +39,7 @@ static const char * const check_usage[]
const struct option check_options[] = {
OPT_BOOLEAN('f', "no-fp", &no_fp, "Skip frame pointer validation"),
OPT_BOOLEAN('u', "no-unreachable", &no_unreachable, "Skip 'unreachable instruction' warnings"),
+ OPT_BOOLEAN('r', "retpoline", &retpoline, "Validate retpoline assumptions"),
OPT_END(),
};
--- a/tools/objtool/builtin.h
+++ b/tools/objtool/builtin.h
@@ -20,7 +20,7 @@
#include <subcmd/parse-options.h>
extern const struct option check_options[];
-extern bool no_fp, no_unreachable;
+extern bool no_fp, no_unreachable, retpoline;
extern int cmd_check(int argc, const char **argv);
extern int cmd_orc(int argc, const char **argv);
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -496,6 +496,7 @@ static int add_jump_destinations(struct
* disguise, so convert them accordingly.
*/
insn->type = INSN_JUMP_DYNAMIC;
+ insn->retpoline_safe = true;
continue;
} else {
/* sibling call */
@@ -547,7 +548,8 @@ static int add_call_destinations(struct
if (!insn->call_dest && !insn->ignore) {
WARN_FUNC("unsupported intra-function call",
insn->sec, insn->offset);
- WARN("If this is a retpoline, please patch it in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE.");
+ if (retpoline)
+ WARN("If this is a retpoline, please patch it in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE.");
return -1;
}
@@ -1070,6 +1072,54 @@ static int read_unwind_hints(struct objt
return 0;
}
+static int read_retpoline_hints(struct objtool_file *file)
+{
+ struct section *sec, *relasec;
+ struct instruction *insn;
+ struct rela *rela;
+ int i;
+
+ sec = find_section_by_name(file->elf, ".discard.retpoline_safe");
+ if (!sec)
+ return 0;
+
+ relasec = sec->rela;
+ if (!relasec) {
+ WARN("missing .rela.discard.retpoline_safe section");
+ return -1;
+ }
+
+ if (sec->len % sizeof(unsigned long)) {
+ WARN("retpoline_safe size mismatch: %d %ld", sec->len, sizeof(unsigned long));
+ return -1;
+ }
+
+ for (i = 0; i < sec->len / sizeof(unsigned long); i++) {
+ rela = find_rela_by_dest(sec, i * sizeof(unsigned long));
+ if (!rela) {
+ WARN("can't find rela for retpoline_safe[%d]", i);
+ return -1;
+ }
+
+ insn = find_insn(file, rela->sym->sec, rela->addend);
+ if (!insn) {
+ WARN("can't find insn for retpoline_safe[%d]", i);
+ return -1;
+ }
+
+ if (insn->type != INSN_JUMP_DYNAMIC &&
+ insn->type != INSN_CALL_DYNAMIC) {
+ WARN_FUNC("retpoline_safe hint not a indirect jump/call",
+ insn->sec, insn->offset);
+ return -1;
+ }
+
+ insn->retpoline_safe = true;
+ }
+
+ return 0;
+}
+
static int decode_sections(struct objtool_file *file)
{
int ret;
@@ -1108,6 +1158,10 @@ static int decode_sections(struct objtoo
if (ret)
return ret;
+ ret = read_retpoline_hints(file);
+ if (ret)
+ return ret;
+
return 0;
}
@@ -1853,6 +1907,29 @@ static int validate_unwind_hints(struct
return warnings;
}
+static int validate_retpoline(struct objtool_file *file)
+{
+ struct instruction *insn;
+ int warnings = 0;
+
+ for_each_insn(file, insn) {
+ if (insn->type != INSN_JUMP_DYNAMIC &&
+ insn->type != INSN_CALL_DYNAMIC)
+ continue;
+
+ if (insn->retpoline_safe)
+ continue;
+
+ WARN_FUNC("indirect %s found in RETPOLINE build",
+ insn->sec, insn->offset,
+ insn->type == INSN_JUMP_DYNAMIC ? "jump" : "call");
+
+ warnings++;
+ }
+
+ return warnings;
+}
+
static bool is_kasan_insn(struct instruction *insn)
{
return (insn->type == INSN_CALL &&
@@ -2028,6 +2105,13 @@ int check(const char *_objname, bool orc
if (list_empty(&file.insn_list))
goto out;
+ if (retpoline) {
+ ret = validate_retpoline(&file);
+ if (ret < 0)
+ return ret;
+ warnings += ret;
+ }
+
ret = validate_functions(&file);
if (ret < 0)
goto out;
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -45,6 +45,7 @@ struct instruction {
unsigned char type;
unsigned long immediate;
bool alt_group, visited, dead_end, ignore, hint, save, restore, ignore_alts;
+ bool retpoline_safe;
struct symbol *call_dest;
struct instruction *jump_dest;
struct list_head alts;
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 3/7] objtool: Add module specific retpoline rules
2018-02-01 14:34 [PATCH 0/7] objtool: retpoline validation Peter Zijlstra
2018-02-01 14:34 ` [PATCH 1/7] objtool: Use existing global variables for options Peter Zijlstra
2018-02-01 14:34 ` [PATCH 2/7] objtool: Add retpoline validation Peter Zijlstra
@ 2018-02-01 14:34 ` Peter Zijlstra
2018-02-01 14:34 ` [PATCH 4/7] x86,nospec: Annotate indirect calls/jumps Peter Zijlstra
` (5 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Peter Zijlstra @ 2018-02-01 14:34 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, Peter Zijlstra
[-- Attachment #1: peterz-objtool-module-rules.patch --]
[-- Type: text/plain, Size: 2443 bytes --]
David wanted to not use retpolines in .init.text but that will trip up
objtool retpoline validation, fix that.
Requested-by: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
scripts/Makefile.build | 2 ++
tools/objtool/builtin-check.c | 3 ++-
tools/objtool/builtin.h | 2 +-
tools/objtool/check.c | 9 +++++++++
4 files changed, 14 insertions(+), 2 deletions(-)
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -256,6 +256,8 @@ __objtool_obj := $(objtree)/tools/objtoo
objtool_args = $(if $(CONFIG_UNWINDER_ORC),orc generate,check)
+objtool_args += $(if $(part-of-module), --module,)
+
ifndef CONFIG_FRAME_POINTER
objtool_args += --no-fp
endif
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -29,7 +29,7 @@
#include "builtin.h"
#include "check.h"
-bool no_fp, no_unreachable, retpoline;
+bool no_fp, no_unreachable, retpoline, module;
static const char * const check_usage[] = {
"objtool check [<options>] file.o",
@@ -40,6 +40,7 @@ const struct option check_options[] = {
OPT_BOOLEAN('f', "no-fp", &no_fp, "Skip frame pointer validation"),
OPT_BOOLEAN('u', "no-unreachable", &no_unreachable, "Skip 'unreachable instruction' warnings"),
OPT_BOOLEAN('r', "retpoline", &retpoline, "Validate retpoline assumptions"),
+ OPT_BOOLEAN('m', "module", &module, "Indicates the object will be part of a kernel module"),
OPT_END(),
};
--- a/tools/objtool/builtin.h
+++ b/tools/objtool/builtin.h
@@ -20,7 +20,7 @@
#include <subcmd/parse-options.h>
extern const struct option check_options[];
-extern bool no_fp, no_unreachable, retpoline;
+extern bool no_fp, no_unreachable, retpoline, module;
extern int cmd_check(int argc, const char **argv);
extern int cmd_orc(int argc, const char **argv);
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1920,6 +1920,15 @@ static int validate_retpoline(struct obj
if (insn->retpoline_safe)
continue;
+ /*
+ * .init.text code is ran before userspace and thus doesn't
+ * strictly need retpolines, except for modules which are
+ * loaded late, they very much do need retpoline in their
+ * .init.text
+ */
+ if (!strcmp(insn->sec->name, ".init.text") && !module)
+ continue;
+
WARN_FUNC("indirect %s found in RETPOLINE build",
insn->sec, insn->offset,
insn->type == INSN_JUMP_DYNAMIC ? "jump" : "call");
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 4/7] x86,nospec: Annotate indirect calls/jumps
2018-02-01 14:34 [PATCH 0/7] objtool: retpoline validation Peter Zijlstra
` (2 preceding siblings ...)
2018-02-01 14:34 ` [PATCH 3/7] objtool: Add module specific retpoline rules Peter Zijlstra
@ 2018-02-01 14:34 ` Peter Zijlstra
2018-02-01 14:55 ` David Woodhouse
2018-02-01 14:34 ` [PATCH 5/7] x86/paravirt: Annotate indirect calls Peter Zijlstra
` (4 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Peter Zijlstra @ 2018-02-01 14:34 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, Peter Zijlstra, David Woodhouse
[-- Attachment #1: peterz-retpoline-annotate-nospec.patch --]
[-- Type: text/plain, Size: 2358 bytes --]
Annotate the indirect calls/jumps in the CALL_NOSPEC/JUMP_NOSPEC
alternatives.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/x86/include/asm/nospec-branch.h | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -67,6 +67,18 @@
.endm
/*
+ * This should be used immediately before an indirect jump/call. It tells
+ * objtool the subsequent indirect jump/call is vouched safe for retpoline
+ * builds.
+ */
+.macro ANNOTATE_RETPOLINE_SAFE
+ .Lannotate_\@:
+ .pushsection .discard.retpoline_safe
+ _ASM_PTR .Lannotate_\@
+ .popsection
+.endm
+
+/*
* These are the bare retpoline primitives for indirect jmp and call.
* Do not use these directly; they only exist to make the ALTERNATIVE
* invocation below less ugly.
@@ -102,9 +114,9 @@
.macro JMP_NOSPEC reg:req
#ifdef CONFIG_RETPOLINE
ANNOTATE_NOSPEC_ALTERNATIVE
- ALTERNATIVE_2 __stringify(jmp *\reg), \
+ ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *\reg), \
__stringify(RETPOLINE_JMP \reg), X86_FEATURE_RETPOLINE, \
- __stringify(lfence; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
+ __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
#else
jmp *\reg
#endif
@@ -113,9 +125,9 @@
.macro CALL_NOSPEC reg:req
#ifdef CONFIG_RETPOLINE
ANNOTATE_NOSPEC_ALTERNATIVE
- ALTERNATIVE_2 __stringify(call *\reg), \
+ ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; call *\reg), \
__stringify(RETPOLINE_CALL \reg), X86_FEATURE_RETPOLINE,\
- __stringify(lfence; call *\reg), X86_FEATURE_RETPOLINE_AMD
+ __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; call *\reg), X86_FEATURE_RETPOLINE_AMD
#else
call *\reg
#endif
@@ -143,6 +155,12 @@
".long 999b - .\n\t" \
".popsection\n\t"
+#define ANNOTATE_RETPOLINE_SAFE \
+ "999:\n\t" \
+ ".pushsection .discard.retpoline_safe\n\t" \
+ _ASM_PTR " 999b\n\t" \
+ ".popsection\n\t"
+
#if defined(CONFIG_X86_64) && defined(RETPOLINE)
/*
@@ -152,6 +170,7 @@
# define CALL_NOSPEC \
ANNOTATE_NOSPEC_ALTERNATIVE \
ALTERNATIVE( \
+ ANNOTATE_RETPOLINE_SAFE \
"call *%[thunk_target]\n", \
"call __x86_indirect_thunk_%V[thunk_target]\n", \
X86_FEATURE_RETPOLINE)
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 4/7] x86,nospec: Annotate indirect calls/jumps
2018-02-01 14:34 ` [PATCH 4/7] x86,nospec: Annotate indirect calls/jumps Peter Zijlstra
@ 2018-02-01 14:55 ` David Woodhouse
2018-02-01 15:11 ` Peter Zijlstra
0 siblings, 1 reply; 24+ messages in thread
From: David Woodhouse @ 2018-02-01 14:55 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
[-- Attachment #1: Type: text/plain, Size: 1044 bytes --]
On Thu, 2018-02-01 at 15:34 +0100, Peter Zijlstra wrote:
>
> * These are the bare retpoline primitives for indirect jmp and call.
> * Do not use these directly; they only exist to make the ALTERNATIVE
> * invocation below less ugly.
> @@ -102,9 +114,9 @@
> .macro JMP_NOSPEC reg:req
> #ifdef CONFIG_RETPOLINE
> ANNOTATE_NOSPEC_ALTERNATIVE
> - ALTERNATIVE_2 __stringify(jmp *\reg), \
> + ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *\reg), \
> __stringify(RETPOLINE_JMP \reg), X86_FEATURE_RETPOLINE, \
> - __stringify(lfence; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
> + __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
> #else
> jmp *\reg
> #endif
The first one, yes. But the second one for the AMD retpoline is
redundant, isn't it? Objtool isn't going to look there.
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/7] x86,nospec: Annotate indirect calls/jumps
2018-02-01 14:55 ` David Woodhouse
@ 2018-02-01 15:11 ` Peter Zijlstra
2018-02-01 15:13 ` Peter Zijlstra
0 siblings, 1 reply; 24+ messages in thread
From: Peter Zijlstra @ 2018-02-01 15:11 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
On Thu, Feb 01, 2018 at 02:55:26PM +0000, David Woodhouse wrote:
>
>
> On Thu, 2018-02-01 at 15:34 +0100, Peter Zijlstra wrote:
> >
> > * These are the bare retpoline primitives for indirect jmp and call.
> > * Do not use these directly; they only exist to make the ALTERNATIVE
> > * invocation below less ugly.
> > @@ -102,9 +114,9 @@
> > .macro JMP_NOSPEC reg:req
> > #ifdef CONFIG_RETPOLINE
> > ANNOTATE_NOSPEC_ALTERNATIVE
> > - ALTERNATIVE_2 __stringify(jmp *\reg), \
> > + ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *\reg), \
> > __stringify(RETPOLINE_JMP \reg), X86_FEATURE_RETPOLINE, \
> > - __stringify(lfence; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
> > + __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
> > #else
> > jmp *\reg
> > #endif
>
> The first one, yes. But the second one for the AMD retpoline is
> redundant, isn't it? Objtool isn't going to look there.
It was when I wrote it.. lemme try again.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/7] x86,nospec: Annotate indirect calls/jumps
2018-02-01 15:11 ` Peter Zijlstra
@ 2018-02-01 15:13 ` Peter Zijlstra
2018-02-01 15:21 ` Josh Poimboeuf
0 siblings, 1 reply; 24+ messages in thread
From: Peter Zijlstra @ 2018-02-01 15:13 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
On Thu, Feb 01, 2018 at 04:11:36PM +0100, Peter Zijlstra wrote:
> On Thu, Feb 01, 2018 at 02:55:26PM +0000, David Woodhouse wrote:
> >
> >
> > On Thu, 2018-02-01 at 15:34 +0100, Peter Zijlstra wrote:
> > >
> > > * These are the bare retpoline primitives for indirect jmp and call.
> > > * Do not use these directly; they only exist to make the ALTERNATIVE
> > > * invocation below less ugly.
> > > @@ -102,9 +114,9 @@
> > > .macro JMP_NOSPEC reg:req
> > > #ifdef CONFIG_RETPOLINE
> > > ANNOTATE_NOSPEC_ALTERNATIVE
> > > - ALTERNATIVE_2 __stringify(jmp *\reg), \
> > > + ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *\reg), \
> > > __stringify(RETPOLINE_JMP \reg), X86_FEATURE_RETPOLINE, \
> > > - __stringify(lfence; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
> > > + __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
> > > #else
> > > jmp *\reg
> > > #endif
> >
> > The first one, yes. But the second one for the AMD retpoline is
> > redundant, isn't it? Objtool isn't going to look there.
>
> It was when I wrote it.. lemme try again.
Insta complaint:
arch/x86/entry/.tmp_entry_64.o: warning: objtool: .altinstr_replacement+0x19: indirect jump found in RETPOLINE build
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/7] x86,nospec: Annotate indirect calls/jumps
2018-02-01 15:13 ` Peter Zijlstra
@ 2018-02-01 15:21 ` Josh Poimboeuf
2018-02-01 15:30 ` Peter Zijlstra
0 siblings, 1 reply; 24+ messages in thread
From: Josh Poimboeuf @ 2018-02-01 15:21 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
On Thu, Feb 01, 2018 at 04:13:48PM +0100, Peter Zijlstra wrote:
> On Thu, Feb 01, 2018 at 04:11:36PM +0100, Peter Zijlstra wrote:
> > On Thu, Feb 01, 2018 at 02:55:26PM +0000, David Woodhouse wrote:
> > >
> > >
> > > On Thu, 2018-02-01 at 15:34 +0100, Peter Zijlstra wrote:
> > > >
> > > > * These are the bare retpoline primitives for indirect jmp and call.
> > > > * Do not use these directly; they only exist to make the ALTERNATIVE
> > > > * invocation below less ugly.
> > > > @@ -102,9 +114,9 @@
> > > > .macro JMP_NOSPEC reg:req
> > > > #ifdef CONFIG_RETPOLINE
> > > > ANNOTATE_NOSPEC_ALTERNATIVE
> > > > - ALTERNATIVE_2 __stringify(jmp *\reg), \
> > > > + ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *\reg), \
> > > > __stringify(RETPOLINE_JMP \reg), X86_FEATURE_RETPOLINE, \
> > > > - __stringify(lfence; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
> > > > + __stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
> > > > #else
> > > > jmp *\reg
> > > > #endif
> > >
> > > The first one, yes. But the second one for the AMD retpoline is
> > > redundant, isn't it? Objtool isn't going to look there.
> >
> > It was when I wrote it.. lemme try again.
>
> Insta complaint:
>
> arch/x86/entry/.tmp_entry_64.o: warning: objtool: .altinstr_replacement+0x19: indirect jump found in RETPOLINE build
Right, objtool was recently made smarter, such that it actually decodes
the ignored alternatives. The check for that warning needs to also
check that insn->ignore isn't set.
--
Josh
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/7] x86,nospec: Annotate indirect calls/jumps
2018-02-01 15:21 ` Josh Poimboeuf
@ 2018-02-01 15:30 ` Peter Zijlstra
0 siblings, 0 replies; 24+ messages in thread
From: Peter Zijlstra @ 2018-02-01 15:30 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
On Thu, Feb 01, 2018 at 09:21:34AM -0600, Josh Poimboeuf wrote:
> On Thu, Feb 01, 2018 at 04:13:48PM +0100, Peter Zijlstra wrote:
> > arch/x86/entry/.tmp_entry_64.o: warning: objtool: .altinstr_replacement+0x19: indirect jump found in RETPOLINE build
>
> Right, objtool was recently made smarter, such that it actually decodes
> the ignored alternatives.
I think it always did, you just ignored the alternatives for the code
flow stuff.
> The check for that warning needs to also check that insn->ignore isn't
> set.
So I tried to keep the two annotations independent, thinking the code
flow ignore would eventually go away when we got smarter about it. It
even has a comment about that:
/*
* FIXME: For now, just ignore any alternatives which add retpolines. This is
* a temporary hack, as it doesn't allow ORC to unwind from inside a retpoline.
* But it at least allows objtool to understand the control flow *around* the
* retpoline.
*/
So I'm not seeing how making retpoline_safe depend on nospec_ignores is
a good thing.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 5/7] x86/paravirt: Annotate indirect calls
2018-02-01 14:34 [PATCH 0/7] objtool: retpoline validation Peter Zijlstra
` (3 preceding siblings ...)
2018-02-01 14:34 ` [PATCH 4/7] x86,nospec: Annotate indirect calls/jumps Peter Zijlstra
@ 2018-02-01 14:34 ` Peter Zijlstra
2018-02-01 14:34 ` [PATCH 6/7] x86: Annotate indirect jump in head_64.S Peter Zijlstra
` (3 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Peter Zijlstra @ 2018-02-01 14:34 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, Peter Zijlstra, David Woodhouse
[-- Attachment #1: peterz-retpoline-annotate-pv.patch --]
[-- Type: text/plain, Size: 3813 bytes --]
Paravirt emits indirect calls which get flagged by objtool retpoline
checks, annotate it away because all these indirect calls will be
patched out before we start userspace.
This patching happens through alternative_instructions() ->
apply_paravirt() -> pv_init_ops.patch() which will eventually end up
in paravirt_patch_default(). This function _will_ write direct
alternatives.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/x86/include/asm/paravirt.h | 17 +++++++++++++----
arch/x86/include/asm/paravirt_types.h | 5 ++++-
2 files changed, 17 insertions(+), 5 deletions(-)
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -7,6 +7,7 @@
#ifdef CONFIG_PARAVIRT
#include <asm/pgtable_types.h>
#include <asm/asm.h>
+#include <asm/nospec-branch.h>
#include <asm/paravirt_types.h>
@@ -879,23 +880,27 @@ extern void default_banner(void);
#define INTERRUPT_RETURN \
PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_iret), CLBR_NONE, \
- jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_iret))
+ ANNOTATE_RETPOLINE_SAFE; \
+ jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_iret);)
#define DISABLE_INTERRUPTS(clobbers) \
PARA_SITE(PARA_PATCH(pv_irq_ops, PV_IRQ_irq_disable), clobbers, \
PV_SAVE_REGS(clobbers | CLBR_CALLEE_SAVE); \
+ ANNOTATE_RETPOLINE_SAFE; \
call PARA_INDIRECT(pv_irq_ops+PV_IRQ_irq_disable); \
PV_RESTORE_REGS(clobbers | CLBR_CALLEE_SAVE);)
#define ENABLE_INTERRUPTS(clobbers) \
PARA_SITE(PARA_PATCH(pv_irq_ops, PV_IRQ_irq_enable), clobbers, \
PV_SAVE_REGS(clobbers | CLBR_CALLEE_SAVE); \
+ ANNOTATE_RETPOLINE_SAFE; \
call PARA_INDIRECT(pv_irq_ops+PV_IRQ_irq_enable); \
PV_RESTORE_REGS(clobbers | CLBR_CALLEE_SAVE);)
#ifdef CONFIG_X86_32
#define GET_CR0_INTO_EAX \
push %ecx; push %edx; \
+ ANNOTATE_RETPOLINE_SAFE; \
call PARA_INDIRECT(pv_cpu_ops+PV_CPU_read_cr0); \
pop %edx; pop %ecx
#else /* !CONFIG_X86_32 */
@@ -917,21 +922,25 @@ extern void default_banner(void);
*/
#define SWAPGS \
PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_swapgs), CLBR_NONE, \
- call PARA_INDIRECT(pv_cpu_ops+PV_CPU_swapgs) \
+ ANNOTATE_RETPOLINE_SAFE; \
+ call PARA_INDIRECT(pv_cpu_ops+PV_CPU_swapgs); \
)
#define GET_CR2_INTO_RAX \
- call PARA_INDIRECT(pv_mmu_ops+PV_MMU_read_cr2)
+ ANNOTATE_RETPOLINE_SAFE; \
+ call PARA_INDIRECT(pv_mmu_ops+PV_MMU_read_cr2);
#define USERGS_SYSRET64 \
PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_usergs_sysret64), \
CLBR_NONE, \
- jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_usergs_sysret64))
+ ANNOTATE_RETPOLINE_SAFE; \
+ jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_usergs_sysret64);)
#ifdef CONFIG_DEBUG_ENTRY
#define SAVE_FLAGS(clobbers) \
PARA_SITE(PARA_PATCH(pv_irq_ops, PV_IRQ_save_fl), clobbers, \
PV_SAVE_REGS(clobbers | CLBR_CALLEE_SAVE); \
+ ANNOTATE_RETPOLINE_SAFE; \
call PARA_INDIRECT(pv_irq_ops+PV_IRQ_save_fl); \
PV_RESTORE_REGS(clobbers | CLBR_CALLEE_SAVE);)
#endif
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -43,6 +43,7 @@
#include <asm/desc_defs.h>
#include <asm/kmap_types.h>
#include <asm/pgtable_types.h>
+#include <asm/nospec-branch.h>
struct page;
struct thread_struct;
@@ -392,7 +393,9 @@ int paravirt_disable_iospace(void);
* offset into the paravirt_patch_template structure, and can therefore be
* freely converted back into a structure offset.
*/
-#define PARAVIRT_CALL "call *%c[paravirt_opptr];"
+#define PARAVIRT_CALL \
+ ANNOTATE_RETPOLINE_SAFE \
+ "call *%c[paravirt_opptr];"
/*
* These macros are intended to wrap calls through one of the paravirt
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 6/7] x86: Annotate indirect jump in head_64.S
2018-02-01 14:34 [PATCH 0/7] objtool: retpoline validation Peter Zijlstra
` (4 preceding siblings ...)
2018-02-01 14:34 ` [PATCH 5/7] x86/paravirt: Annotate indirect calls Peter Zijlstra
@ 2018-02-01 14:34 ` Peter Zijlstra
2018-02-01 14:34 ` [PATCH 7/7] x86,sme: Annotate indirect call Peter Zijlstra
` (2 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Peter Zijlstra @ 2018-02-01 14:34 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, Peter Zijlstra, David Woodhouse
[-- Attachment #1: peterz-objtool-retpoline-annotate.patch --]
[-- Type: text/plain, Size: 763 bytes --]
The objtool retpoline validation found this indirect jump. Seeing how
it's on CPU bringup before we run userspace it should be safe, annotate
it.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/x86/kernel/head_64.S | 2 ++
1 file changed, 2 insertions(+)
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -23,6 +23,7 @@
#include <asm/nops.h>
#include "../entry/calling.h"
#include <asm/export.h>
+#include <asm/nospec-branch.h>
#ifdef CONFIG_PARAVIRT
#include <asm/asm-offsets.h>
@@ -134,6 +135,7 @@ ENTRY(secondary_startup_64)
/* Ensure I am executing from virtual addresses */
movq $1f, %rax
+ ANNOTATE_RETPOLINE_SAFE
jmp *%rax
1:
UNWIND_HINT_EMPTY
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 7/7] x86,sme: Annotate indirect call
2018-02-01 14:34 [PATCH 0/7] objtool: retpoline validation Peter Zijlstra
` (5 preceding siblings ...)
2018-02-01 14:34 ` [PATCH 6/7] x86: Annotate indirect jump in head_64.S Peter Zijlstra
@ 2018-02-01 14:34 ` Peter Zijlstra
2018-02-01 15:28 ` [PATCH 0/7] objtool: retpoline validation Josh Poimboeuf
2018-02-01 15:50 ` Josh Poimboeuf
8 siblings, 0 replies; 24+ messages in thread
From: Peter Zijlstra @ 2018-02-01 14:34 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, 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] 24+ messages in thread* Re: [PATCH 0/7] objtool: retpoline validation
2018-02-01 14:34 [PATCH 0/7] objtool: retpoline validation Peter Zijlstra
` (6 preceding siblings ...)
2018-02-01 14:34 ` [PATCH 7/7] x86,sme: Annotate indirect call Peter Zijlstra
@ 2018-02-01 15:28 ` Josh Poimboeuf
2018-02-01 15:32 ` David Woodhouse
2018-02-01 15:32 ` Peter Zijlstra
2018-02-01 15:50 ` Josh Poimboeuf
8 siblings, 2 replies; 24+ messages in thread
From: Josh Poimboeuf @ 2018-02-01 15:28 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
On Thu, Feb 01, 2018 at 03:34:21PM +0100, Peter Zijlstra wrote:
> There are the retpoline validation patches; they work with the __noretpoline
> thing from David.
Have you run this through 0-day bot yet? A manual awk/sed found another
one, which objtool confirms:
drivers/watchdog/.tmp_hpwdt.o: warning: objtool: .text+0x24: indirect call found in RETPOLINE build
And my search wasn't exhaustive so it would be good to sic 0-day bot on
it.
--
Josh
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 0/7] objtool: retpoline validation
2018-02-01 15:28 ` [PATCH 0/7] objtool: retpoline validation Josh Poimboeuf
@ 2018-02-01 15:32 ` David Woodhouse
2018-02-01 15:40 ` Peter Zijlstra
2018-02-01 15:32 ` Peter Zijlstra
1 sibling, 1 reply; 24+ messages in thread
From: David Woodhouse @ 2018-02-01 15:32 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
[-- Attachment #1: Type: text/plain, Size: 751 bytes --]
On Thu, 2018-02-01 at 09:28 -0600, Josh Poimboeuf wrote:
> On Thu, Feb 01, 2018 at 03:34:21PM +0100, Peter Zijlstra wrote:
> >
> > There are the retpoline validation patches; they work with the
> > __noretpoline
> > thing from David.
> Have you run this through 0-day bot yet? A manual awk/sed found
> another
> one, which objtool confirms:
>
> drivers/watchdog/.tmp_hpwdt.o: warning: objtool: .text+0x24:
> indirect call found in RETPOLINE build
>
> And my search wasn't exhaustive so it would be good to sic 0-day bot on
> it.
We discussed that one. It's correct; we're calling into firmware so
there's *no* point in retpolining that one. We need to set IBRS before
any runtime calls into firmware, if we want to be safe.
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/7] objtool: retpoline validation
2018-02-01 15:32 ` David Woodhouse
@ 2018-02-01 15:40 ` Peter Zijlstra
2018-02-01 16:51 ` David Woodhouse
0 siblings, 1 reply; 24+ messages in thread
From: Peter Zijlstra @ 2018-02-01 15:40 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
On Thu, Feb 01, 2018 at 03:32:11PM +0000, David Woodhouse wrote:
> On Thu, 2018-02-01 at 09:28 -0600, Josh Poimboeuf wrote:
> > On Thu, Feb 01, 2018 at 03:34:21PM +0100, Peter Zijlstra wrote:
> > >
> > > There are the retpoline validation patches; they work with the
> > > __noretpoline
> > > thing from David.
> > Have you run this through 0-day bot yet? A manual awk/sed found
> > another
> > one, which objtool confirms:
> >
> > drivers/watchdog/.tmp_hpwdt.o: warning: objtool: .text+0x24:
> > indirect call found in RETPOLINE build
> >
> > And my search wasn't exhaustive so it would be good to sic 0-day bot on
> > it.
>
> We discussed that one. It's correct; we're calling into firmware so
> there's *no* point in retpolining that one. We need to set IBRS before
> any runtime calls into firmware, if we want to be safe.
Ideally we'd have a way to mark the module 'unsafe' or something.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/7] objtool: retpoline validation
2018-02-01 15:40 ` Peter Zijlstra
@ 2018-02-01 16:51 ` David Woodhouse
2018-02-01 17:14 ` Peter Zijlstra
2018-02-01 18:16 ` Tim Chen
0 siblings, 2 replies; 24+ messages in thread
From: David Woodhouse @ 2018-02-01 16:51 UTC (permalink / raw)
To: Peter Zijlstra
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
[-- Attachment #1: Type: text/plain, Size: 1244 bytes --]
On Thu, 2018-02-01 at 16:40 +0100, Peter Zijlstra wrote:
> On Thu, Feb 01, 2018 at 03:32:11PM +0000, David Woodhouse wrote:
> >
> > On Thu, 2018-02-01 at 09:28 -0600, Josh Poimboeuf wrote:
> > >
> > > On Thu, Feb 01, 2018 at 03:34:21PM +0100, Peter Zijlstra wrote:
> > > >
> > > >
> > > > There are the retpoline validation patches; they work with the
> > > > __noretpoline
> > > > thing from David.
> > > Have you run this through 0-day bot yet? A manual awk/sed found
> > > another
> > > one, which objtool confirms:
> > >
> > > drivers/watchdog/.tmp_hpwdt.o: warning: objtool: .text+0x24:
> > > indirect call found in RETPOLINE build
> > >
> > > And my search wasn't exhaustive so it would be good to sic 0-day bot on
> > > it.
> > We discussed that one. It's correct; we're calling into firmware so
> > there's *no* point in retpolining that one. We need to set IBRS before
> > any runtime calls into firmware, if we want to be safe.
>
> Ideally we'd have a way to mark the module 'unsafe' or something.
No, we just need to set IBRS before doing it. The same applies to any
EFI runtime calls, APM and all kinds of other random crap that calls
into firmware. I'm not sure why those aren't showing up.
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/7] objtool: retpoline validation
2018-02-01 16:51 ` David Woodhouse
@ 2018-02-01 17:14 ` Peter Zijlstra
2018-02-01 17:43 ` Josh Poimboeuf
2018-02-01 18:16 ` Tim Chen
1 sibling, 1 reply; 24+ messages in thread
From: Peter Zijlstra @ 2018-02-01 17:14 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
On Thu, Feb 01, 2018 at 04:51:35PM +0000, David Woodhouse wrote:
> > Ideally we'd have a way to mark the module 'unsafe' or something.
>
> No, we just need to set IBRS before doing it.
That would work, assuming IBRS is available to begin with of course. Do
we WARN if we hit this code and don't have IBRS available?
> The same applies to any
> EFI runtime calls, APM and all kinds of other random crap that calls
> into firmware. I'm not sure why those aren't showing up.
arch/x86/platform/efi/Makefile:OBJECT_FILES_NON_STANDARD_efi_thunk_$(BITS).o := y
arch/x86/platform/efi/Makefile:OBJECT_FILES_NON_STANDARD_efi_stub_$(BITS).o := y
And similar things tell objtool to please not look..
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/7] objtool: retpoline validation
2018-02-01 17:14 ` Peter Zijlstra
@ 2018-02-01 17:43 ` Josh Poimboeuf
0 siblings, 0 replies; 24+ messages in thread
From: Josh Poimboeuf @ 2018-02-01 17:43 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
On Thu, Feb 01, 2018 at 06:14:27PM +0100, Peter Zijlstra wrote:
> On Thu, Feb 01, 2018 at 04:51:35PM +0000, David Woodhouse wrote:
> > > Ideally we'd have a way to mark the module 'unsafe' or something.
> >
> > No, we just need to set IBRS before doing it.
>
> That would work, assuming IBRS is available to begin with of course. Do
> we WARN if we hit this code and don't have IBRS available?
Perhaps it should just be reported in the spectre_v2 sysfs file.
So
"Mitigation: Full generic retpoline"
would instead be something like
"Vulnerable: Retpoline without IBRS"
?
> > The same applies to any
> > EFI runtime calls, APM and all kinds of other random crap that calls
> > into firmware. I'm not sure why those aren't showing up.
>
> arch/x86/platform/efi/Makefile:OBJECT_FILES_NON_STANDARD_efi_thunk_$(BITS).o := y
> arch/x86/platform/efi/Makefile:OBJECT_FILES_NON_STANDARD_efi_stub_$(BITS).o := y
>
> And similar things tell objtool to please not look..
Right, some of the corner cases like efi, vdso, and bpf tend to be
ignored by objtool right now.
--
Josh
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/7] objtool: retpoline validation
2018-02-01 16:51 ` David Woodhouse
2018-02-01 17:14 ` Peter Zijlstra
@ 2018-02-01 18:16 ` Tim Chen
2018-02-06 21:23 ` David Woodhouse
1 sibling, 1 reply; 24+ messages in thread
From: Tim Chen @ 2018-02-01 18:16 UTC (permalink / raw)
To: David Woodhouse, Peter Zijlstra
Cc: Josh Poimboeuf, Thomas Gleixner, linux-kernel, Dave Hansen,
Ashok Raj, Andy Lutomirski, Linus Torvalds, Greg KH,
Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
Paolo Bonzini, Jun Nakajima, Asit Mallick
On 02/01/2018 08:51 AM, David Woodhouse wrote:
>
> No, we just need to set IBRS before doing it. The same applies to any
> EFI runtime calls, APM and all kinds of other random crap that calls
> into firmware. I'm not sure why those aren't showing up.
>
Dave,
Are you planning to update your IBRS for firmware call patches
and repost those?
Tim
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/7] objtool: retpoline validation
2018-02-01 18:16 ` Tim Chen
@ 2018-02-06 21:23 ` David Woodhouse
0 siblings, 0 replies; 24+ messages in thread
From: David Woodhouse @ 2018-02-06 21:23 UTC (permalink / raw)
To: Tim Chen, Peter Zijlstra
Cc: Josh Poimboeuf, Thomas Gleixner, linux-kernel, Dave Hansen,
Ashok Raj, Andy Lutomirski, Linus Torvalds, Greg KH,
Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, Dan Williams,
Paolo Bonzini, Jun Nakajima, Asit Mallick
[-- Attachment #1: Type: text/plain, Size: 533 bytes --]
On Thu, 2018-02-01 at 10:16 -0800, Tim Chen wrote:
> On 02/01/2018 08:51 AM, David Woodhouse wrote:
> > No, we just need to set IBRS before doing it. The same applies to any
> > EFI runtime calls, APM and all kinds of other random crap that calls
> > into firmware. I'm not sure why those aren't showing up.
> >
> Dave,
>
> Are you planning to update your IBRS for firmware call patches
> and repost those?
Yep. In the tree now...
http://git.infradead.org/users/dwmw2/linux-retpoline.git/shortlog/refs/heads/ibpb
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/7] objtool: retpoline validation
2018-02-01 15:28 ` [PATCH 0/7] objtool: retpoline validation Josh Poimboeuf
2018-02-01 15:32 ` David Woodhouse
@ 2018-02-01 15:32 ` Peter Zijlstra
2018-02-01 19:36 ` Peter Zijlstra
1 sibling, 1 reply; 24+ messages in thread
From: Peter Zijlstra @ 2018-02-01 15:32 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
On Thu, Feb 01, 2018 at 09:28:56AM -0600, Josh Poimboeuf wrote:
> On Thu, Feb 01, 2018 at 03:34:21PM +0100, Peter Zijlstra wrote:
> > There are the retpoline validation patches; they work with the __noretpoline
> > thing from David.
>
> Have you run this through 0-day bot yet?
Yes, it complains a _lot_ because no retpoline supported compiler.
> A manual awk/sed found another
> one, which objtool confirms:
>
> drivers/watchdog/.tmp_hpwdt.o: warning: objtool: .text+0x24: indirect call found in RETPOLINE build
That is the only known one left. It calls into BIOS code, so its not
safe and using a retpoline for it is pointless since nobody audited the
BIOS code.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/7] objtool: retpoline validation
2018-02-01 15:32 ` Peter Zijlstra
@ 2018-02-01 19:36 ` Peter Zijlstra
0 siblings, 0 replies; 24+ messages in thread
From: Peter Zijlstra @ 2018-02-01 19:36 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
On Thu, Feb 01, 2018 at 04:32:35PM +0100, Peter Zijlstra wrote:
> On Thu, Feb 01, 2018 at 09:28:56AM -0600, Josh Poimboeuf wrote:
> > On Thu, Feb 01, 2018 at 03:34:21PM +0100, Peter Zijlstra wrote:
> > > There are the retpoline validation patches; they work with the __noretpoline
> > > thing from David.
> >
> > Have you run this through 0-day bot yet?
>
> Yes, it complains a _lot_ because no retpoline supported compiler.
I think I finally got something that works there... Damn I hate
makefiles..
--- a/Makefile
+++ b/Makefile
@@ -486,6 +486,11 @@ KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG
KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC)
endif
+ifneq ($(call cc-option,-mindirect-branch=thunk-extern -mindirect-branch-register),)
+ CC_HAS_RETPOLINE := 1
+endif
+export CC_HAS_RETPOLINE
+
ifeq ($(config-targets),1)
# ===========================================================================
# *config targets only - make sure prerequisites are updated, and descend
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -267,8 +267,10 @@ else
objtool_args += $(call cc-ifversion, -lt, 0405, --no-unreachable)
endif
ifdef CONFIG_RETPOLINE
+ifdef CC_HAS_RETPOLINE
objtool_args += --retpoline
endif
+endif
ifdef CONFIG_MODVERSIONS
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/7] objtool: retpoline validation
2018-02-01 14:34 [PATCH 0/7] objtool: retpoline validation Peter Zijlstra
` (7 preceding siblings ...)
2018-02-01 15:28 ` [PATCH 0/7] objtool: retpoline validation Josh Poimboeuf
@ 2018-02-01 15:50 ` Josh Poimboeuf
8 siblings, 0 replies; 24+ messages in thread
From: Josh Poimboeuf @ 2018-02-01 15:50 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
On Thu, Feb 01, 2018 at 03:34:21PM +0100, Peter Zijlstra wrote:
> There are the retpoline validation patches; they work with the __noretpoline
> thing from David.
For the series:
Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
--
Josh
^ permalink raw reply [flat|nested] 24+ messages in thread