2016-04-23 11:10:59

by Ingo Molnar

[permalink] [raw]
Subject: [GIT PULL] objtool fixes

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: c2bb9e32e2315971a8535fee77335c04a739d71d objtool: Fix Makefile to properly see if libelf is supported

A handful of objtool fixes: two improvements to how warnings are printed plus a
false positive warning fix, and build environment fix.

Thanks,

Ingo

------------------>
Josh Poimboeuf (2):
objtool: Add workaround for GCC switch jump table bug
objtool: Detect falling through to the next function

Steven Rostedt (1):
objtool: Fix Makefile to properly see if libelf is supported


Makefile | 3 +-
tools/objtool/Documentation/stack-validation.txt | 38 +++++++---
tools/objtool/builtin-check.c | 97 ++++++++++++++++++------
3 files changed, 103 insertions(+), 35 deletions(-)

diff --git a/Makefile b/Makefile
index 1d0aef03eae7..70ca38ef9f4b 100644
--- a/Makefile
+++ b/Makefile
@@ -1008,7 +1008,8 @@ prepare0: archprepare FORCE
prepare: prepare0 prepare-objtool

ifdef CONFIG_STACK_VALIDATION
- has_libelf := $(shell echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf - &> /dev/null && echo 1 || echo 0)
+ 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
diff --git a/tools/objtool/Documentation/stack-validation.txt b/tools/objtool/Documentation/stack-validation.txt
index 5a95896105bc..55a60d331f47 100644
--- a/tools/objtool/Documentation/stack-validation.txt
+++ b/tools/objtool/Documentation/stack-validation.txt
@@ -299,18 +299,38 @@ they mean, and suggestions for how to fix them.
Errors in .c files
------------------

-If you're getting an objtool error in a compiled .c file, chances are
-the file uses an asm() statement which has a "call" instruction. An
-asm() statement with a call instruction must declare the use of the
-stack pointer in its output operand. For example, on x86_64:
+1. c_file.o: warning: objtool: funcA() falls through to next function funcB()

- register void *__sp asm("rsp");
- asm volatile("call func" : "+r" (__sp));
+ This means that funcA() doesn't end with a return instruction or an
+ unconditional jump, and that objtool has determined that the function
+ can fall through into the next function. There could be different
+ reasons for this:

-Otherwise the stack frame may not get created before the call.
+ 1) funcA()'s last instruction is a call to a "noreturn" function like
+ panic(). In this case the noreturn function needs to be added to
+ objtool's hard-coded global_noreturns array. Feel free to bug the
+ objtool maintainer, or you can submit a patch.

-Another possible cause for errors in C code is if the Makefile removes
--fno-omit-frame-pointer or adds -fomit-frame-pointer to the gcc options.
+ 2) funcA() uses the unreachable() annotation in a section of code
+ that is actually reachable.
+
+ 3) If funcA() calls an inline function, the object code for funcA()
+ might be corrupt due to a gcc bug. For more details, see:
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646
+
+2. If you're getting any other objtool error in a compiled .c file, it
+ may be because the file uses an asm() statement which has a "call"
+ instruction. An asm() statement with a call instruction must declare
+ the use of the stack pointer in its output operand. For example, on
+ x86_64:
+
+ register void *__sp asm("rsp");
+ asm volatile("call func" : "+r" (__sp));
+
+ Otherwise the stack frame may not get created before the call.
+
+3. Another possible cause for errors in C code is if the Makefile removes
+ -fno-omit-frame-pointer or adds -fomit-frame-pointer to the gcc options.

Also see the above section for .S file errors for more information what
the individual error messages mean.
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 7515cb2e879a..e8a1e69eb92c 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -54,6 +54,7 @@ struct instruction {
struct symbol *call_dest;
struct instruction *jump_dest;
struct list_head alts;
+ struct symbol *func;
};

struct alternative {
@@ -66,6 +67,7 @@ struct objtool_file {
struct list_head insn_list;
DECLARE_HASHTABLE(insn_hash, 16);
struct section *rodata, *whitelist;
+ bool ignore_unreachables, c_file;
};

const char *objname;
@@ -228,7 +230,7 @@ static int __dead_end_function(struct objtool_file *file, struct symbol *func,
}
}

- if (insn->type == INSN_JUMP_DYNAMIC)
+ if (insn->type == INSN_JUMP_DYNAMIC && list_empty(&insn->alts))
/* sibling call */
return 0;
}
@@ -248,6 +250,7 @@ static int dead_end_function(struct objtool_file *file, struct symbol *func)
static int decode_instructions(struct objtool_file *file)
{
struct section *sec;
+ struct symbol *func;
unsigned long offset;
struct instruction *insn;
int ret;
@@ -281,6 +284,21 @@ static int decode_instructions(struct objtool_file *file)
hash_add(file->insn_hash, &insn->hash, insn->offset);
list_add_tail(&insn->list, &file->insn_list);
}
+
+ list_for_each_entry(func, &sec->symbol_list, list) {
+ if (func->type != STT_FUNC)
+ continue;
+
+ if (!find_insn(file, sec, func->offset)) {
+ WARN("%s(): can't find starting instruction",
+ func->name);
+ return -1;
+ }
+
+ func_for_each_insn(file, func, insn)
+ if (!insn->func)
+ insn->func = func;
+ }
}

return 0;
@@ -664,13 +682,40 @@ static int add_func_switch_tables(struct objtool_file *file,
text_rela->addend);

/*
- * TODO: Document where this is needed, or get rid of it.
- *
* rare case: jmpq *[addr](%rip)
+ *
+ * This check is for a rare gcc quirk, currently only seen in
+ * three driver functions in the kernel, only with certain
+ * obscure non-distro configs.
+ *
+ * As part of an optimization, gcc makes a copy of an existing
+ * switch jump table, modifies it, and then hard-codes the jump
+ * (albeit with an indirect jump) to use a single entry in the
+ * table. The rest of the jump table and some of its jump
+ * targets remain as dead code.
+ *
+ * In such a case we can just crudely ignore all unreachable
+ * instruction warnings for the entire object file. Ideally we
+ * would just ignore them for the function, but that would
+ * require redesigning the code quite a bit. And honestly
+ * that's just not worth doing: unreachable instruction
+ * warnings are of questionable value anyway, and this is such
+ * a rare issue.
+ *
+ * kbuild reports:
+ * - https://lkml.kernel.org/r/201603231906.LWcVUpxm%[email protected]
+ * - https://lkml.kernel.org/r/201603271114.K9i45biy%[email protected]
+ * - https://lkml.kernel.org/r/201603291058.zuJ6ben1%[email protected]
+ *
+ * gcc bug:
+ * - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70604
*/
- if (!rodata_rela)
+ if (!rodata_rela) {
rodata_rela = find_rela_by_dest(file->rodata,
text_rela->addend + 4);
+ if (rodata_rela)
+ file->ignore_unreachables = true;
+ }

if (!rodata_rela)
continue;
@@ -732,9 +777,6 @@ static int decode_sections(struct objtool_file *file)
{
int ret;

- file->whitelist = find_section_by_name(file->elf, "__func_stack_frame_non_standard");
- file->rodata = find_section_by_name(file->elf, ".rodata");
-
ret = decode_instructions(file);
if (ret)
return ret;
@@ -799,6 +841,7 @@ static int validate_branch(struct objtool_file *file,
struct alternative *alt;
struct instruction *insn;
struct section *sec;
+ struct symbol *func = NULL;
unsigned char state;
int ret;

@@ -813,6 +856,16 @@ static int validate_branch(struct objtool_file *file,
}

while (1) {
+ if (file->c_file && insn->func) {
+ if (func && func != insn->func) {
+ WARN("%s() falls through to next function %s()",
+ func->name, insn->func->name);
+ return 1;
+ }
+
+ func = insn->func;
+ }
+
if (insn->visited) {
if (frame_state(insn->state) != frame_state(state)) {
WARN_FUNC("frame pointer state mismatch",
@@ -823,13 +876,6 @@ static int validate_branch(struct objtool_file *file,
return 0;
}

- /*
- * Catch a rare case where a noreturn function falls through to
- * the next function.
- */
- if (is_fentry_call(insn) && (state & STATE_FENTRY))
- return 0;
-
insn->visited = true;
insn->state = state;

@@ -1035,12 +1081,8 @@ static int validate_functions(struct objtool_file *file)
continue;

insn = find_insn(file, sec, func->offset);
- if (!insn) {
- WARN("%s(): can't find starting instruction",
- func->name);
- warnings++;
+ if (!insn)
continue;
- }

ret = validate_branch(file, insn, 0);
warnings += ret;
@@ -1056,13 +1098,14 @@ static int validate_functions(struct objtool_file *file)
if (insn->visited)
continue;

- if (!ignore_unreachable_insn(func, insn) &&
- !warnings) {
- WARN_FUNC("function has unreachable instruction", insn->sec, insn->offset);
- warnings++;
- }
-
insn->visited = true;
+
+ if (file->ignore_unreachables || warnings ||
+ ignore_unreachable_insn(func, insn))
+ continue;
+
+ WARN_FUNC("function has unreachable instruction", insn->sec, insn->offset);
+ warnings++;
}
}
}
@@ -1133,6 +1176,10 @@ int cmd_check(int argc, const char **argv)

INIT_LIST_HEAD(&file.insn_list);
hash_init(file.insn_hash);
+ file.whitelist = find_section_by_name(file.elf, "__func_stack_frame_non_standard");
+ file.rodata = find_section_by_name(file.elf, ".rodata");
+ file.ignore_unreachables = false;
+ file.c_file = find_section_by_name(file.elf, ".comment");

ret = decode_sections(&file);
if (ret < 0)


2017-03-01 06:30:43

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [GIT PULL] objtool fixes

On Tue, Feb 28, 2017 at 05:55:11PM -0800, Linus Torvalds wrote:
> Guys,
> the recent 'objtool' pull request broke things.
>
> I haven't bisected it, but I'm pretty sure that this part is pure garbage:
>
> On Mon, Feb 27, 2017 at 11:53 PM, Ingo Molnar <[email protected]> wrote:
> >
> > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> > index e79f15f108a8..ad0118fbce90 100644
> > --- a/arch/x86/kernel/vmlinux.lds.S
> > +++ b/arch/x86/kernel/vmlinux.lds.S
> > @@ -346,6 +346,7 @@ SECTIONS
> > /DISCARD/ : {
> > *(.eh_frame)
> > *(__func_stack_frame_non_standard)
> > + *(__unreachable)
> > }
> > }
> >
> > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> > index 0444b1336268..f457b520ead6 100644
> > --- a/include/linux/compiler-gcc.h
> > +++ b/include/linux/compiler-gcc.h
> > @@ -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
>
> 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...

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]>
Suggested-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(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index f457b520..7bd21e8 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -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
--
2.7.4

Subject: [tip:core/urgent] 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.

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
Signed-off-by: Ingo Molnar <[email protected]>
---
include/linux/compiler-gcc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 76e28c2..91a77a5 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -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

2017-03-01 08:42:59

by H. Peter Anvin

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

On March 1, 2017 12:10:59 AM PST, tip-bot for Josh Poimboeuf <[email protected]> wrote:
>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
>Signed-off-by: Ingo Molnar <[email protected]>
>---
> include/linux/compiler-gcc.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/include/linux/compiler-gcc.h
>b/include/linux/compiler-gcc.h
>index 76e28c2..91a77a5 100644
>--- a/include/linux/compiler-gcc.h
>+++ b/include/linux/compiler-gcc.h
>@@ -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)


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

2017-03-01 10:38:25

by Ingo Molnar

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


* tip-bot for Josh Poimboeuf <[email protected]> wrote:

> 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

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

2017-03-01 12:51:27

by Josh Poimboeuf

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

On Wed, Mar 01, 2017 at 11:37:12AM +0100, Ingo Molnar wrote:
>
> * tip-bot for Josh Poimboeuf <[email protected]> wrote:
>
> > 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
>
> 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.

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.

--
Josh

2017-03-01 12:51:23

by Josh Poimboeuf

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

On Wed, Mar 01, 2017 at 12:15:38AM -0800, [email protected] wrote:
> On March 1, 2017 12:10:59 AM PST, tip-bot for Josh Poimboeuf <[email protected]> wrote:
> >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
> >Signed-off-by: Ingo Molnar <[email protected]>
> >---
> > include/linux/compiler-gcc.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/include/linux/compiler-gcc.h
> >b/include/linux/compiler-gcc.h
> >index 76e28c2..91a77a5 100644
> >--- a/include/linux/compiler-gcc.h
> >+++ b/include/linux/compiler-gcc.h
> >@@ -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)

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.

--
Josh

2017-03-01 12:52:09

by Ingo Molnar

[permalink] [raw]
Subject: [PATCH] objtool, compiler.h: Fix __unreachable section relocation size


* Josh Poimboeuf <[email protected]> wrote:

> On Wed, Mar 01, 2017 at 12:15:38AM -0800, [email protected] wrote:
> > On March 1, 2017 12:10:59 AM PST, tip-bot for Josh Poimboeuf <[email protected]> wrote:
> > >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
> > >Signed-off-by: Ingo Molnar <[email protected]>
> > >---
> > > include/linux/compiler-gcc.h | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > >diff --git a/include/linux/compiler-gcc.h
> > >b/include/linux/compiler-gcc.h
> > >index 76e28c2..91a77a5 100644
> > >--- a/include/linux/compiler-gcc.h
> > >+++ b/include/linux/compiler-gcc.h
> > >@@ -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)
>
> 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.

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

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"

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
Signed-off-by: Ingo Molnar <[email protected]>
---
include/linux/compiler-gcc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 76e28c229805..b6bb9019d87f 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -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

2017-03-01 12:58:46

by Josh Poimboeuf

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

On Wed, Mar 01, 2017 at 01:44:01PM +0100, Ingo Molnar wrote:
>
> * Josh Poimboeuf <[email protected]> wrote:
>
> > On Wed, Mar 01, 2017 at 12:15:38AM -0800, [email protected] wrote:
> > > On March 1, 2017 12:10:59 AM PST, tip-bot for Josh Poimboeuf <[email protected]> wrote:
> > > >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
> > > >Signed-off-by: Ingo Molnar <[email protected]>
> > > >---
> > > > include/linux/compiler-gcc.h | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > >diff --git a/include/linux/compiler-gcc.h
> > > >b/include/linux/compiler-gcc.h
> > > >index 76e28c2..91a77a5 100644
> > > >--- a/include/linux/compiler-gcc.h
> > > >+++ b/include/linux/compiler-gcc.h
> > > >@@ -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)
> >
> > 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.
>
> 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")
>
> 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!

--
Josh

Subject: [tip:core/urgent] 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"

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
Signed-off-by: Ingo Molnar <[email protected]>
---
include/linux/compiler-gcc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 76e28c2..b6bb901 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -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

Subject: [tip:core/urgent] 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

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"

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
Signed-off-by: Ingo Molnar <[email protected]>
---
include/linux/compiler-gcc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 76e28c2..b6bb901 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -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