objtool: seg fault fixes and retpoline improvements
cover objtool: seg fault fixes and retpoline improvements
Some assorted fixes and improvements.
Josh Poimboeuf (4):
objtool: Improve retpoline alternative handling
objtool: Add support for alternatives at the end of a section
objtool: Warn on stripped section symbol
objtool: Don't print '.tmp_' prefix for .o files
tools/objtool/check.c | 110 ++++++++++++++++++++++++++++++------------------ tools/objtool/orc_gen.c | 5 +++ 2 files changed, 73 insertions(+), 42 deletions(-)
Signature · 1 line
2.14.3
PATCH 1/4 objtool: Improve retpoline alternative handling
Currently objtool requires all retpolines to be
a) patched in with alternatives; and
b) annotated with ANNOTATE_NOSPEC_ALTERNATIVE.
If you forget to do both of the above, objtool seg faults trying to dereference a NULL 'insn->call_dest' pointer.
Avoid that situation and print a more helpful error message:
quirks.o: warning: objtool: efi_delete_dummy_variable()+0x99: unsupported intra-function call
quirks.o: warning: objtool: If this is a retpoline, please patch it in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE.
Future improvements can be made to make objtool smarter with respect to retpolines, but this is a good incremental improvement for now.
--- tools/objtool/check.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-)
tools/objtool/check.c +16 -20
@@ -543,18 +543,14 @@ static int add_call_destinations(struct objtool_file *file) dest_off = insn->offset + insn->len + insn->immediate; insn->call_dest =find_symbol_by_offset (insn->sec, dest_off); -/* -* FIXME: Thanks to retpolines, it's now considered -* normal for a function to call within itself. So -* disable this warning for now. -*/ -#if 0 -if (!insn->call_dest) { -WARN_FUNC ("can't find call dest symbol at offset 0x%lx" , - insn->sec, insn->offset, dest_off); + +if (!insn->call_dest && !insn->ignore) { +WARN_FUNC ("unsupported intra-function call" , + insn->sec, insn->offset); +WARN ("If this is a retpoline, please patch it in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE." );return -1 ; } -#endif + }else if (rela->sym->type ==STT_SECTION ) { insn->call_dest =find_symbol_by_offset (rela->sym->sec, rela->addend+4 );
@@ -648,6 +644,8 @@ static int handle_group_alt(struct objtool_file *file, last_new_insn = insn; + insn->ignore = orig_insn->ignore_alts; +if (insn->type !=INSN_JUMP_CONDITIONAL && insn->type !=INSN_JUMP_UNCONDITIONAL )continue ;
@@ -729,10 +727,6 @@ static int add_special_section_alts(struct objtool_file *file) goto out; } -/* Ignore retpoline alternatives. */ -if (orig_insn->ignore_alts) -continue ; - new_insn =NULL ;if (!special_alt->group || special_alt->new_len) { new_insn = find_insn(file, special_alt->new_sec,
@@ -1089,11 +1083,11 @@ static int decode_sections(struct objtool_file *file)if (ret)return ret; - ret =add_call_destinations (file); + ret =add_special_section_alts (file);if (ret)return ret; - ret =add_special_section_alts (file); + ret =add_call_destinations (file);if (ret)return ret;
@@ -1720,10 +1714,12 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, insn->visited = true; -list_for_each_entry (alt, &insn->alts, list) { - ret =validate_branch (file, alt->insn, state); -if (ret) -return 1 ; +if (!insn->ignore_alts) { +list_for_each_entry (alt, &insn->alts, list) { + ret =validate_branch (file, alt->insn, state); +if (ret) +return 1 ; + } }switch (insn->type) {
Signature · 1 line
2.14.3
tip-bot for Josh Poimboeuf
merge bot[tip:x86/pti] objtool: Improve retpoline alternative handling
objtool: Improve retpoline alternative handling
Commit-ID: 44510d9e1656fbc52721e7ceb41033359576f2a7 Gitweb: https://git.kernel.org/tip/44510d9e1656fbc52721e7ceb41033359576f2a7 Author: Josh Poimboeuf <[email protected]> AuthorDate: Mon, 29 Jan 2018 22:00:39 -0600 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Jan 2018 07:55:04 +0100
objtool: Improve retpoline alternative handling
Currently objtool requires all retpolines to be:
a) patched in with alternatives; and
b) annotated with ANNOTATE_NOSPEC_ALTERNATIVE.
If you forget to do both of the above, objtool segfaults trying to dereference a NULL 'insn->call_dest' pointer.
Avoid that situation and print a more helpful error message:
quirks.o: warning: objtool: efi_delete_dummy_variable()+0x99: unsupported intra-function call
quirks.o: warning: objtool: If this is a retpoline, please patch it in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE.
Future improvements can be made to make objtool smarter with respect to retpolines, but this is a good incremental improvement for now.
--- tools/objtool/check.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-)
tools/objtool/check.c +16 -20
@@ -543,18 +543,14 @@ static int add_call_destinations(struct objtool_file *file) dest_off = insn->offset + insn->len + insn->immediate; insn->call_dest =find_symbol_by_offset (insn->sec, dest_off); -/* -* FIXME: Thanks to retpolines, it's now considered -* normal for a function to call within itself. So -* disable this warning for now. -*/ -#if 0 -if (!insn->call_dest) { -WARN_FUNC ("can't find call dest symbol at offset 0x%lx" , - insn->sec, insn->offset, dest_off); + +if (!insn->call_dest && !insn->ignore) { +WARN_FUNC ("unsupported intra-function call" , + insn->sec, insn->offset); +WARN ("If this is a retpoline, please patch it in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE." );return -1 ; } -#endif + }else if (rela->sym->type ==STT_SECTION ) { insn->call_dest =find_symbol_by_offset (rela->sym->sec, rela->addend+4 );
@@ -648,6 +644,8 @@ static int handle_group_alt(struct objtool_file *file, last_new_insn = insn; + insn->ignore = orig_insn->ignore_alts; +if (insn->type !=INSN_JUMP_CONDITIONAL && insn->type !=INSN_JUMP_UNCONDITIONAL )continue ;
@@ -729,10 +727,6 @@ static int add_special_section_alts(struct objtool_file *file) goto out; } -/* Ignore retpoline alternatives. */ -if (orig_insn->ignore_alts) -continue ; - new_insn =NULL ;if (!special_alt->group || special_alt->new_len) { new_insn = find_insn(file, special_alt->new_sec,
@@ -1089,11 +1083,11 @@ static int decode_sections(struct objtool_file *file)if (ret)return ret; - ret =add_call_destinations (file); + ret =add_special_section_alts (file);if (ret)return ret; - ret =add_special_section_alts (file); + ret =add_call_destinations (file);if (ret)return ret;
@@ -1720,10 +1714,12 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, insn->visited = true; -list_for_each_entry (alt, &insn->alts, list) { - ret =validate_branch (file, alt->insn, state); -if (ret) -return 1 ; +if (!insn->ignore_alts) { +list_for_each_entry (alt, &insn->alts, list) { + ret =validate_branch (file, alt->insn, state); +if (ret) +return 1 ; + } }switch (insn->type) {
tip-bot for Josh Poimboeuf
merge bot[tip:x86/pti] objtool: Improve retpoline alternative handling
In reply to Josh Poimboeufobjtool: Improve retpoline alternative handling
Commit-ID: a845c7cf4b4cb5e9e3b2823867892b27646f3a98 Gitweb: https://git.kernel.org/tip/a845c7cf4b4cb5e9e3b2823867892b27646f3a98 Author: Josh Poimboeuf <[email protected]> AuthorDate: Mon, 29 Jan 2018 22:00:39 -0600 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Jan 2018 15:09:14 +0100
objtool: Improve retpoline alternative handling
Currently objtool requires all retpolines to be:
a) patched in with alternatives; and
b) annotated with ANNOTATE_NOSPEC_ALTERNATIVE.
If you forget to do both of the above, objtool segfaults trying to dereference a NULL 'insn->call_dest' pointer.
Avoid that situation and print a more helpful error message:
quirks.o: warning: objtool: efi_delete_dummy_variable()+0x99: unsupported intra-function call
quirks.o: warning: objtool: If this is a retpoline, please patch it in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE.
Future improvements can be made to make objtool smarter with respect to retpolines, but this is a good incremental improvement for now.
--- tools/objtool/check.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-)
tools/objtool/check.c +16 -20
@@ -543,18 +543,14 @@ static int add_call_destinations(struct objtool_file *file) dest_off = insn->offset + insn->len + insn->immediate; insn->call_dest =find_symbol_by_offset (insn->sec, dest_off); -/* -* FIXME: Thanks to retpolines, it's now considered -* normal for a function to call within itself. So -* disable this warning for now. -*/ -#if 0 -if (!insn->call_dest) { -WARN_FUNC ("can't find call dest symbol at offset 0x%lx" , - insn->sec, insn->offset, dest_off); + +if (!insn->call_dest && !insn->ignore) { +WARN_FUNC ("unsupported intra-function call" , + insn->sec, insn->offset); +WARN ("If this is a retpoline, please patch it in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE." );return -1 ; } -#endif + }else if (rela->sym->type ==STT_SECTION ) { insn->call_dest =find_symbol_by_offset (rela->sym->sec, rela->addend+4 );
@@ -648,6 +644,8 @@ static int handle_group_alt(struct objtool_file *file, last_new_insn = insn; + insn->ignore = orig_insn->ignore_alts; +if (insn->type !=INSN_JUMP_CONDITIONAL && insn->type !=INSN_JUMP_UNCONDITIONAL )continue ;
@@ -729,10 +727,6 @@ static int add_special_section_alts(struct objtool_file *file) goto out; } -/* Ignore retpoline alternatives. */ -if (orig_insn->ignore_alts) -continue ; - new_insn =NULL ;if (!special_alt->group || special_alt->new_len) { new_insn = find_insn(file, special_alt->new_sec,
@@ -1089,11 +1083,11 @@ static int decode_sections(struct objtool_file *file)if (ret)return ret; - ret =add_call_destinations (file); + ret =add_special_section_alts (file);if (ret)return ret; - ret =add_special_section_alts (file); + ret =add_call_destinations (file);if (ret)return ret;
@@ -1720,10 +1714,12 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, insn->visited = true; -list_for_each_entry (alt, &insn->alts, list) { - ret =validate_branch (file, alt->insn, state); -if (ret) -return 1 ; +if (!insn->ignore_alts) { +list_for_each_entry (alt, &insn->alts, list) { + ret =validate_branch (file, alt->insn, state); +if (ret) +return 1 ; + } }switch (insn->type) {
PATCH 2/4 objtool: Add support for alternatives at the end of a section
In reply to Josh PoimboeufNow that the previous patch gave objtool the ability to read retpoline alternatives, it shows a new warning:
arch/x86/entry/entry_64.o: warning: objtool: .entry_trampoline: don't know how to handle alternatives at end of section
This is due to the JMP_NOSPEC in entry_SYSCALL_64_trampoline().
Previously, objtool ignored this situation because it wasn't needed, and it would have required a bit of extra code. Now that this case exists, add proper support for it.
--- tools/objtool/check.c | 53 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 22 deletions(-)
tools/objtool/check.c +31 -22
@@ -594,7 +594,7 @@ static int handle_group_alt(struct objtool_file *file,struct instruction *orig_insn, struct instruction **new_insn) { - struct instruction *last_orig_insn, *last_new_insn, *insn, *fake_jump; + struct instruction *last_orig_insn, *last_new_insn, *insn, *fake_jump = NULL;unsigned long dest_off; last_orig_insn =NULL ;
@@ -610,28 +610,30 @@ static int handle_group_alt(struct objtool_file *file, last_orig_insn = insn; } -if (!next_insn_same_sec (file, last_orig_insn)) { -WARN ("%s: don't know how to handle alternatives at end of section" , - special_alt->orig_sec->name); -return -1 ; - } - - fake_jump =malloc (sizeof (*fake_jump)); -if (!fake_jump) { -WARN ("malloc failed" ); -return -1 ; +if (next_insn_same_sec (file, last_orig_insn)) { + fake_jump =malloc (sizeof (*fake_jump)); +if (!fake_jump) { +WARN ("malloc failed" ); +return -1 ; + } +memset (fake_jump,0 ,sizeof (*fake_jump)); +INIT_LIST_HEAD (&fake_jump->alts); +clear_insn_state (&fake_jump->state); + + fake_jump->sec = special_alt->new_sec; + fake_jump->offset =-1 ; + fake_jump->type =INSN_JUMP_UNCONDITIONAL ; + fake_jump->jump_dest =list_next_entry (last_orig_insn, list); + fake_jump->ignore = true; } -memset (fake_jump,0 ,sizeof (*fake_jump)); -INIT_LIST_HEAD (&fake_jump->alts); -clear_insn_state (&fake_jump->state); - - fake_jump->sec = special_alt->new_sec; - fake_jump->offset =-1 ; - fake_jump->type =INSN_JUMP_UNCONDITIONAL ; - fake_jump->jump_dest =list_next_entry (last_orig_insn, list); - fake_jump->ignore = true;if (!special_alt->new_len) { +if (!fake_jump) { +WARN ("%s: empty alternative at end of section" , + special_alt->orig_sec->name); +return -1 ; + } + *new_insn = fake_jump;return 0 ; }
@@ -654,8 +656,14 @@ static int handle_group_alt(struct objtool_file *file,continue ; dest_off = insn->offset + insn->len + insn->immediate; -if (dest_off == special_alt->new_off + special_alt->new_len) +if (dest_off == special_alt->new_off + special_alt->new_len) { +if (!fake_jump) { +WARN ("%s: alternative jump to end of section" , + special_alt->orig_sec->name); +return -1 ; + } insn->jump_dest = fake_jump; + }if (!insn->jump_dest) { WARN_FUNC("can't find alternative jump destination",
@@ -670,7 +678,8 @@ static int handle_group_alt(struct objtool_file *file,return -1 ; } -list_add (&fake_jump->list, &last_new_insn->list); +if (fake_jump) +list_add (&fake_jump->list, &last_new_insn->list);return 0 ; }
Signature · 1 line
2.14.3
tip-bot for Josh Poimboeuf
merge bot[tip:x86/pti] objtool: Add support for alternatives at the end of a section
objtool: Add support for alternatives at the end of a section
Commit-ID: b18cbc614b4ddca4019ecd2e6663d54bd495d413 Gitweb: https://git.kernel.org/tip/b18cbc614b4ddca4019ecd2e6663d54bd495d413 Author: Josh Poimboeuf <[email protected]> AuthorDate: Mon, 29 Jan 2018 22:00:40 -0600 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Jan 2018 07:55:05 +0100
objtool: Add support for alternatives at the end of a section
Now that the previous patch gave objtool the ability to read retpoline alternatives, it shows a new warning:
arch/x86/entry/entry_64.o: warning: objtool: .entry_trampoline: don't know how to handle alternatives at end of section
This is due to the JMP_NOSPEC in entry_SYSCALL_64_trampoline().
Previously, objtool ignored this situation because it wasn't needed, and it would have required a bit of extra code. Now that this case exists, add proper support for it.
--- tools/objtool/check.c | 53 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 22 deletions(-)
tools/objtool/check.c +31 -22
@@ -594,7 +594,7 @@ static int handle_group_alt(struct objtool_file *file,struct instruction *orig_insn, struct instruction **new_insn) { - struct instruction *last_orig_insn, *last_new_insn, *insn, *fake_jump; + struct instruction *last_orig_insn, *last_new_insn, *insn, *fake_jump = NULL;unsigned long dest_off; last_orig_insn =NULL ;
@@ -610,28 +610,30 @@ static int handle_group_alt(struct objtool_file *file, last_orig_insn = insn; } -if (!next_insn_same_sec (file, last_orig_insn)) { -WARN ("%s: don't know how to handle alternatives at end of section" , - special_alt->orig_sec->name); -return -1 ; - } - - fake_jump =malloc (sizeof (*fake_jump)); -if (!fake_jump) { -WARN ("malloc failed" ); -return -1 ; +if (next_insn_same_sec (file, last_orig_insn)) { + fake_jump =malloc (sizeof (*fake_jump)); +if (!fake_jump) { +WARN ("malloc failed" ); +return -1 ; + } +memset (fake_jump,0 ,sizeof (*fake_jump)); +INIT_LIST_HEAD (&fake_jump->alts); +clear_insn_state (&fake_jump->state); + + fake_jump->sec = special_alt->new_sec; + fake_jump->offset =-1 ; + fake_jump->type =INSN_JUMP_UNCONDITIONAL ; + fake_jump->jump_dest =list_next_entry (last_orig_insn, list); + fake_jump->ignore = true; } -memset (fake_jump,0 ,sizeof (*fake_jump)); -INIT_LIST_HEAD (&fake_jump->alts); -clear_insn_state (&fake_jump->state); - - fake_jump->sec = special_alt->new_sec; - fake_jump->offset =-1 ; - fake_jump->type =INSN_JUMP_UNCONDITIONAL ; - fake_jump->jump_dest =list_next_entry (last_orig_insn, list); - fake_jump->ignore = true;if (!special_alt->new_len) { +if (!fake_jump) { +WARN ("%s: empty alternative at end of section" , + special_alt->orig_sec->name); +return -1 ; + } + *new_insn = fake_jump;return 0 ; }
@@ -654,8 +656,14 @@ static int handle_group_alt(struct objtool_file *file,continue ; dest_off = insn->offset + insn->len + insn->immediate; -if (dest_off == special_alt->new_off + special_alt->new_len) +if (dest_off == special_alt->new_off + special_alt->new_len) { +if (!fake_jump) { +WARN ("%s: alternative jump to end of section" , + special_alt->orig_sec->name); +return -1 ; + } insn->jump_dest = fake_jump; + }if (!insn->jump_dest) { WARN_FUNC("can't find alternative jump destination",
@@ -670,7 +678,8 @@ static int handle_group_alt(struct objtool_file *file,return -1 ; } -list_add (&fake_jump->list, &last_new_insn->list); +if (fake_jump) +list_add (&fake_jump->list, &last_new_insn->list);return 0 ; }
tip-bot for Josh Poimboeuf
merge bot[tip:x86/pti] objtool: Add support for alternatives at the end of a section
In reply to Josh Poimboeufobjtool: Add support for alternatives at the end of a section
Commit-ID: 17bc33914bcc98ba3c6b426fd1c49587a25c0597 Gitweb: https://git.kernel.org/tip/17bc33914bcc98ba3c6b426fd1c49587a25c0597 Author: Josh Poimboeuf <[email protected]> AuthorDate: Mon, 29 Jan 2018 22:00:40 -0600 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Jan 2018 15:09:17 +0100
objtool: Add support for alternatives at the end of a section
Now that the previous patch gave objtool the ability to read retpoline alternatives, it shows a new warning:
arch/x86/entry/entry_64.o: warning: objtool: .entry_trampoline: don't know how to handle alternatives at end of section
This is due to the JMP_NOSPEC in entry_SYSCALL_64_trampoline().
Previously, objtool ignored this situation because it wasn't needed, and it would have required a bit of extra code. Now that this case exists, add proper support for it.
--- tools/objtool/check.c | 53 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 22 deletions(-)
tools/objtool/check.c +31 -22
@@ -594,7 +594,7 @@ static int handle_group_alt(struct objtool_file *file,struct instruction *orig_insn, struct instruction **new_insn) { - struct instruction *last_orig_insn, *last_new_insn, *insn, *fake_jump; + struct instruction *last_orig_insn, *last_new_insn, *insn, *fake_jump = NULL;unsigned long dest_off; last_orig_insn =NULL ;
@@ -610,28 +610,30 @@ static int handle_group_alt(struct objtool_file *file, last_orig_insn = insn; } -if (!next_insn_same_sec (file, last_orig_insn)) { -WARN ("%s: don't know how to handle alternatives at end of section" , - special_alt->orig_sec->name); -return -1 ; - } - - fake_jump =malloc (sizeof (*fake_jump)); -if (!fake_jump) { -WARN ("malloc failed" ); -return -1 ; +if (next_insn_same_sec (file, last_orig_insn)) { + fake_jump =malloc (sizeof (*fake_jump)); +if (!fake_jump) { +WARN ("malloc failed" ); +return -1 ; + } +memset (fake_jump,0 ,sizeof (*fake_jump)); +INIT_LIST_HEAD (&fake_jump->alts); +clear_insn_state (&fake_jump->state); + + fake_jump->sec = special_alt->new_sec; + fake_jump->offset =-1 ; + fake_jump->type =INSN_JUMP_UNCONDITIONAL ; + fake_jump->jump_dest =list_next_entry (last_orig_insn, list); + fake_jump->ignore = true; } -memset (fake_jump,0 ,sizeof (*fake_jump)); -INIT_LIST_HEAD (&fake_jump->alts); -clear_insn_state (&fake_jump->state); - - fake_jump->sec = special_alt->new_sec; - fake_jump->offset =-1 ; - fake_jump->type =INSN_JUMP_UNCONDITIONAL ; - fake_jump->jump_dest =list_next_entry (last_orig_insn, list); - fake_jump->ignore = true;if (!special_alt->new_len) { +if (!fake_jump) { +WARN ("%s: empty alternative at end of section" , + special_alt->orig_sec->name); +return -1 ; + } + *new_insn = fake_jump;return 0 ; }
@@ -654,8 +656,14 @@ static int handle_group_alt(struct objtool_file *file,continue ; dest_off = insn->offset + insn->len + insn->immediate; -if (dest_off == special_alt->new_off + special_alt->new_len) +if (dest_off == special_alt->new_off + special_alt->new_len) { +if (!fake_jump) { +WARN ("%s: alternative jump to end of section" , + special_alt->orig_sec->name); +return -1 ; + } insn->jump_dest = fake_jump; + }if (!insn->jump_dest) { WARN_FUNC("can't find alternative jump destination",
@@ -670,7 +678,8 @@ static int handle_group_alt(struct objtool_file *file,return -1 ; } -list_add (&fake_jump->list, &last_new_insn->list); +if (fake_jump) +list_add (&fake_jump->list, &last_new_insn->list);return 0 ; }
PATCH 3/4 objtool: Warn on stripped section symbol
In reply to Josh PoimboeufWith the following fix:
2a0098d70640 ("objtool: Fix seg fault with gold linker")
... a seg fault was avoided, but the original seg fault condition in objtool wasn't fixed. Replace the seg fault with an error message.
--- tools/objtool/orc_gen.c | 5 +++++ 1 file changed, 5 insertions(+)
tools/objtool/orc_gen.c +5 -0
@@ -98,6 +98,11 @@ static int create_orc_entry(struct section *u_sec, struct section *ip_relasec,struct orc_entry *orc;struct rela *rela; +if (!insn_sec->sym) { +WARN ("missing symbol for section %s" , insn_sec->name); +return -1 ; + } +/* populate ORC data */ orc = (struct orc_entry *)u_sec->data->d_buf + idx;memcpy (orc, o,sizeof (*orc));
Signature · 1 line
2.14.3
tip-bot for Josh Poimboeuf
merge bot[tip:x86/pti] objtool: Warn on stripped section symbol
objtool: Warn on stripped section symbol
Commit-ID: 2e9490ba5830a60a18a359192e938b2ad1710120 Gitweb: https://git.kernel.org/tip/2e9490ba5830a60a18a359192e938b2ad1710120 Author: Josh Poimboeuf <[email protected]> AuthorDate: Mon, 29 Jan 2018 22:00:41 -0600 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Jan 2018 07:55:05 +0100
objtool: Warn on stripped section symbol
With the following fix:
2a0098d70640 ("objtool: Fix seg fault with gold linker")
... a seg fault was avoided, but the original seg fault condition in objtool wasn't fixed. Replace the seg fault with an error message.
--- tools/objtool/orc_gen.c | 5 +++++ 1 file changed, 5 insertions(+)
tools/objtool/orc_gen.c +5 -0
@@ -98,6 +98,11 @@ static int create_orc_entry(struct section *u_sec, struct section *ip_relasec,struct orc_entry *orc;struct rela *rela; +if (!insn_sec->sym) { +WARN ("missing symbol for section %s" , insn_sec->name); +return -1 ; + } +/* populate ORC data */ orc = (struct orc_entry *)u_sec->data->d_buf + idx;memcpy (orc, o,sizeof (*orc));
tip-bot for Josh Poimboeuf
merge bot[tip:x86/pti] objtool: Warn on stripped section symbol
In reply to Josh Poimboeufobjtool: Warn on stripped section symbol
Commit-ID: 830c1e3d16b2c1733cd1ec9c8f4d47a398ae31bc Gitweb: https://git.kernel.org/tip/830c1e3d16b2c1733cd1ec9c8f4d47a398ae31bc Author: Josh Poimboeuf <[email protected]> AuthorDate: Mon, 29 Jan 2018 22:00:41 -0600 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Jan 2018 15:09:23 +0100
objtool: Warn on stripped section symbol
With the following fix:
2a0098d70640 ("objtool: Fix seg fault with gold linker")
... a seg fault was avoided, but the original seg fault condition in objtool wasn't fixed. Replace the seg fault with an error message.
--- tools/objtool/orc_gen.c | 5 +++++ 1 file changed, 5 insertions(+)
tools/objtool/orc_gen.c +5 -0
@@ -98,6 +98,11 @@ static int create_orc_entry(struct section *u_sec, struct section *ip_relasec,struct orc_entry *orc;struct rela *rela; +if (!insn_sec->sym) { +WARN ("missing symbol for section %s" , insn_sec->name); +return -1 ; + } +/* populate ORC data */ orc = (struct orc_entry *)u_sec->data->d_buf + idx;memcpy (orc, o,sizeof (*orc));
PATCH 4/4 objtool: Don't print '.tmp_' prefix for .o files
In reply to Josh PoimboeufWith the following commit:
2a0098d70640 ("objtool: Fix seg fault with gold linker")
... objtool warnings started showing the modversions '.tmp_' prefix in the .o file name, like:
arch/x86/mm/.tmp_mem_encrypt_boot.o: warning: objtool: sme_encrypt_execute()+0x48: indirect call found in RETPOLINE build
The prefix is confusing. Remove it from the printed 'objname' variable.
--- tools/objtool/check.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
tools/objtool/check.c +21 -0
@@ -1978,6 +1978,25 @@ static void cleanup(struct objtool_file *file)elf_close (file->elf); } +/* +* With CONFIG_MODVERSIONS, the object name has '.tmp_' prepended to it. +* After the file has been opened, remove the prefix so warnings will look +* sensible. +*/ +static void fix_objname (void ) +{ +char *s; + + s =strstr (objname,".tmp_" ); +if (!s) +return ; + +for (; s[5 ]; s++) + s[0 ] = s[5 ]; + + s[0 ] =0 ; +} +int check (const char *_objname,bool _no_fp,bool no_unreachable,bool orc) {struct objtool_file file;
@@ -1990,6 +2009,8 @@ int check(const char *_objname, bool _no_fp, bool no_unreachable, bool orc)if (!file.elf)return 1 ; +fix_objname (); +INIT_LIST_HEAD (&file.insn_list);hash_init (file.insn_hash); file.whitelist =find_section_by_name (file.elf,".discard.func_stack_frame_non_standard" );
Signature · 1 line
2.14.3
tip-bot for Josh Poimboeuf
merge bot[tip:x86/pti] objtool: Don't print '.tmp_' prefix for .o files when CONFIG_MODVERSIONS=y
objtool: Don't print '.tmp_' prefix for .o files when CONFIG_MODVERSIONS=y
Commit-ID: dfcb8dae5d28b7cf259b367ed9ccbc1296284e0d Gitweb: https://git.kernel.org/tip/dfcb8dae5d28b7cf259b367ed9ccbc1296284e0d Author: Josh Poimboeuf <[email protected]> AuthorDate: Mon, 29 Jan 2018 22:00:42 -0600 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Jan 2018 07:55:05 +0100
objtool: Don't print '.tmp_' prefix for .o files when CONFIG_MODVERSIONS=y
With the following commit:
2a0098d70640 ("objtool: Fix seg fault with gold linker")
... objtool warnings started showing the modversions '.tmp_' prefix in the .o file name, like:
arch/x86/mm/.tmp_mem_encrypt_boot.o: warning: objtool: sme_encrypt_execute()+0x48: indirect call found in RETPOLINE build
The prefix is confusing. Remove it from the printed 'objname' variable.
--- tools/objtool/check.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
tools/objtool/check.c +21 -0
@@ -1978,6 +1978,25 @@ static void cleanup(struct objtool_file *file)elf_close (file->elf); } +/* +* With CONFIG_MODVERSIONS=y, the object name has '.tmp_' prepended to it. +* After the file has been opened, remove the prefix so warnings will look +* sensible. +*/ +static void fix_objname (void ) +{ +char *s; + + s =strstr (objname,".tmp_" ); +if (!s) +return ; + +for (; s[5 ]; s++) + s[0 ] = s[5 ]; + + s[0 ] =0 ; +} +int check (const char *_objname,bool _no_fp,bool no_unreachable,bool orc) {struct objtool_file file;
@@ -1990,6 +2009,8 @@ int check(const char *_objname, bool _no_fp, bool no_unreachable, bool orc)if (!file.elf)return 1 ; +fix_objname (); +INIT_LIST_HEAD (&file.insn_list);hash_init (file.insn_hash); file.whitelist =find_section_by_name (file.elf,".discard.func_stack_frame_non_standard" );
Peter Zijlstra
maintainerRe: [PATCH 4/4] objtool: Don't print '.tmp_' prefix for .o files
In reply to Josh PoimboeufQuoted 10 lines · Josh Poimboeuf
With the following commit:
2a0098d70640("objtool: Fix seg fault with gold linker")... objtool warnings started showing the modversions '.tmp_' prefix in the .o file name, like:
arch/x86/mm/.tmp_mem_encrypt_boot.o: warning: objtool: sme_encrypt_execute()+0x48: indirect call found in RETPOLINE buildThe prefix is confusing. Remove it from the printed 'objname' variable.
This patch actually tripped me up today. Turns out I have both:
build/kernel/sched/core.o
build/kernel/sched/.tmp_core.o
and ended up staring at the wrong file and going WTF because the offsets didn't match.
I much prefer the actual real filename printed. Yes the .tmp_ crud is weird, but at least its the real file.
Re: [PATCH 4/4] objtool: Don't print '.tmp_' prefix for .o files
Quoted 14 lines · Peter Zijlstra
Quoted 10 lines · Josh Poimboeuf
With the following commit:
2a0098d70640("objtool: Fix seg fault with gold linker")... objtool warnings started showing the modversions '.tmp_' prefix in the .o file name, like:
arch/x86/mm/.tmp_mem_encrypt_boot.o: warning: objtool: sme_encrypt_execute()+0x48: indirect call found in RETPOLINE buildThe prefix is confusing. Remove it from the printed 'objname' variable.
This patch actually tripped me up today. Turns out I have both:
Quoted 8 lines
build/kernel/sched/core.o
build/kernel/sched/.tmp_core.oand ended up staring at the wrong file and going WTF because the offsets didn't match.
I much prefer the actual real filename printed. Yes the .tmp_ crud is weird, but at least its the real file.
Good point, sorry for tripping you up there. I see it's been removed from -tip now. We should look at changing how the modversions stuff does its renaming so this will be less confusing.
Signature · 1 line
Josh
Re: [PATCH 4/4] objtool: Don't print '.tmp_' prefix for .o files
Quoted 18 lines · Josh Poimboeuf
Quoted 17 lines · Peter Zijlstra
Quoted 10 lines · Josh Poimboeuf
With the following commit:
2a0098d70640("objtool: Fix seg fault with gold linker")... objtool warnings started showing the modversions '.tmp_' prefix in the .o file name, like:
arch/x86/mm/.tmp_mem_encrypt_boot.o: warning: objtool: sme_encrypt_execute()+0x48: indirect call found in RETPOLINE buildThe prefix is confusing. Remove it from the printed 'objname' variable.
This patch actually tripped me up today. Turns out I have both:
build/kernel/sched/core.o
build/kernel/sched/.tmp_core.o
Quoted 8 lines
Quoted 5 lines
and ended up staring at the wrong file and going WTF because the offsets didn't match.
I much prefer the actual real filename printed. Yes the .tmp_ crud is weird, but at least its the real file.
Good point, sorry for tripping you up there. I see it's been removed from -tip now. [...]
Yeah, and we had to rebase to a later kernel anyway so it was easy to remove it for now.
Thanks,
Ingo