Now that objtool is able to rewrite jump_label instructions, have the
compiler emit a JMP, such that it can decide on the optimal encoding,
and set jump_entry::key bit1 to indicate that objtool should rewrite
the instruction to a matching NOP.
For x86_64-allyesconfig this gives:
jl\ NOP JMP
short: 22997 124
long: 30874 90
IOW, we save (22997+124) * 3 bytes of kernel text in hotpaths.
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
arch/x86/include/asm/jump_label.h | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -28,6 +28,22 @@
_ASM_PTR "%c0 + %c1 - .\n\t" \
".popsection \n\t"
+#ifdef CONFIG_STACK_VALIDATION
+
+static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
+{
+ asm_volatile_goto("1:"
+ "jmp %l[l_yes] # objtool NOPs this \n\t"
+ JUMP_TABLE_ENTRY
+ : : "i" (key), "i" (2 | branch) : : l_yes);
+
+ return false;
+l_yes:
+ return true;
+}
+
+#else
+
static __always_inline bool arch_static_branch(struct static_key * const key, const bool branch)
{
asm_volatile_goto("1:"
@@ -40,6 +56,8 @@ static __always_inline bool arch_static_
return true;
}
+#endif /* STACK_VALIDATION */
+
static __always_inline bool arch_static_branch_jump(struct static_key * const key, const bool branch)
{
asm_volatile_goto("1:"
The following commit has been merged into the objtool/core branch of tip:
Commit-ID: ab3257042c26d0cd44793c741e2f89bf38b21fe8
Gitweb: https://git.kernel.org/tip/ab3257042c26d0cd44793c741e2f89bf38b21fe8
Author: Peter Zijlstra <[email protected]>
AuthorDate: Thu, 06 May 2021 21:34:05 +02:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 12 May 2021 14:54:56 +02:00
jump_label, x86: Allow short NOPs
Now that objtool is able to rewrite jump_label instructions, have the
compiler emit a JMP, such that it can decide on the optimal encoding,
and set jump_entry::key bit1 to indicate that objtool should rewrite
the instruction to a matching NOP.
For x86_64-allyesconfig this gives:
jl\ NOP JMP
short: 22997 124
long: 30874 90
IOW, we save (22997+124) * 3 bytes of kernel text in hotpaths.
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/jump_label.h | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index ef819e3..0449b12 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -20,6 +20,22 @@
_ASM_PTR "%c0 + %c1 - .\n\t" \
".popsection \n\t"
+#ifdef CONFIG_STACK_VALIDATION
+
+static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
+{
+ asm_volatile_goto("1:"
+ "jmp %l[l_yes] # objtool NOPs this \n\t"
+ JUMP_TABLE_ENTRY
+ : : "i" (key), "i" (2 | branch) : : l_yes);
+
+ return false;
+l_yes:
+ return true;
+}
+
+#else
+
static __always_inline bool arch_static_branch(struct static_key * const key, const bool branch)
{
asm_volatile_goto("1:"
@@ -32,6 +48,8 @@ l_yes:
return true;
}
+#endif /* STACK_VALIDATION */
+
static __always_inline bool arch_static_branch_jump(struct static_key * const key, const bool branch)
{
asm_volatile_goto("1:"
On Wed, May 12, 2021 at 01:19:47PM -0000, tip-bot2 for Peter Zijlstra wrote:
> The following commit has been merged into the objtool/core branch of tip:
>
> Commit-ID: ab3257042c26d0cd44793c741e2f89bf38b21fe8
> Gitweb: https://git.kernel.org/tip/ab3257042c26d0cd44793c741e2f89bf38b21fe8
> Author: Peter Zijlstra <[email protected]>
> AuthorDate: Thu, 06 May 2021 21:34:05 +02:00
> Committer: Ingo Molnar <[email protected]>
> CommitterDate: Wed, 12 May 2021 14:54:56 +02:00
>
> jump_label, x86: Allow short NOPs
>
> Now that objtool is able to rewrite jump_label instructions, have the
> compiler emit a JMP, such that it can decide on the optimal encoding,
> and set jump_entry::key bit1 to indicate that objtool should rewrite
> the instruction to a matching NOP.
>
> For x86_64-allyesconfig this gives:
>
> jl\ NOP JMP
> short: 22997 124
> long: 30874 90
>
> IOW, we save (22997+124) * 3 bytes of kernel text in hotpaths.
>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> Signed-off-by: Ingo Molnar <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
So Willy is having some trouble with this commit; for some reason his
kernel is no longer booting in his qemu thing, but I can't reproduce.
I've hacked up the below vmlinux.o validation, willy can you run this on
your vmlinux.o, something like:
build/tools/objtool/objtool check -abdJsuld build/vmlinux.o
Where I'm assuming you build with O=build/. When I run it on my build
(with your .config) I get absolutely nothing :/
Alternatively, can you get me your vmlinux.o + bzImage ?
Also helpful might be trying to attach gdb to the qemu gdbstub and
looking where the boot fails.
---
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 8b38b5d6fec7..100f3efa6136 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -20,7 +20,7 @@
#include <objtool/objtool.h>
bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats,
- validate_dup, vmlinux, mcount, noinstr, backup;
+ validate_dup, vmlinux, mcount, noinstr, backup, validate_jl;
static const char * const check_usage[] = {
"objtool check [<options>] file.o",
@@ -45,6 +45,7 @@ const struct option check_options[] = {
OPT_BOOLEAN('l', "vmlinux", &vmlinux, "vmlinux.o validation"),
OPT_BOOLEAN('M', "mcount", &mcount, "generate __mcount_loc"),
OPT_BOOLEAN('B', "backup", &backup, "create .orig files before modification"),
+ OPT_BOOLEAN('J', "jump-label", &validate_jl, "validate jump-label tables"),
OPT_END(),
};
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 2c6a93edf27e..c3c82e40cbee 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1225,6 +1225,33 @@ static int handle_jump_alt(struct objtool_file *file,
struct instruction *orig_insn,
struct instruction **new_insn)
{
+ if (validate_jl) {
+#if 0
+ if (special_alt->key_addend & 2) {
+ WARN_FUNC("jump-label mod: %s", orig_insn->sec, orig_insn->offset,
+ orig_insn->type == INSN_NOP ? "nop" : "jmp");
+ }
+#endif
+
+ if (orig_insn->len == 2) {
+ s32 disp;
+
+ if (special_alt->orig_sec != special_alt->new_sec) {
+ WARN_FUNC("short jump-label cannot cross sections",
+ orig_insn->sec, orig_insn->offset);
+ return -1;
+ }
+
+ disp = special_alt->new_off - (special_alt->orig_off + 2);
+
+ if ((disp >> 31) != (disp >> 7)) {
+ WARN_FUNC("short jump-label, displacement too large: 0x%08x",
+ orig_insn->sec, orig_insn->offset, disp);
+ return -1;
+ }
+ }
+ }
+
if (orig_insn->type == INSN_NOP) {
do_nop:
if (orig_insn->len == 2)
@@ -1244,6 +1271,11 @@ static int handle_jump_alt(struct objtool_file *file,
if (special_alt->key_addend & 2) {
struct reloc *reloc = insn_reloc(file, orig_insn);
+ if (validate_jl) {
+ WARN_FUNC("jump-label unpatched", orig_insn->sec, orig_insn->offset);
+ return -1;
+ }
+
if (reloc) {
reloc->type = R_NONE;
elf_write_reloc(file->elf, reloc);
@@ -1341,6 +1373,8 @@ static int add_special_section_alts(struct objtool_file *file)
}
if (stats) {
+ if (validate_jl)
+ printf("validate-");
printf("jl\\\tNOP\tJMP\n");
printf("short:\t%ld\t%ld\n", file->jl_nop_short, file->jl_short);
printf("long:\t%ld\t%ld\n", file->jl_nop_long, file->jl_long);
diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h
index 15ac0b7d3d6a..c9a00423ebd5 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -9,7 +9,7 @@
extern const struct option check_options[];
extern bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats,
- validate_dup, vmlinux, mcount, noinstr, backup;
+ validate_dup, vmlinux, mcount, noinstr, backup, validate_jl;
extern int cmd_parse_options(int argc, const char **argv, const char * const usage[]);
+kbuild maintainers
On Tue, May 18, 2021 at 09:50:04PM +0200, Peter Zijlstra wrote:
> On Wed, May 12, 2021 at 01:19:47PM -0000, tip-bot2 for Peter Zijlstra wrote:
> > The following commit has been merged into the objtool/core branch of tip:
> >
> > Commit-ID: ab3257042c26d0cd44793c741e2f89bf38b21fe8
> > Gitweb: https://git.kernel.org/tip/ab3257042c26d0cd44793c741e2f89bf38b21fe8
> > Author: Peter Zijlstra <[email protected]>
> > AuthorDate: Thu, 06 May 2021 21:34:05 +02:00
> > Committer: Ingo Molnar <[email protected]>
> > CommitterDate: Wed, 12 May 2021 14:54:56 +02:00
> >
> > jump_label, x86: Allow short NOPs
> >
> > Now that objtool is able to rewrite jump_label instructions, have the
> > compiler emit a JMP, such that it can decide on the optimal encoding,
> > and set jump_entry::key bit1 to indicate that objtool should rewrite
> > the instruction to a matching NOP.
> >
> > For x86_64-allyesconfig this gives:
> >
> > jl\ NOP JMP
> > short: 22997 124
> > long: 30874 90
> >
> > IOW, we save (22997+124) * 3 bytes of kernel text in hotpaths.
> >
> > Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> > Signed-off-by: Ingo Molnar <[email protected]>
> > Link: https://lore.kernel.org/r/[email protected]
>
> So Willy is having some trouble with this commit; for some reason his
> kernel is no longer booting in his qemu thing, but I can't reproduce.
>
> I've hacked up the below vmlinux.o validation, willy can you run this on
> your vmlinux.o, something like:
>
> build/tools/objtool/objtool check -abdJsuld build/vmlinux.o
>
> Where I'm assuming you build with O=build/. When I run it on my build
> (with your .config) I get absolutely nothing :/
>
> Alternatively, can you get me your vmlinux.o + bzImage ?
>
> Also helpful might be trying to attach gdb to the qemu gdbstub and
> looking where the boot fails.
OK, willy followed up on IRC, and it turns out there's a kbuild
dependency missing; then objtool changes we don't rebuild:
arch/x86/entry/vdso/vma.o
even though we should, this led to an unpatched 2 byte jump-label and
things went sideways. I'm not sure I understand the whole build
machinery well enough to know where to begin chasing this.
Now, this file is mighty magical, due to:
arch/x86/entry/vdso/Makefile:OBJECT_FILES_NON_STANDARD := y
arch/x86/entry/vdso/Makefile:OBJECT_FILES_NON_STANDARD_vma.o := n
Maybe that's related.
On Tue, May 18, 2021 at 10:24:43PM +0200, Peter Zijlstra wrote:
> OK, willy followed up on IRC, and it turns out there's a kbuild
> dependency missing; then objtool changes we don't rebuild:
>
> arch/x86/entry/vdso/vma.o
>
> even though we should, this led to an unpatched 2 byte jump-label and
> things went sideways. I'm not sure I understand the whole build
> machinery well enough to know where to begin chasing this.
>
> Now, this file is mighty magical, due to:
>
> arch/x86/entry/vdso/Makefile:OBJECT_FILES_NON_STANDARD := y
> arch/x86/entry/vdso/Makefile:OBJECT_FILES_NON_STANDARD_vma.o := n
>
> Maybe that's related.
I'm not exactly thrilled that objtool now has the power to easily brick
a system :-/ Is it really worth it?
Anyway, here's one way to fix it. Maybe Masahiro has a better idea.
From f88b208677953bc445db08ac46b6e4259217bb8a Mon Sep 17 00:00:00 2001
Message-Id: <f88b208677953bc445db08ac46b6e4259217bb8a.1621384807.git.jpoimboe@redhat.com>
From: Josh Poimboeuf <[email protected]>
Date: Tue, 18 May 2021 18:59:15 -0500
Subject: [PATCH] kbuild: Fix objtool dependency for
'OBJECT_FILES_NON_STANDARD_<obj> := n'
"OBJECT_FILES_NON_STANDARD_vma.o := n" has a dependency bug. When
objtool source is updated, the affected object doesn't get re-analyzed
by objtool.
Peter's new variable-sized jump label feature relies on objtool
rewriting the object file. Otherwise the system can fail to boot. That
effectively upgrades this minor dependency issue to a major bug.
The problem is that variables in prerequisites are expanded early,
during the read-in phase. The '$(objtool_dep)' variable indirectly uses
'$@', which isn't yet available when the target prerequisites are
evaluated.
Use '.SECONDEXPANSION:' which causes '$(objtool_dep)' to be expanded in
a later phase, after the target-specific '$@' variable has been defined.
Fixes: b9ab5ebb14ec ("objtool: Add CONFIG_STACK_VALIDATION option")
Fixes: ab3257042c26 ("jump_label, x86: Allow short NOPs")
Reported-by: Matthew Wilcox <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
---
scripts/Makefile.build | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 949f723efe53..34d257653fb4 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -268,7 +268,8 @@ define rule_as_o_S
endef
# Built-in and composite module parts
-$(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
+.SECONDEXPANSION:
+$(obj)/%.o: $(src)/%.c $(recordmcount_source) $$(objtool_dep) FORCE
$(call if_changed_rule,cc_o_c)
$(call cmd,force_checksrc)
@@ -349,7 +350,7 @@ cmd_modversions_S = \
fi
endif
-$(obj)/%.o: $(src)/%.S $(objtool_dep) FORCE
+$(obj)/%.o: $(src)/%.S $$(objtool_dep) FORCE
$(call if_changed_rule,as_o_S)
targets += $(filter-out $(subdir-builtin), $(real-obj-y))
--
2.31.1
On Tue, May 18, 2021 at 07:44:11PM -0500, Josh Poimboeuf wrote:
> I'm not exactly thrilled that objtool now has the power to easily brick
> a system :-/ Is it really worth it?
The way I look at it is that not running objtool is a bug either way,
bricking a system is ofcourse a somewhat more drastic failure mode than
missing ORC info for example, but neither are good.
As to worth, about half the jump labels are shorter now, this reduces I$
pressure on hot paths. Any little thing to offset the ever increasing
bulk seems like a good thing to me. But yes, it would be nice if the
assemblers wouldn't suck so bad and this wouldn't need objtool :/ But
I've tried poking the tools guys and they don't really seem interested
:-(
Also, only dirty builds are affected here; clean builds (always
recommended afaik, because dep trouble isn't unheard of) are fine.
> Anyway, here's one way to fix it. Maybe Masahiro has a better idea.
Thanks! lemme go read up on this magic :-)
So this got merged without the corresponding Kbuild update being merged,
and my kernel failed to boot. Bisect got as far as
$ git bisect good
Bisecting: 4 revisions left to test after this (roughly 2 steps)
[ab3257042c26d0cd44793c741e2f89bf38b21fe8] jump_label, x86: Allow short NOPs
before my sluggish memory remembered this thread from six weeks ago.
So if anybody else hits this, do a make clean.
On Wed, May 19, 2021 at 08:56:33AM +0200, Peter Zijlstra wrote:
> On Tue, May 18, 2021 at 07:44:11PM -0500, Josh Poimboeuf wrote:
>
> > I'm not exactly thrilled that objtool now has the power to easily brick
> > a system :-/ Is it really worth it?
>
> The way I look at it is that not running objtool is a bug either way,
> bricking a system is ofcourse a somewhat more drastic failure mode than
> missing ORC info for example, but neither are good.
>
> As to worth, about half the jump labels are shorter now, this reduces I$
> pressure on hot paths. Any little thing to offset the ever increasing
> bulk seems like a good thing to me. But yes, it would be nice if the
> assemblers wouldn't suck so bad and this wouldn't need objtool :/ But
> I've tried poking the tools guys and they don't really seem interested
> :-(
>
> Also, only dirty builds are affected here; clean builds (always
> recommended afaik, because dep trouble isn't unheard of) are fine.
>
> > Anyway, here's one way to fix it. Maybe Masahiro has a better idea.
>
> Thanks! lemme go read up on this magic :-)
On Tue, Jun 29, 2021 at 09:01:25PM +0100, Matthew Wilcox wrote:
> So this got merged without the corresponding Kbuild update being merged,
> and my kernel failed to boot. Bisect got as far as
>
> $ git bisect good
> Bisecting: 4 revisions left to test after this (roughly 2 steps)
> [ab3257042c26d0cd44793c741e2f89bf38b21fe8] jump_label, x86: Allow short NOPs
>
> before my sluggish memory remembered this thread from six weeks ago.
>
> So if anybody else hits this, do a make clean.
Actually, this is a different bug with the same symptom.
Applying the patch from Peter, and running it:
$ ./.build_test_kernel-x86_64/tools/objtool/objtool check -abdJsuld .build_test_kernel-x86_64/vmlinux.o
nr_sections: 15446
section_bits: 13
nr_symbols: 116448
symbol_bits: 16
max_reloc: 8031700
tot_reloc: 12477754
reloc_bits: 19
nr_insns: 2523443
.build_test_kernel-x86_64/vmlinux.o: warning: objtool: want_init_on_free()+0x0: jump-label unpatched
This is against a freshly built kernel -- i removed the build directory,
copied in a .config file and built a fresh kernel.
On Tue, Jun 29, 2021 at 09:35:37PM +0100, Matthew Wilcox wrote:
> On Tue, Jun 29, 2021 at 09:01:25PM +0100, Matthew Wilcox wrote:
> > So this got merged without the corresponding Kbuild update being merged,
> > and my kernel failed to boot. Bisect got as far as
> >
> > $ git bisect good
> > Bisecting: 4 revisions left to test after this (roughly 2 steps)
> > [ab3257042c26d0cd44793c741e2f89bf38b21fe8] jump_label, x86: Allow short NOPs
> >
> > before my sluggish memory remembered this thread from six weeks ago.
> >
> > So if anybody else hits this, do a make clean.
>
> Actually, this is a different bug with the same symptom.
>
> Applying the patch from Peter, and running it:
>
> $ ./.build_test_kernel-x86_64/tools/objtool/objtool check -abdJsuld .build_test_kernel-x86_64/vmlinux.o
> nr_sections: 15446
> section_bits: 13
> nr_symbols: 116448
> symbol_bits: 16
> max_reloc: 8031700
> tot_reloc: 12477754
> reloc_bits: 19
> nr_insns: 2523443
> .build_test_kernel-x86_64/vmlinux.o: warning: objtool: want_init_on_free()+0x0: jump-label unpatched
>
> This is against a freshly built kernel -- i removed the build directory,
> copied in a .config file and built a fresh kernel.
You happen to have said .config for me?
On Wed, Jun 30, 2021 at 09:07:05AM +0200, Peter Zijlstra wrote:
> On Tue, Jun 29, 2021 at 09:35:37PM +0100, Matthew Wilcox wrote:
> > On Tue, Jun 29, 2021 at 09:01:25PM +0100, Matthew Wilcox wrote:
> > > So this got merged without the corresponding Kbuild update being merged,
> > > and my kernel failed to boot. Bisect got as far as
> > >
> > > $ git bisect good
> > > Bisecting: 4 revisions left to test after this (roughly 2 steps)
> > > [ab3257042c26d0cd44793c741e2f89bf38b21fe8] jump_label, x86: Allow short NOPs
> > >
> > > before my sluggish memory remembered this thread from six weeks ago.
> > >
> > > So if anybody else hits this, do a make clean.
> >
> > Actually, this is a different bug with the same symptom.
> >
> > Applying the patch from Peter, and running it:
> >
> > $ ./.build_test_kernel-x86_64/tools/objtool/objtool check -abdJsuld .build_test_kernel-x86_64/vmlinux.o
> > nr_sections: 15446
> > section_bits: 13
> > nr_symbols: 116448
> > symbol_bits: 16
> > max_reloc: 8031700
> > tot_reloc: 12477754
> > reloc_bits: 19
> > nr_insns: 2523443
> > .build_test_kernel-x86_64/vmlinux.o: warning: objtool: want_init_on_free()+0x0: jump-label unpatched
> >
> > This is against a freshly built kernel -- i removed the build directory,
> > copied in a .config file and built a fresh kernel.
>
> You happen to have said .config for me?
Also GCC version I suppose. The thing I'm wondering about in particular
is what translation unit is responsible for that symbol.
AFAICT the function itself is an inline from linux/mm.h, but I cannot
find any of the files or functions it's used in as being excluded from
objtool coverage :/