2017-03-01 15:21:49

by Josh Poimboeuf

[permalink] [raw]
Subject: [PATCH] objtool, module: discard __unreachable section for modules

The __unreachable section is only used at compile time. It's discarded
for vmlinux but it should also be discarded for modules.

Fixes: d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")
Signed-off-by: Josh Poimboeuf <[email protected]>
---
scripts/module-common.lds | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/module-common.lds b/scripts/module-common.lds
index 73a2c7d..936a3c6 100644
--- a/scripts/module-common.lds
+++ b/scripts/module-common.lds
@@ -4,7 +4,10 @@
* combine them automatically.
*/
SECTIONS {
- /DISCARD/ : { *(.discard) }
+ /DISCARD/ : {
+ *(.discard)
+ *(__unreachable)
+ }

__ksymtab 0 : { *(SORT(___ksymtab+*)) }
__ksymtab_gpl 0 : { *(SORT(___ksymtab_gpl+*)) }
--
2.7.4


2017-03-01 16:44:59

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] objtool, module: discard __unreachable section for modules

On March 1, 2017 7:20:03 AM PST, Josh Poimboeuf <[email protected]> wrote:
>The __unreachable section is only used at compile time. It's discarded
>for vmlinux but it should also be discarded for modules.
>
>Fixes: d1091c7fa3d5 ("objtool: Improve detection of BUG() and other
>dead ends")
>Signed-off-by: Josh Poimboeuf <[email protected]>
>---
> scripts/module-common.lds | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/scripts/module-common.lds b/scripts/module-common.lds
>index 73a2c7d..936a3c6 100644
>--- a/scripts/module-common.lds
>+++ b/scripts/module-common.lds
>@@ -4,7 +4,10 @@
> * combine them automatically.
> */
> SECTIONS {
>- /DISCARD/ : { *(.discard) }
>+ /DISCARD/ : {
>+ *(.discard)
>+ *(__unreachable)
>+ }
>
> __ksymtab 0 : { *(SORT(___ksymtab+*)) }
> __ksymtab_gpl 0 : { *(SORT(___ksymtab_gpl+*)) }

I would like to see a name like, say, ".annot.unreachable", since is odds are pretty high we are going to need more annotations in the future.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2017-03-01 17:02:11

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] objtool, module: discard __unreachable section for modules

On Wed, Mar 1, 2017 at 8:44 AM, <[email protected]> wrote:
>
> I would like to see a name like, say, ".annot.unreachable", since is odds are pretty high we are going to need more annotations in the future.

Why not just make it ".discard.unreachable", and then make the discard
pattern be ".discard*".

We already have .discard.text for some trace hackery, so it follows a pattern.

And that's what we do for things like .rodata.* etc.

In fact, we already have a pattern for '.discard.*' in the main
vmlinux.lds.h file exactly for the .discard.text case (but not for
modules, it looks like).

Linus

2017-03-01 17:43:00

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] objtool, module: discard __unreachable section for modules

On March 1, 2017 9:01:54 AM PST, Linus Torvalds <[email protected]> wrote:
>On Wed, Mar 1, 2017 at 8:44 AM, <[email protected]> wrote:
>>
>> I would like to see a name like, say, ".annot.unreachable", since is
>odds are pretty high we are going to need more annotations in the
>future.
>
>Why not just make it ".discard.unreachable", and then make the discard
>pattern be ".discard*".
>
>We already have .discard.text for some trace hackery, so it follows a
>pattern.
>
>And that's what we do for things like .rodata.* etc.
>
>In fact, we already have a pattern for '.discard.*' in the main
>vmlinux.lds.h file exactly for the .discard.text case (but not for
>modules, it looks like).
>
> Linus

I was assuming it would be used for more than just discarding.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2017-03-01 18:23:06

by Josh Poimboeuf

[permalink] [raw]
Subject: [PATCH v2] objtool, module: discard objtool annotation sections for modules


The '__unreachable' and '__func_stack_frame_non_standard' sections are
only used at compile time. They're discarded for vmlinux but they
should also be discarded for modules.

Since this is a recurring pattern, prefix the section names with
".discard.". It's a nice convention and vmlinux.lds.h already discards
such sections.

Also remove the 'a' (allocatable) flag from the __unreachable section
since it doesn't make sense for a discarded section.

Suggested-by: Linus Torvalds <[email protected]>
Fixes: d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")
Signed-off-by: Josh Poimboeuf <[email protected]>
---
arch/x86/kernel/vmlinux.lds.S | 2 --
include/linux/compiler-gcc.h | 2 +-
include/linux/frame.h | 2 +-
scripts/mod/modpost.c | 1 +
scripts/module-common.lds | 5 ++++-
tools/objtool/builtin-check.c | 7 ++++---
6 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index ad0118f..c74ae9c 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -345,8 +345,6 @@ SECTIONS
DISCARDS
/DISCARD/ : {
*(.eh_frame)
- *(__func_stack_frame_non_standard)
- *(__unreachable)
}
}

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index b6bb901..0efef9c 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -200,7 +200,7 @@
#ifdef CONFIG_STACK_VALIDATION
#define annotate_unreachable() ({ \
asm("%c0:\t\n" \
- ".pushsection __unreachable, \"a\"\t\n" \
+ ".pushsection .discard.unreachable\t\n" \
".long %c0b - .\t\n" \
".popsection\t\n" : : "i" (__LINE__)); \
})
diff --git a/include/linux/frame.h b/include/linux/frame.h
index e6baaba..d772c61 100644
--- a/include/linux/frame.h
+++ b/include/linux/frame.h
@@ -11,7 +11,7 @@
* For more information, see tools/objtool/Documentation/stack-validation.txt.
*/
#define STACK_FRAME_NON_STANDARD(func) \
- static void __used __section(__func_stack_frame_non_standard) \
+ static void __used __section(.discard.func_stack_frame_non_standard) \
*__func_stack_frame_non_standard_##func = func

#else /* !CONFIG_STACK_VALIDATION */
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 4dedd0d..30d752a 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -854,6 +854,7 @@ static const char *const section_white_list[] =
".cmem*", /* EZchip */
".fmt_slot*", /* EZchip */
".gnu.lto*",
+ ".discard.*",
NULL
};

diff --git a/scripts/module-common.lds b/scripts/module-common.lds
index 73a2c7d..cf7e52e 100644
--- a/scripts/module-common.lds
+++ b/scripts/module-common.lds
@@ -4,7 +4,10 @@
* combine them automatically.
*/
SECTIONS {
- /DISCARD/ : { *(.discard) }
+ /DISCARD/ : {
+ *(.discard)
+ *(.discard.*)
+ }

__ksymtab 0 : { *(SORT(___ksymtab+*)) }
__ksymtab_gpl 0 : { *(SORT(___ksymtab_gpl+*)) }
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 5fc52ee..bd12eb1 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -339,13 +339,14 @@ static int add_dead_ends(struct objtool_file *file)
struct instruction *insn;
bool found;

- sec = find_section_by_name(file->elf, ".rela__unreachable");
+ sec = find_section_by_name(file->elf, ".rela.discard.unreachable");
if (!sec)
return 0;

list_for_each_entry(rela, &sec->rela_list, list) {
if (rela->sym->type != STT_SECTION) {
- WARN("unexpected relocation symbol type in .rela__unreachable");
+ WARN("unexpected relocation symbol type in %s",
+ sec->name);
return -1;
}
insn = find_insn(file, rela->sym->sec, rela->addend);
@@ -1272,7 +1273,7 @@ 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.whitelist = find_section_by_name(file.elf, ".discard.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");
--
2.7.4

Subject: [tip:core/urgent] objtool, modules: Discard objtool annotation sections for modules

Commit-ID: e390f9a9689a42f477a6073e2e7df530a4c1b740
Gitweb: http://git.kernel.org/tip/e390f9a9689a42f477a6073e2e7df530a4c1b740
Author: Josh Poimboeuf <[email protected]>
AuthorDate: Wed, 1 Mar 2017 12:04:44 -0600
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 1 Mar 2017 20:32:25 +0100

objtool, modules: Discard objtool annotation sections for modules

The '__unreachable' and '__func_stack_frame_non_standard' sections are
only used at compile time. They're discarded for vmlinux but they
should also be discarded for modules.

Since this is a recurring pattern, prefix the section names with
".discard.". It's a nice convention and vmlinux.lds.h already discards
such sections.

Also remove the 'a' (allocatable) flag from the __unreachable section
since it doesn't make sense for a discarded section.

Suggested-by: Linus Torvalds <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
Cc: Jessica Yu <[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/20170301180444.lhd53c5tibc4ns77@treble
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/vmlinux.lds.S | 2 --
include/linux/compiler-gcc.h | 2 +-
include/linux/frame.h | 2 +-
scripts/mod/modpost.c | 1 +
scripts/module-common.lds | 5 ++++-
tools/objtool/builtin-check.c | 6 +++---
6 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index ad0118f..c74ae9c 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -345,8 +345,6 @@ SECTIONS
DISCARDS
/DISCARD/ : {
*(.eh_frame)
- *(__func_stack_frame_non_standard)
- *(__unreachable)
}
}

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index b6bb901..0efef9c 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -200,7 +200,7 @@
#ifdef CONFIG_STACK_VALIDATION
#define annotate_unreachable() ({ \
asm("%c0:\t\n" \
- ".pushsection __unreachable, \"a\"\t\n" \
+ ".pushsection .discard.unreachable\t\n" \
".long %c0b - .\t\n" \
".popsection\t\n" : : "i" (__LINE__)); \
})
diff --git a/include/linux/frame.h b/include/linux/frame.h
index e6baaba..d772c61 100644
--- a/include/linux/frame.h
+++ b/include/linux/frame.h
@@ -11,7 +11,7 @@
* For more information, see tools/objtool/Documentation/stack-validation.txt.
*/
#define STACK_FRAME_NON_STANDARD(func) \
- static void __used __section(__func_stack_frame_non_standard) \
+ static void __used __section(.discard.func_stack_frame_non_standard) \
*__func_stack_frame_non_standard_##func = func

#else /* !CONFIG_STACK_VALIDATION */
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 4dedd0d..30d752a 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -854,6 +854,7 @@ static const char *const section_white_list[] =
".cmem*", /* EZchip */
".fmt_slot*", /* EZchip */
".gnu.lto*",
+ ".discard.*",
NULL
};

diff --git a/scripts/module-common.lds b/scripts/module-common.lds
index 73a2c7d..cf7e52e 100644
--- a/scripts/module-common.lds
+++ b/scripts/module-common.lds
@@ -4,7 +4,10 @@
* combine them automatically.
*/
SECTIONS {
- /DISCARD/ : { *(.discard) }
+ /DISCARD/ : {
+ *(.discard)
+ *(.discard.*)
+ }

__ksymtab 0 : { *(SORT(___ksymtab+*)) }
__ksymtab_gpl 0 : { *(SORT(___ksymtab_gpl+*)) }
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 5fc52ee..4cfdbb5 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -339,13 +339,13 @@ static int add_dead_ends(struct objtool_file *file)
struct instruction *insn;
bool found;

- sec = find_section_by_name(file->elf, ".rela__unreachable");
+ sec = find_section_by_name(file->elf, ".rela.discard.unreachable");
if (!sec)
return 0;

list_for_each_entry(rela, &sec->rela_list, list) {
if (rela->sym->type != STT_SECTION) {
- WARN("unexpected relocation symbol type in .rela__unreachable");
+ WARN("unexpected relocation symbol type in %s", sec->name);
return -1;
}
insn = find_insn(file, rela->sym->sec, rela->addend);
@@ -1272,7 +1272,7 @@ 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.whitelist = find_section_by_name(file.elf, ".discard.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");