objtool fixes

GIT PULL In review Ingo Molnar 5 participants 12 messages Feb 28, 2017 – Mar 1, 2017

Subsystems (3)

x86 Kbuild objtool

lore

Share & tools

lore

Ingo Molnar

author

2017-02-28 07:53 UTC #0 lore raw

Linus,

Please pull the latest core-urgent-for-linus git tree from:

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core-urgent-for-linus

# HEAD: 4e4636cf981b5b629fbfb78aa9f232e015f7d521 objtool: Enclose contents of unreachable() macro in a block

A handful of objtool fixes related to unreachable code, plus a build fix for out of tree modules.

Thanks,

Ingo

------------------>
Josh Poimboeuf (4):
objtool: Fix CONFIG_STACK_VALIDATION=y warning for out-of-tree modules
objtool: Improve detection of BUG() and other dead ends
objtool: Prevent GCC from merging annotate_unreachable()
objtool: Enclose contents of unreachable() macro in a block

 Makefile                        | 24 ++++++++---------
 arch/x86/kernel/vmlinux.lds.S   |  1 +
 include/linux/compiler-gcc.h    | 14 +++++++++-
 tools/objtool/arch.h            |  5 ++--
 tools/objtool/arch/x86/decode.c |  3 ---
 tools/objtool/builtin-check.c   | 60 ++++++++++++++++++++++++++++++++++++++---
 6 files changed, 84 insertions(+), 23 deletions(-)

Makefile +12 ~~-12~~

index f1e6a02a0c19..32c84577aa93 100644
@@ -908,6 +908,18 @@ mod_sign_cmd = true
 endif
 export mod_sign_cmd
 
+ifdef CONFIG_STACK_VALIDATION
+  has_libelf := $(call try-run,\
+		echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -,1,0)
+  ifeq ($(has_libelf),1)
+    objtool_target := tools/objtool FORCE
+  else
+    $(warning "Cannot use CONFIG_STACK_VALIDATION, please install libelf-dev, libelf-devel or elfutils-libelf-devel")
+    SKIP_STACK_VALIDATION := 1
+    export SKIP_STACK_VALIDATION
+  endif
+endif
+
 
 ifeq ($(KBUILD_EXTMOD),)
 core-y		+= kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/

@@ -1035,18 +1047,6 @@ prepare0: archprepare gcc-plugins
 # All the preparing..
 prepare: prepare0 prepare-objtool
 
-ifdef CONFIG_STACK_VALIDATION
-  has_libelf := $(call try-run,\
-		echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -,1,0)
-  ifeq ($(has_libelf),1)
-    objtool_target := tools/objtool FORCE
-  else
-    $(warning "Cannot use CONFIG_STACK_VALIDATION, please install libelf-dev, libelf-devel or elfutils-libelf-devel")
-    SKIP_STACK_VALIDATION := 1
-    export SKIP_STACK_VALIDATION
-  endif
-endif
-
 PHONY += prepare-objtool
 prepare-objtool: $(objtool_target)

arch/x86/kernel/vmlinux.lds.S +1 -0

index e79f15f108a8..ad0118fbce90 100644
@@ -346,6 +346,7 @@ SECTIONS
 	/DISCARD/ : {
 		*(.eh_frame)
 		*(__func_stack_frame_non_standard)
+		*(__unreachable)
 	}
 }

include/linux/compiler-gcc.h +13 -1

index 0444b1336268..f457b520ead6 100644
@@ -195,6 +195,17 @@
 #endif
 #endif
 
+#ifdef CONFIG_STACK_VALIDATION
+#define annotate_unreachable() ({					\
+	asm("%c0:\t\n"							\
+	    ".pushsection __unreachable, \"a\"\t\n"			\
+	    ".long %c0b\t\n"						\
+	    ".popsection\t\n" : : "i" (__LINE__));			\
+})
+#else
+#define annotate_unreachable()
+#endif
+
 /*
  * Mark a position in code as unreachable.  This can be used to
  * suppress control flow warnings after asm blocks that transfer

@@ -204,7 +215,8 @@
  * this in the preprocessor, but we can live with this because they're
  * unreleased.  Really, we need to have autoconf for the kernel.
  */
-#define unreachable() __builtin_unreachable()
+#define unreachable() \
+	do { annotate_unreachable(); __builtin_unreachable(); } while (0)
 
 /* Mark a function definition as prohibited from being cloned. */
 #define __noclone	__attribute__((__noclone__, __optimize__("no-tracer")))

tools/objtool/arch.h +2 -3

index f7350fcedc70..a59e061c0b4a 100644
@@ -31,9 +31,8 @@
 #define INSN_CALL_DYNAMIC	8
 #define INSN_RETURN		9
 #define INSN_CONTEXT_SWITCH	10
-#define INSN_BUG		11
-#define INSN_NOP		12
-#define INSN_OTHER		13
+#define INSN_NOP		11
+#define INSN_OTHER		12
 #define INSN_LAST		INSN_OTHER
 
 int arch_decode_instruction(struct elf *elf, struct section *sec,

tools/objtool/arch/x86/decode.c +0 -3

index 039636ffb6c8..6ac99e3266eb 100644
@@ -118,9 +118,6 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
 			 op2 == 0x35)
 			/* sysenter, sysret */
 			*type = INSN_CONTEXT_SWITCH;
-		else if (op2 == 0x0b || op2 == 0xb9)
-			/* ud2 */
-			*type = INSN_BUG;
 		else if (op2 == 0x0d || op2 == 0x1f)
 			/* nopl/nopw */
 			*type = INSN_NOP;

tools/objtool/builtin-check.c +56 -4

88 lines not on this page · raw message

Linus Torvalds

2017-03-01 02:03 UTC #1 lore raw

Guys,
the recent 'objtool' pull request broke things.

I haven't bisected it, but I'm pretty sure that this part is pure garbage:

Quoted 21 lines · Ingo Molnar

arch/x86/kernel/vmlinux.lds.S +1 -0

index e79f15f108a8..ad0118fbce90 100644
@@ -346,6 +346,7 @@ SECTIONS
        /DISCARD/ : {
                *(.eh_frame)
                *(__func_stack_frame_non_standard)
+               *(__unreachable)
        }
 }

include/linux/compiler-gcc.h +0 -0

index 0444b1336268..f457b520ead6 100644
@@ -195,6 +195,17 @@
 #endif
 #endif

include/linux/compiler-gcc.h line 207

Quoted 10 lines

include/linux/compiler-gcc.h +10 -0

+#ifdef CONFIG_STACK_VALIDATION
+#define annotate_unreachable() ({                                      \
+       asm("%c0:\t\n"                                                  \
+           ".pushsection __unreachable, \"a\"\t\n"                     \
+           ".long %c0b\t\n"                                            \
+           ".popsection\t\n" : : "i" (__LINE__));                      \
+})
+#else
+#define annotate_unreachable()
+#endif

and I think the above is what breaks module loading for me right now on my laptop.

I get this during bootup:

module: overflow in relocation type 10 val ffffffffc02afc81
module: 'nvme' likely not compiled with -mcmodel=kernel

(and similar errors for other modules too), but those modules very much *are* compiled with all the normal kernel build flags, including -mcmodel=kernel.

Now, relocation type 10 is R_X86_64_32, so the warning is very true:
that address would fit in a _signed_ 32-bit value, but that's supposedly a 32-bit unsigned relocation.

Trying to figure out what the hell is going on, I do:

    objdump -r nvme.ko | grep 64_32

and what do I find? I find

RELOCATION RECORDS FOR [__unreachable]:

  OFFSET           TYPE              VALUE
  0000000000000000 R_X86_64_32       .text+0x0000000000000c81
  0000000000000004 R_X86_64_32       .text+0x0000000000000cb5
  0000000000000008 R_X86_64_32       .text+0x0000000000001a18
  000000000000000c R_X86_64_32       .text+0x0000000000001a36
  0000000000000010 R_X86_64_32       .text+0x0000000000001e38
  0000000000000014 R_X86_64_32       .text+0x0000000000001ec2
  0000000000000018 R_X86_64_32       .text+0x00000000000034e2
  000000000000001c R_X86_64_32       .text+0x0000000000003536

and then when I look more closely (objdump --disassemble), I see that the offset 000c81 in the module refers to this:

0000000000000c60 <nvme_admin_init_request>:

        ....
     c7f:       0f 0b                   ud2
     c81:       0f 1f 44 00 00          nopl   0x0(%rax,%rax,1)

so it very much looks like those relocations are still around on modules, and so module loading fails.

Anyway, those annotations are completely bogus anyway, it looks. You guys should use relative offsets in order to be able to specify a kernel address. So doing

    .long %c0

is garbage - either it needs to be a .quad, or it needs to be relative to the text section to fit in a .long.

Hmm? Revert or fix, but please quickly...

Linus

Josh Poimboeuf

2017-03-01 06:30 UTC #2 lore raw

Quoted 86 lines · Linus Torvalds

Guys,
the recent 'objtool' pull request broke things.
I haven't bisected it, but I'm pretty sure that this part is pure garbage:
Quoted 21 lines · Ingo Molnar
arch/x86/kernel/vmlinux.lds.S +1 -0
index e79f15f108a8..ad0118fbce90 100644
@@ -346,6 +346,7 @@ SECTIONS
        /DISCARD/ : {
                *(.eh_frame)
                *(__func_stack_frame_non_standard)
+               *(__unreachable)
        }
 }
 
include/linux/compiler-gcc.h +0 -0
index 0444b1336268..f457b520ead6 100644
@@ -195,6 +195,17 @@
 #endif
 #endif
 
include/linux/compiler-gcc.h line 207
Quoted 10 lines
include/linux/compiler-gcc.h +10 -0
+#ifdef CONFIG_STACK_VALIDATION
+#define annotate_unreachable() ({                                      \
+       asm("%c0:\t\n"                                                  \
+           ".pushsection __unreachable, \"a\"\t\n"                     \
+           ".long %c0b\t\n"                                            \
+           ".popsection\t\n" : : "i" (__LINE__));                      \
+})
+#else
+#define annotate_unreachable()
+#endif
and I think the above is what breaks module loading for me right now on my laptop.
I get this during bootup:
module: overflow in relocation type 10 val ffffffffc02afc81
module: 'nvme' likely not compiled with -mcmodel=kernel
(and similar errors for other modules too), but those modules very much *are* compiled with all the normal kernel build flags, including -mcmodel=kernel.
Now, relocation type 10 is R_X86_64_32, so the warning is very true:
that address would fit in a _signed_ 32-bit value, but that's supposedly a 32-bit unsigned relocation.
Trying to figure out what the hell is going on, I do:
    objdump -r nvme.ko | grep 64_32
and what do I find? I find
RELOCATION RECORDS FOR [__unreachable]:
  OFFSET           TYPE              VALUE
  0000000000000000 R_X86_64_32       .text+0x0000000000000c81
  0000000000000004 R_X86_64_32       .text+0x0000000000000cb5
  0000000000000008 R_X86_64_32       .text+0x0000000000001a18
  000000000000000c R_X86_64_32       .text+0x0000000000001a36
  0000000000000010 R_X86_64_32       .text+0x0000000000001e38
  0000000000000014 R_X86_64_32       .text+0x0000000000001ec2
  0000000000000018 R_X86_64_32       .text+0x00000000000034e2
  000000000000001c R_X86_64_32       .text+0x0000000000003536
and then when I look more closely (objdump --disassemble), I see that the offset 000c81 in the module refers to this:
0000000000000c60 <nvme_admin_init_request>:
        ....
     c7f:       0f 0b                   ud2
     c81:       0f 1f 44 00 00          nopl   0x0(%rax,%rax,1)
so it very much looks like those relocations are still around on modules, and so module loading fails.
Anyway, those annotations are completely bogus anyway, it looks. You guys should use relative offsets in order to be able to specify a kernel address. So doing

Quoted 6 lines

    .long %c0
is garbage - either it needs to be a .quad, or it needs to be relative to the text section to fit in a .long.
Hmm? Revert or fix, but please quickly...

Yuck, sorry about that. Patch to fix it below.

This also highlights another (minor) issue: the '__unreachable' section is meant to be a compile-time-only thing. It's supposed to be discarded at link time, but apparently that isn't happening for modules.

I tried excluding it from linking with the .pushsection "e" flag, but no luck. I'll try to figure out how to fix that shortly.

In the meantime, here's the fix you need. It now uses X86_64_64 relocations.

----

From: Josh Poimboeuf <[email protected]>
Subject: [PATCH] objtool: fix __unreachable section relocation size

Linus reported the following commit broke module loading on his laptop:

d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")

It showed errors like the following:

module: overflow in relocation type 10 val ffffffffc02afc81
module: 'nvme' likely not compiled with -mcmodel=kernel

The problem is that the __unreachable section addresses are stored using the '.long' asm directive, which isn't big enough for .text section relative kernel addresses. Use '.quad' instead.

Reported-by Linus Torvalds <[email protected]>

Fixes d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")

Signed-off-by Josh Poimboeuf <[email protected]>

---
 include/linux/compiler-gcc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/linux/compiler-gcc.h +1 -1

index f457b520..7bd21e8 100644
@@ -199,7 +199,7 @@
 #define annotate_unreachable() ({					\
 	asm("%c0:\t\n"							\
 	    ".pushsection __unreachable, \"a\"\t\n"			\
-	    ".long %c0b\t\n"						\
+	    ".quad %c0b\t\n"						\
 	    ".popsection\t\n" : : "i" (__LINE__));			\
 })
 #else

Signature · 1 line

2.7.4

tip-bot for Josh Poimboeuf

merge bot

2017-03-01 08:14 UTC #3 lore raw

[tip:core/urgent] objtool: Fix __unreachable section relocation size

objtool: Fix __unreachable section relocation size

Commit-ID:  90a7e63a31b8f7d630d12ef0d8d37d3ab87f76e5
Gitweb:     http://git.kernel.org/tip/90a7e63a31b8f7d630d12ef0d8d37d3ab87f76e5
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Wed, 1 Mar 2017 00:05:04 -0600
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 1 Mar 2017 07:38:25 +0100

objtool: Fix __unreachable section relocation size

Linus reported the following commit broke module loading on his laptop:

d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")

It showed errors like the following:

module: overflow in relocation type 10 val ffffffffc02afc81
module: 'nvme' likely not compiled with -mcmodel=kernel

The problem is that the __unreachable section addresses are stored using the '.long' asm directive, which isn't big enough for .text section relative kernel addresses. Use '.quad' instead.

Reported-by Linus Torvalds <[email protected]>

Signed-off-by Josh Poimboeuf <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Fixes d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")

Link http://lkml.kernel.org/r/20170301060504.oltm3iws6fmubnom@treble open here

Signed-off-by Ingo Molnar <[email protected]>

---
 include/linux/compiler-gcc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/linux/compiler-gcc.h +1 -1

index 76e28c2..91a77a5 100644
@@ -201,7 +201,7 @@
 #define annotate_unreachable() ({					\
 	asm("%c0:\t\n"							\
 	    ".pushsection __unreachable, \"a\"\t\n"			\
-	    ".long %c0b\t\n"						\
+	    ".quad %c0b\t\n"						\
 	    ".popsection\t\n" : : "i" (__LINE__));			\
 })
 #else

[email protected]

reviewer

2017-03-01 08:42 UTC #4 lore raw

Re: [tip:core/urgent] objtool: Fix __unreachable section relocation size

Quoted 45 lines · tip-bot for Josh Poimboeuf

Commit-ID: 90a7e63a31b8f7d630d12ef0d8d37d3ab87f76e5 Gitweb:
http://git.kernel.org/tip/90a7e63a31b8f7d630d12ef0d8d37d3ab87f76e5
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Wed, 1 Mar 2017 00:05:04 -0600
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 1 Mar 2017 07:38:25 +0100
objtool: Fix __unreachable section relocation size
Linus reported the following commit broke module loading on his laptop:
d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")
It showed errors like the following:
module: overflow in relocation type 10 val ffffffffc02afc81
module: 'nvme' likely not compiled with -mcmodel=kernel
The problem is that the __unreachable section addresses are stored using
the '.long' asm directive, which isn't big enough for .text section relative kernel addresses. Use '.quad' instead.
Suggested-by Linus Torvalds <[email protected]>
Reported-by Linus Torvalds <[email protected]>
Signed-off-by Josh Poimboeuf <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Fixes d1091c7fa3d5 ("objtool: Improve detection of BUG() and other
dead ends")
Link http://lkml.kernel.org/r/20170301060504.oltm3iws6fmubnom@treble open here
Signed-off-by Ingo Molnar <[email protected]>
---
include/linux/compiler-gcc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/compiler-gcc.h +0 -0
index 76e28c2..91a77a5 100644

include/linux/compiler-gcc.h line 207

Quoted 8 lines

include/linux/compiler-gcc.h +1 -1

@@ -201,7 +201,7 @@
 #define annotate_unreachable() ({					\
 	asm("%c0:\t\n"							\
 	    ".pushsection __unreachable, \"a\"\t\n"			\
-	    ".long %c0b\t\n"						\
+	    ".quad %c0b\t\n"						\
 	    ".popsection\t\n" : : "i" (__LINE__));			\
 })
 #else

Or perhaps better use relative addresses, so:

.long foo - (.+4)

Signature · 1 line

Sent from my Android device with K-9 Mail. Please excuse my brevity.

Josh Poimboeuf

2017-03-01 12:51 UTC #5 lore raw

Re: [tip:core/urgent] objtool: Fix __unreachable section relocation size

Quoted 57 lines · On Wed, Mar 01, 2017 at 12:15:38AM -0800, [email protected] wrote:

Quoted 45 lines · tip-bot for Josh Poimboeuf
Commit-ID: 90a7e63a31b8f7d630d12ef0d8d37d3ab87f76e5 Gitweb:
http://git.kernel.org/tip/90a7e63a31b8f7d630d12ef0d8d37d3ab87f76e5
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Wed, 1 Mar 2017 00:05:04 -0600
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 1 Mar 2017 07:38:25 +0100
objtool: Fix __unreachable section relocation size
Linus reported the following commit broke module loading on his laptop:
d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")
It showed errors like the following:
module: overflow in relocation type 10 val ffffffffc02afc81
module: 'nvme' likely not compiled with -mcmodel=kernel
The problem is that the __unreachable section addresses are stored using
the '.long' asm directive, which isn't big enough for .text section relative kernel addresses. Use '.quad' instead.
Suggested-by Linus Torvalds <[email protected]>
Reported-by Linus Torvalds <[email protected]>
Signed-off-by Josh Poimboeuf <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Fixes d1091c7fa3d5 ("objtool: Improve detection of BUG() and other
dead ends")
Link http://lkml.kernel.org/r/20170301060504.oltm3iws6fmubnom@treble open here
Signed-off-by Ingo Molnar <[email protected]>
---
include/linux/compiler-gcc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/compiler-gcc.h +0 -0
index 76e28c2..91a77a5 100644
include/linux/compiler-gcc.h line 207
Quoted 8 lines
include/linux/compiler-gcc.h +1 -1
@@ -201,7 +201,7 @@
 #define annotate_unreachable() ({					\
 	asm("%c0:\t\n"							\
 	    ".pushsection __unreachable, \"a\"\t\n"			\
-	    ".long %c0b\t\n"						\
+	    ".quad %c0b\t\n"						\
 	    ".popsection\t\n" : : "i" (__LINE__));			\
 })
 #else
Or perhaps better use relative addresses, so:

Quoted 1 line

.long foo - (.+4)

Hm, yeah, something like that would indeed be better as it only needs 4 bytes instead of 8. For this use case, the following seems to work:

".long %c0b - .\t\n"

It produces the same relocation section+addend as before but with a X86_64_PC32 relocation.

It looks like the above patch was already merged so I'll add that change to the TODO list unless Ingo can squash it with the above patch.

Signature · 1 line

Josh

Ingo Molnar

author

2017-03-01 12:52 UTC #6 lore raw

PATCH 1/1 objtool, compiler.h: Fix __unreachable section relocation size

Quoted 63 lines · Josh Poimboeuf

Quoted 57 lines · On Wed, Mar 01, 2017 at 12:15:38AM -0800, [email protected] wrote:
Quoted 45 lines · tip-bot for Josh Poimboeuf
Commit-ID: 90a7e63a31b8f7d630d12ef0d8d37d3ab87f76e5 Gitweb:
http://git.kernel.org/tip/90a7e63a31b8f7d630d12ef0d8d37d3ab87f76e5
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Wed, 1 Mar 2017 00:05:04 -0600
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 1 Mar 2017 07:38:25 +0100
objtool: Fix __unreachable section relocation size
Linus reported the following commit broke module loading on his laptop:
d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")
It showed errors like the following:
module: overflow in relocation type 10 val ffffffffc02afc81
module: 'nvme' likely not compiled with -mcmodel=kernel
The problem is that the __unreachable section addresses are stored using
the '.long' asm directive, which isn't big enough for .text section relative kernel addresses. Use '.quad' instead.
Suggested-by Linus Torvalds <[email protected]>
Reported-by Linus Torvalds <[email protected]>
Signed-off-by Josh Poimboeuf <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Fixes d1091c7fa3d5 ("objtool: Improve detection of BUG() and other
dead ends")
Link http://lkml.kernel.org/r/20170301060504.oltm3iws6fmubnom@treble open here
Signed-off-by Ingo Molnar <[email protected]>
---
include/linux/compiler-gcc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/compiler-gcc.h +0 -0
index 76e28c2..91a77a5 100644
include/linux/compiler-gcc.h line 207
Quoted 8 lines
include/linux/compiler-gcc.h +1 -1
@@ -201,7 +201,7 @@
 #define annotate_unreachable() ({					\
 	asm("%c0:\t\n"							\
 	    ".pushsection __unreachable, \"a\"\t\n"			\
-	    ".long %c0b\t\n"						\
+	    ".quad %c0b\t\n"						\
 	    ".popsection\t\n" : : "i" (__LINE__));			\
 })
 #else
Or perhaps better use relative addresses, so:
Quoted 1 line
.long foo - (.+4)
Hm, yeah, something like that would indeed be better as it only needs 4 bytes instead of 8. For this use case, the following seems to work:

Quoted 7 lines

".long %c0b - .\t\n"
It produces the same relocation section+addend as before but with a X86_64_PC32 relocation.
It looks like the above patch was already merged so I'll add that change to the TODO list unless Ingo can squash it with the above patch.

I've squashed it - attached the latest version. Does this look good to everyone?

Thanks,

Ingo

================>

Quoted 1 line

From a6ae17f553bf9b4a0c7f8534e5f0f983bc7955c7 Mon Sep 17 00:00:00 2001

From: Josh Poimboeuf <[email protected]>
Date: Wed, 1 Mar 2017 00:05:04 -0600
Subject: [PATCH] objtool, compiler.h: Fix __unreachable section relocation size

Linus reported the following commit broke module loading on his laptop:

d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")

It showed errors like the following:

module: overflow in relocation type 10 val ffffffffc02afc81
module: 'nvme' likely not compiled with -mcmodel=kernel

The problem is that the __unreachable section addresses are stored using the '.long' asm directive, which isn't big enough for .text section relative kernel addresses. Use relative addresses instead:

".long %c0b - .\t\n"

Reported-by Linus Torvalds <[email protected]>

Signed-off-by Josh Poimboeuf <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Fixes d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")

Link http://lkml.kernel.org/r/20170301060504.oltm3iws6fmubnom@treble open here

Signed-off-by Ingo Molnar <[email protected]>

---
 include/linux/compiler-gcc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/linux/compiler-gcc.h +1 -1

index 76e28c229805..b6bb9019d87f 100644
@@ -201,7 +201,7 @@
 #define annotate_unreachable() ({					\
 	asm("%c0:\t\n"							\
 	    ".pushsection __unreachable, \"a\"\t\n"			\
-	    ".long %c0b\t\n"						\
+	    ".long %c0b - .\t\n"					\
 	    ".popsection\t\n" : : "i" (__LINE__));			\
 })
 #else

Josh Poimboeuf

2017-03-01 12:58 UTC #7 lore raw

Re: [PATCH] objtool, compiler.h: Fix __unreachable section relocation size

Quoted 90 lines · Ingo Molnar

Quoted 63 lines · Josh Poimboeuf
Quoted 57 lines · On Wed, Mar 01, 2017 at 12:15:38AM -0800, [email protected] wrote:
Quoted 45 lines · tip-bot for Josh Poimboeuf
Commit-ID: 90a7e63a31b8f7d630d12ef0d8d37d3ab87f76e5 Gitweb:
http://git.kernel.org/tip/90a7e63a31b8f7d630d12ef0d8d37d3ab87f76e5
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Wed, 1 Mar 2017 00:05:04 -0600
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 1 Mar 2017 07:38:25 +0100
objtool: Fix __unreachable section relocation size
Linus reported the following commit broke module loading on his laptop:
d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")
It showed errors like the following:
module: overflow in relocation type 10 val ffffffffc02afc81
module: 'nvme' likely not compiled with -mcmodel=kernel
The problem is that the __unreachable section addresses are stored using
the '.long' asm directive, which isn't big enough for .text section relative kernel addresses. Use '.quad' instead.
Suggested-by Linus Torvalds <[email protected]>
Reported-by Linus Torvalds <[email protected]>
Signed-off-by Josh Poimboeuf <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Fixes d1091c7fa3d5 ("objtool: Improve detection of BUG() and other
dead ends")
Link http://lkml.kernel.org/r/20170301060504.oltm3iws6fmubnom@treble open here
Signed-off-by Ingo Molnar <[email protected]>
---
include/linux/compiler-gcc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/compiler-gcc.h +0 -0
index 76e28c2..91a77a5 100644
include/linux/compiler-gcc.h line 207
Quoted 8 lines
include/linux/compiler-gcc.h +1 -1
@@ -201,7 +201,7 @@
 #define annotate_unreachable() ({					\
 	asm("%c0:\t\n"							\
 	    ".pushsection __unreachable, \"a\"\t\n"			\
-	    ".long %c0b\t\n"						\
+	    ".quad %c0b\t\n"						\
 	    ".popsection\t\n" : : "i" (__LINE__));			\
 })
 #else
Or perhaps better use relative addresses, so:
Quoted 1 line
.long foo - (.+4)
Hm, yeah, something like that would indeed be better as it only needs 4 bytes instead of 8. For this use case, the following seems to work:
Quoted 7 lines
".long %c0b - .\t\n"
It produces the same relocation section+addend as before but with a X86_64_PC32 relocation.
It looks like the above patch was already merged so I'll add that change to the TODO list unless Ingo can squash it with the above patch.
I've squashed it - attached the latest version. Does this look good to everyone?
Thanks,
Ingo
================>
From a6ae17f553bf9b4a0c7f8534e5f0f983bc7955c7 Mon Sep 17 00:00:00 2001
From: Josh Poimboeuf <[email protected]>
Date: Wed, 1 Mar 2017 00:05:04 -0600
Subject: [PATCH] objtool, compiler.h: Fix __unreachable section relocation size
Linus reported the following commit broke module loading on his laptop:
d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")

Quoted 8 lines

It showed errors like the following:
module: overflow in relocation type 10 val ffffffffc02afc81
module: 'nvme' likely not compiled with -mcmodel=kernel
The problem is that the __unreachable section addresses are stored using the '.long' asm directive, which isn't big enough for .text section relative kernel addresses. Use relative addresses instead:

Maybe remove the first "relative" in the last line. Otherwise it looks good to me. Thanks Ingo!

Signature · 1 line

Josh

Ingo Molnar

author

2017-03-01 10:38 UTC #8 lore raw

Re: [tip:core/urgent] objtool: Fix __unreachable section relocation size

In reply to tip-bot for Josh Poimboeuf

Quoted 9 lines · tip-bot for Josh Poimboeuf

Commit-ID:  90a7e63a31b8f7d630d12ef0d8d37d3ab87f76e5
Gitweb:     http://git.kernel.org/tip/90a7e63a31b8f7d630d12ef0d8d37d3ab87f76e5
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Wed, 1 Mar 2017 00:05:04 -0600
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 1 Mar 2017 07:38:25 +0100

objtool: Fix __unreachable section relocation size

Quoted 8 lines

Linus reported the following commit broke module loading on his laptop:
d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")
It showed errors like the following:
module: overflow in relocation type 10 val ffffffffc02afc81
module: 'nvme' likely not compiled with -mcmodel=kernel

BTW., beyond inadequate testing, another mistake I failed to notice during review is that the title of the patch was actively misleading: an "objtool: " prefix suggests a change that only affects objtool, while the changes here were to compiler-gcc.h, affecting pretty much everyone...

So a better, more informative commit title would have been:

objtool, compiler.h: Improve detection of BUG() and other dead ends

... to at least warn people about the wider impact. (To be honest I personally probably would still not have noticed the bug, but maybe others.)

Along the same lines I'll fix the title of this fix patch as well, once a final version has been reached.

Thanks,

Ingo

Josh Poimboeuf

2017-03-01 12:51 UTC #9 lore raw

Re: [tip:core/urgent] objtool: Fix __unreachable section relocation size

Quoted 28 lines · Ingo Molnar

Quoted 9 lines · tip-bot for Josh Poimboeuf
Commit-ID:  90a7e63a31b8f7d630d12ef0d8d37d3ab87f76e5
Gitweb:     http://git.kernel.org/tip/90a7e63a31b8f7d630d12ef0d8d37d3ab87f76e5
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Wed, 1 Mar 2017 00:05:04 -0600
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 1 Mar 2017 07:38:25 +0100
objtool: Fix __unreachable section relocation size
Quoted 8 lines
Linus reported the following commit broke module loading on his laptop:
d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")
It showed errors like the following:
module: overflow in relocation type 10 val ffffffffc02afc81
module: 'nvme' likely not compiled with -mcmodel=kernel
BTW., beyond inadequate testing, another mistake I failed to notice during review is that the title of the patch was actively misleading: an "objtool: " prefix suggests a change that only affects objtool, while the changes here were to compiler-gcc.h, affecting pretty much everyone...
So a better, more informative commit title would have been:

Quoted 7 lines

objtool, compiler.h: Improve detection of BUG() and other dead ends
... to at least warn people about the wider impact. (To be honest I personally probably would still not have noticed the bug, but maybe others.)
Along the same lines I'll fix the title of this fix patch as well, once a final version has been reached.

Yeah, that would have been a more informative title indeed.

For what seemed (to me) like a straightforward improvement, which was 90% objtool and 10% kernel, that commit introduced a surprising number of subtle bugs in the kernel.

Signature · 1 line

Josh

tip-bot for Josh Poimboeuf

merge bot

2017-03-01 13:26 UTC #10 lore raw

[tip:core/urgent] objtool, compiler.h: Fix __unreachable section relocation size

In reply to Josh Poimboeuf

objtool, compiler.h: Fix __unreachable section relocation size

Commit-ID:  55149d06534ae2a7ba5f7a078353deb89b3fe891
Gitweb:     http://git.kernel.org/tip/55149d06534ae2a7ba5f7a078353deb89b3fe891
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Wed, 1 Mar 2017 00:05:04 -0600
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 1 Mar 2017 14:20:18 +0100

objtool, compiler.h: Fix __unreachable section relocation size

Linus reported the following commit broke module loading on his laptop:

d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")

It showed errors like the following:

module: overflow in relocation type 10 val ffffffffc02afc81
module: 'nvme' likely not compiled with -mcmodel=kernel

The problem is that the __unreachable section addresses are stored using the '.long' asm directive, which isn't big enough for .text section kernel addresses. Use relative addresses instead:

".long %c0b - .\t\n"

Reported-by Linus Torvalds <[email protected]>

Signed-off-by Josh Poimboeuf <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Fixes d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")

Link http://lkml.kernel.org/r/20170301060504.oltm3iws6fmubnom@treble open here

Signed-off-by Ingo Molnar <[email protected]>

---
 include/linux/compiler-gcc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/linux/compiler-gcc.h +1 -1

index 76e28c2..b6bb901 100644
@@ -201,7 +201,7 @@
 #define annotate_unreachable() ({					\
 	asm("%c0:\t\n"							\
 	    ".pushsection __unreachable, \"a\"\t\n"			\
-	    ".long %c0b\t\n"						\
+	    ".long %c0b - .\t\n"					\
 	    ".popsection\t\n" : : "i" (__LINE__));			\
 })
 #else

tip-bot for Josh Poimboeuf

merge bot

2017-03-01 14:30 UTC #11 lore raw

[tip:core/urgent] objtool, compiler.h: Fix __unreachable section relocation size

In reply to Josh Poimboeuf

objtool, compiler.h: Fix __unreachable section relocation size

Commit-ID:  a6ae17f553bf9b4a0c7f8534e5f0f983bc7955c7
Gitweb:     http://git.kernel.org/tip/a6ae17f553bf9b4a0c7f8534e5f0f983bc7955c7
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Wed, 1 Mar 2017 00:05:04 -0600
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 1 Mar 2017 13:43:05 +0100

objtool, compiler.h: Fix __unreachable section relocation size

Linus reported the following commit broke module loading on his laptop:

d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")

It showed errors like the following:

module: overflow in relocation type 10 val ffffffffc02afc81
module: 'nvme' likely not compiled with -mcmodel=kernel

".long %c0b - .\t\n"

Reported-by Linus Torvalds <[email protected]>

Signed-off-by Josh Poimboeuf <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Fixes d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")

Link http://lkml.kernel.org/r/20170301060504.oltm3iws6fmubnom@treble open here

Signed-off-by Ingo Molnar <[email protected]>

---
 include/linux/compiler-gcc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/linux/compiler-gcc.h +1 -1

index 76e28c2..b6bb901 100644
@@ -201,7 +201,7 @@
 #define annotate_unreachable() ({					\
 	asm("%c0:\t\n"							\
 	    ".pushsection __unreachable, \"a\"\t\n"			\
-	    ".long %c0b\t\n"						\
+	    ".long %c0b - .\t\n"					\
 	    ".popsection\t\n" : : "i" (__LINE__));			\
 })
 #else