An error report from elf_init_reloc_text_sym() doesn't say what list of
symbols it is working on. Include this on the caller's side so it can be
reported when pathological conditions are encountered.
Signed-off-by: Kees Cook <[email protected]>
---
I added this to confirm debugging of
https://lore.kernel.org/lkml/[email protected]/
---
Cc: Josh Poimboeuf <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Youling Tang <[email protected]>
Cc: Jinyang He <[email protected]>
Cc: Huacai Chen <[email protected]>
Cc: Tiezhu Yang <[email protected]>
---
tools/objtool/arch/loongarch/orc.c | 3 ++-
tools/objtool/arch/x86/orc.c | 3 ++-
tools/objtool/check.c | 21 +++++++++++++--------
tools/objtool/elf.c | 7 ++++---
tools/objtool/include/objtool/elf.h | 3 ++-
5 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/tools/objtool/arch/loongarch/orc.c b/tools/objtool/arch/loongarch/orc.c
index 873536d009d9..5ed9c1bb2de3 100644
--- a/tools/objtool/arch/loongarch/orc.c
+++ b/tools/objtool/arch/loongarch/orc.c
@@ -110,7 +110,8 @@ int write_orc_entry(struct elf *elf, struct section *orc_sec,
memcpy(orc, o, sizeof(*orc));
/* populate reloc for ip */
- if (!elf_init_reloc_text_sym(elf, ip_sec, idx * sizeof(int), idx,
+ if (!elf_init_reloc_text_sym("orc_list", elf, ip_sec,
+ idx * sizeof(int), idx,
insn_sec, insn_off))
return -1;
diff --git a/tools/objtool/arch/x86/orc.c b/tools/objtool/arch/x86/orc.c
index b6cd943e87f9..68a78ad0d99e 100644
--- a/tools/objtool/arch/x86/orc.c
+++ b/tools/objtool/arch/x86/orc.c
@@ -111,7 +111,8 @@ int write_orc_entry(struct elf *elf, struct section *orc_sec,
orc->bp_offset = bswap_if_needed(elf, orc->bp_offset);
/* populate reloc for ip */
- if (!elf_init_reloc_text_sym(elf, ip_sec, idx * sizeof(int), idx,
+ if (!elf_init_reloc_text_sym("orc_list", elf, ip_sec,
+ idx * sizeof(int), idx,
insn_sec, insn_off))
return -1;
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 0a33d9195b7a..e7f7300ba88b 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -701,7 +701,7 @@ static int create_static_call_sections(struct objtool_file *file)
list_for_each_entry(insn, &file->static_call_list, call_node) {
/* populate reloc for 'addr' */
- if (!elf_init_reloc_text_sym(file->elf, sec,
+ if (!elf_init_reloc_text_sym("static_call_list", file->elf, sec,
idx * sizeof(*site), idx * 2,
insn->sec, insn->offset))
return -1;
@@ -782,7 +782,8 @@ static int create_retpoline_sites_sections(struct objtool_file *file)
idx = 0;
list_for_each_entry(insn, &file->retpoline_call_list, call_node) {
- if (!elf_init_reloc_text_sym(file->elf, sec,
+ if (!elf_init_reloc_text_sym("retpoline_call_list",
+ file->elf, sec,
idx * sizeof(int), idx,
insn->sec, insn->offset))
return -1;
@@ -820,7 +821,8 @@ static int create_return_sites_sections(struct objtool_file *file)
idx = 0;
list_for_each_entry(insn, &file->return_thunk_list, call_node) {
- if (!elf_init_reloc_text_sym(file->elf, sec,
+ if (!elf_init_reloc_text_sym("return_thunk_list",
+ file->elf, sec,
idx * sizeof(int), idx,
insn->sec, insn->offset))
return -1;
@@ -874,7 +876,8 @@ static int create_ibt_endbr_seal_sections(struct objtool_file *file)
!strcmp(sym->name, "cleanup_module")))
WARN("%s(): not an indirect call target", sym->name);
- if (!elf_init_reloc_text_sym(file->elf, sec,
+ if (!elf_init_reloc_text_sym("endbr_list",
+ file->elf, sec,
idx * sizeof(int), idx,
insn->sec, insn->offset))
return -1;
@@ -922,7 +925,7 @@ static int create_cfi_sections(struct objtool_file *file)
if (strncmp(sym->name, "__cfi_", 6))
continue;
- if (!elf_init_reloc_text_sym(file->elf, sec,
+ if (!elf_init_reloc_text_sym(".cfi_sites", file->elf, sec,
idx * sizeof(unsigned int), idx,
sym->sec, sym->offset))
return -1;
@@ -966,8 +969,10 @@ static int create_mcount_loc_sections(struct objtool_file *file)
struct reloc *reloc;
- reloc = elf_init_reloc_text_sym(file->elf, sec, idx * addr_size, idx,
- insn->sec, insn->offset);
+ reloc = elf_init_reloc_text_sym("mcount_loc_list",
+ file->elf, sec,
+ idx * addr_size, idx,
+ insn->sec, insn->offset);
if (!reloc)
return -1;
@@ -1007,7 +1012,7 @@ static int create_direct_call_sections(struct objtool_file *file)
idx = 0;
list_for_each_entry(insn, &file->call_list, call_node) {
- if (!elf_init_reloc_text_sym(file->elf, sec,
+ if (!elf_init_reloc_text_sym("call_list", file->elf, sec,
idx * sizeof(unsigned int), idx,
insn->sec, insn->offset))
return -1;
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 3d27983dc908..8615672e2ff9 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -881,7 +881,8 @@ static struct reloc *elf_init_reloc(struct elf *elf, struct section *rsec,
return reloc;
}
-struct reloc *elf_init_reloc_text_sym(struct elf *elf, struct section *sec,
+struct reloc *elf_init_reloc_text_sym(const char *origin,
+ struct elf *elf, struct section *sec,
unsigned long offset,
unsigned int reloc_idx,
struct section *insn_sec,
@@ -891,8 +892,8 @@ struct reloc *elf_init_reloc_text_sym(struct elf *elf, struct section *sec,
int addend = insn_off;
if (!(insn_sec->sh.sh_flags & SHF_EXECINSTR)) {
- WARN("bad call to %s() for data symbol %s",
- __func__, sym->name);
+ WARN("bad call to %s() for %s symbol %s",
+ __func__, origin, sym->name);
return NULL;
}
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index 2b8a69de4db8..d026e707806d 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -115,7 +115,8 @@ struct section *elf_create_section_pair(struct elf *elf, const char *name,
struct symbol *elf_create_prefix_symbol(struct elf *elf, struct symbol *orig, long size);
-struct reloc *elf_init_reloc_text_sym(struct elf *elf, struct section *sec,
+struct reloc *elf_init_reloc_text_sym(const char *origin,
+ struct elf *elf, struct section *sec,
unsigned long offset,
unsigned int reloc_idx,
struct section *insn_sec,
--
2.34.1
On Tue, Apr 30, 2024 at 04:51:07PM -0700, Kees Cook wrote:
> @@ -891,8 +892,8 @@ struct reloc *elf_init_reloc_text_sym(struct elf *elf, struct section *sec,
> int addend = insn_off;
>
> if (!(insn_sec->sh.sh_flags & SHF_EXECINSTR)) {
> - WARN("bad call to %s() for data symbol %s",
> - __func__, sym->name);
> + WARN("bad call to %s() for %s symbol %s",
> + __func__, origin, sym->name);
> return NULL;
Thanks for the patch.
That warning was already phrased pretty awkwardly which was probably
part of the confusion. It could be rephrased to make it a little
clearer:
Something like:
.cfi_sites: unexpected reference to non-executable symbol 'execute_location'
And ".cfi_sites" is already in 'sec->name', so you wouldn't need to add
the new 'origin' arg.
--
Josh
On Sat, May 04, 2024 at 03:24:02PM -0700, Josh Poimboeuf wrote:
> On Tue, Apr 30, 2024 at 04:51:07PM -0700, Kees Cook wrote:
> > @@ -891,8 +892,8 @@ struct reloc *elf_init_reloc_text_sym(struct elf *elf, struct section *sec,
> > int addend = insn_off;
> >
> > if (!(insn_sec->sh.sh_flags & SHF_EXECINSTR)) {
> > - WARN("bad call to %s() for data symbol %s",
> > - __func__, sym->name);
> > + WARN("bad call to %s() for %s symbol %s",
> > + __func__, origin, sym->name);
> > return NULL;
>
> Thanks for the patch.
>
> That warning was already phrased pretty awkwardly which was probably
> part of the confusion. It could be rephrased to make it a little
> clearer:
>
> Something like:
>
> .cfi_sites: unexpected reference to non-executable symbol 'execute_location'
>
> And ".cfi_sites" is already in 'sec->name', so you wouldn't need to add
> the new 'origin' arg.
What's so odd is that "execute_location" wasn't being reported at all.
Just ".rodata":
vmlinux.o: warning: objtool: bad call to elf_init_reloc_text_sym() for data symbol .rodata
But yes, sec->name has what I want, so I can do this easily:
- WARN("bad call to %s() for data symbol %s",
- __func__, sym->name);
+ WARN("bad call to %s() for %s symbol %s",
+ __func__, sec->name, sym->name);
vmlinux.o: warning: objtool: bad call to elf_init_reloc_text_sym() for .cfi_sites symbol .rodata
I think the symbol is missing because this is coming from
create_cfi_sections()/elf_create_section_pair().
Regardless, I'll send a v2...
Thanks!
--
Kees Cook