* [PATCH v2 0/2] objtool: altinstructions fix and cleanup
@ 2021-08-20 19:44 Joe Lawrence
2021-08-20 19:44 ` [PATCH v2 1/2] objtool: make .altinstructions section entry size consistent Joe Lawrence
2021-08-20 19:44 ` [PATCH v2 2/2] objtool: remove redundant len value from struct section Joe Lawrence
0 siblings, 2 replies; 4+ messages in thread
From: Joe Lawrence @ 2021-08-20 19:44 UTC (permalink / raw)
To: x86, Josh Poimboeuf, Peter Zijlstra; +Cc: linux-kernel, Miroslav Benes
Hi Josh,
Here's an updated patchset with suggestions from the initial post.
For the second patch, I assumed that the two values were intended to be
synonymous, so it was effectively a big s/sec->len/sec->sh_size.
v2:
- drop the sec->len update from the first patch [josh]
- rip out sec->len as suggested [josh]
Joe Lawrence (2):
objtool: make .altinstructions section entry size consistent
objtool: remove redundant len value from struct section
tools/objtool/arch/x86/decode.c | 4 ++--
tools/objtool/check.c | 19 ++++++++++---------
tools/objtool/elf.c | 15 +++++++--------
tools/objtool/include/objtool/arch.h | 2 +-
tools/objtool/include/objtool/elf.h | 1 -
tools/objtool/orc_gen.c | 2 +-
tools/objtool/special.c | 4 ++--
7 files changed, 23 insertions(+), 24 deletions(-)
--
2.26.3
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/2] objtool: make .altinstructions section entry size consistent
2021-08-20 19:44 [PATCH v2 0/2] objtool: altinstructions fix and cleanup Joe Lawrence
@ 2021-08-20 19:44 ` Joe Lawrence
2021-08-20 19:44 ` [PATCH v2 2/2] objtool: remove redundant len value from struct section Joe Lawrence
1 sibling, 0 replies; 4+ messages in thread
From: Joe Lawrence @ 2021-08-20 19:44 UTC (permalink / raw)
To: x86, Josh Poimboeuf, Peter Zijlstra; +Cc: linux-kernel, Miroslav Benes
commit e31694e0a7a7 ("objtool: Don't make .altinstructions writable")
aligned objtool-created and kernel-created .altinstructions section
flags, but there remains a minor discrepency in their use of a section
entry size: objtool sets one while the kernel build does not.
While sh_entsize of sizeof(struct alt_instr) seems intuitive, this small
deviation can cause failures with external tooling (kpatch-build).
Fix this by creating new .altinstructions sections with sh_entsize of 0
and then later updating sec->sh_size as alternatives are added to the
section. An added benefit is avoiding the data descriptor and buffer
created by elf_create_section(), but previously unused by
elf_add_alternative().
Fixes: 9bc0bb50727c ("objtool/x86: Rewrite retpoline thunk calls")
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
tools/objtool/arch/x86/decode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index bc821056aba9..0893436cc09f 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -684,7 +684,7 @@ static int elf_add_alternative(struct elf *elf,
sec = find_section_by_name(elf, ".altinstructions");
if (!sec) {
sec = elf_create_section(elf, ".altinstructions",
- SHF_ALLOC, size, 0);
+ SHF_ALLOC, 0, 0);
if (!sec) {
WARN_ELF("elf_create_section");
--
2.26.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] objtool: remove redundant len value from struct section
2021-08-20 19:44 [PATCH v2 0/2] objtool: altinstructions fix and cleanup Joe Lawrence
2021-08-20 19:44 ` [PATCH v2 1/2] objtool: make .altinstructions section entry size consistent Joe Lawrence
@ 2021-08-20 19:44 ` Joe Lawrence
2021-08-22 1:17 ` Joe Lawrence
1 sibling, 1 reply; 4+ messages in thread
From: Joe Lawrence @ 2021-08-20 19:44 UTC (permalink / raw)
To: x86, Josh Poimboeuf, Peter Zijlstra; +Cc: linux-kernel, Miroslav Benes
The section structure already contains sh_size, so just remove the extra
'len' member that requires extra mirroring and potential confusion.
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
tools/objtool/arch/x86/decode.c | 2 +-
tools/objtool/check.c | 19 ++++++++++---------
tools/objtool/elf.c | 15 +++++++--------
tools/objtool/include/objtool/arch.h | 2 +-
tools/objtool/include/objtool/elf.h | 1 -
tools/objtool/orc_gen.c | 2 +-
tools/objtool/special.c | 4 ++--
7 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 0893436cc09f..a33756ff2672 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -104,7 +104,7 @@ unsigned long arch_jump_destination(struct instruction *insn)
int arch_decode_instruction(const struct elf *elf, const struct section *sec,
unsigned long offset, unsigned int maxlen,
- unsigned int *len, enum insn_type *type,
+ unsigned long *len, enum insn_type *type,
unsigned long *immediate,
struct list_head *ops_list)
{
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index e5947fbb9e7a..1d7dbd469896 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -292,7 +292,7 @@ static int decode_instructions(struct objtool_file *file)
!strcmp(sec->name, ".entry.text"))
sec->noinstr = true;
- for (offset = 0; offset < sec->len; offset += insn->len) {
+ for (offset = 0; offset < sec->sh.sh_size; offset += insn->len) {
insn = malloc(sizeof(*insn));
if (!insn) {
WARN("malloc failed");
@@ -307,8 +307,9 @@ static int decode_instructions(struct objtool_file *file)
insn->offset = offset;
ret = arch_decode_instruction(file->elf, sec, offset,
- sec->len - offset,
- &insn->len, &insn->type,
+ sec->sh.sh_size - offset,
+ &insn->sec->sh.sh_size,
+ &insn->type,
&insn->immediate,
&insn->stack_ops);
if (ret)
@@ -349,9 +350,9 @@ static struct instruction *find_last_insn(struct objtool_file *file,
{
struct instruction *insn = NULL;
unsigned int offset;
- unsigned int end = (sec->len > 10) ? sec->len - 10 : 0;
+ unsigned int end = (sec->sh.sh_size > 10) ? sec->sh.sh_size - 10 : 0;
- for (offset = sec->len - 1; offset >= end && !insn; offset--)
+ for (offset = sec->sh.sh_size - 1; offset >= end && !insn; offset--)
insn = find_insn(file, sec, offset);
return insn;
@@ -389,7 +390,7 @@ static int add_dead_ends(struct objtool_file *file)
insn = find_insn(file, reloc->sym->sec, reloc->addend);
if (insn)
insn = list_prev_entry(insn, list);
- else if (reloc->addend == reloc->sym->sec->len) {
+ else if (reloc->addend == reloc->sym->sec->sh.sh_size) {
insn = find_last_insn(file, reloc->sym->sec);
if (!insn) {
WARN("can't find unreachable insn at %s+0x%x",
@@ -424,7 +425,7 @@ static int add_dead_ends(struct objtool_file *file)
insn = find_insn(file, reloc->sym->sec, reloc->addend);
if (insn)
insn = list_prev_entry(insn, list);
- else if (reloc->addend == reloc->sym->sec->len) {
+ else if (reloc->addend == reloc->sym->sec->sh.sh_size) {
insn = find_last_insn(file, reloc->sym->sec);
if (!insn) {
WARN("can't find reachable insn at %s+0x%x",
@@ -1561,14 +1562,14 @@ static int read_unwind_hints(struct objtool_file *file)
return -1;
}
- if (sec->len % sizeof(struct unwind_hint)) {
+ if (sec->sh.sh_size % sizeof(struct unwind_hint)) {
WARN("struct unwind_hint size mismatch");
return -1;
}
file->hints = true;
- for (i = 0; i < sec->len / sizeof(struct unwind_hint); i++) {
+ for (i = 0; i < sec->sh.sh_size / sizeof(struct unwind_hint); i++) {
hint = (struct unwind_hint *)sec->data->d_buf + i;
reloc = find_reloc_by_dest(file->elf, sec, i * sizeof(*hint));
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 8676c7598728..7f66b542f53e 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -286,10 +286,9 @@ static int read_sections(struct elf *elf)
return -1;
}
}
- sec->len = sec->sh.sh_size;
if (sec->sh.sh_flags & SHF_EXECINSTR)
- elf->text_size += sec->len;
+ elf->text_size += sec->sh.sh_size;
list_add_tail(&sec->list, &elf->sections);
elf_hash_add(section, &sec->hash, sec->idx);
@@ -734,8 +733,8 @@ static int elf_add_string(struct elf *elf, struct section *strtab, char *str)
data->d_size = strlen(str) + 1;
data->d_align = 1;
- len = strtab->len;
- strtab->len += data->d_size;
+ len = strtab->sh.sh_size;
+ strtab->sh.sh_size += data->d_size;
strtab->changed = true;
return len;
@@ -790,9 +789,9 @@ struct symbol *elf_create_undef_symbol(struct elf *elf, const char *name)
data->d_align = 1;
data->d_type = ELF_T_SYM;
- sym->idx = symtab->len / sizeof(sym->sym);
+ sym->idx = symtab->sh.sh_size / sizeof(sym->sym);
- symtab->len += data->d_size;
+ symtab->sh.sh_size += data->d_size;
symtab->changed = true;
symtab_shndx = find_section_by_name(elf, ".symtab_shndx");
@@ -814,7 +813,7 @@ struct symbol *elf_create_undef_symbol(struct elf *elf, const char *name)
data->d_align = 4;
data->d_type = ELF_T_WORD;
- symtab_shndx->len += 4;
+ symtab_shndx->sh.sh_size += 4;
symtab_shndx->changed = true;
}
@@ -855,7 +854,7 @@ struct section *elf_create_section(struct elf *elf, const char *name,
}
sec->idx = elf_ndxscn(s);
- sec->len = size;
+ sec->sh.sh_size = size;
sec->changed = true;
sec->data = elf_newdata(s);
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index 062bb6e9b865..958617ce03ef 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -71,7 +71,7 @@ void arch_initial_func_cfi_state(struct cfi_init_state *state);
int arch_decode_instruction(const struct elf *elf, const struct section *sec,
unsigned long offset, unsigned int maxlen,
- unsigned int *len, enum insn_type *type,
+ unsigned long *len, enum insn_type *type,
unsigned long *immediate,
struct list_head *ops_list);
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index e34395047530..075d8291b854 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -38,7 +38,6 @@ struct section {
Elf_Data *data;
char *name;
int idx;
- unsigned int len;
bool changed, text, rodata, noinstr;
};
diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c
index dc9b7dd314b0..b5865e2450cb 100644
--- a/tools/objtool/orc_gen.c
+++ b/tools/objtool/orc_gen.c
@@ -204,7 +204,7 @@ int orc_create(struct objtool_file *file)
/* Add a section terminator */
if (!empty) {
- orc_list_add(&orc_list, &null, sec, sec->len);
+ orc_list_add(&orc_list, &null, sec, sec->sh.sh_size);
nr++;
}
}
diff --git a/tools/objtool/special.c b/tools/objtool/special.c
index bc925cf19e2d..7a0c49421cd8 100644
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -159,13 +159,13 @@ int special_get_alts(struct elf *elf, struct list_head *alts)
if (!sec)
continue;
- if (sec->len % entry->size != 0) {
+ if (sec->sh.sh_size % entry->size != 0) {
WARN("%s size not a multiple of %d",
sec->name, entry->size);
return -1;
}
- nr_entries = sec->len / entry->size;
+ nr_entries = sec->sh.sh_size / entry->size;
for (idx = 0; idx < nr_entries; idx++) {
alt = malloc(sizeof(*alt));
--
2.26.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] objtool: remove redundant len value from struct section
2021-08-20 19:44 ` [PATCH v2 2/2] objtool: remove redundant len value from struct section Joe Lawrence
@ 2021-08-22 1:17 ` Joe Lawrence
0 siblings, 0 replies; 4+ messages in thread
From: Joe Lawrence @ 2021-08-22 1:17 UTC (permalink / raw)
To: x86, Josh Poimboeuf, Peter Zijlstra
Cc: linux-kernel, Miroslav Benes, Andy Lavr
On 8/20/21 3:44 PM, Joe Lawrence wrote:
> [ ... snip ... ]
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> [ ... snip ... ]
> @@ -307,8 +307,9 @@ static int decode_instructions(struct objtool_file *file)
> insn->offset = offset;
>
> ret = arch_decode_instruction(file->elf, sec, offset,
> - sec->len - offset,
> - &insn->len, &insn->type,
> + sec->sh.sh_size - offset,
> + &insn->sec->sh.sh_size,
> + &insn->type,
> &insn->immediate,
> &insn->stack_ops);
> if (ret)
Nack! Not sure what I was thinking here. It was Friday afternoon, so I
probably wans't.
Changing insn->len to insn->sec->sh.sh_size is obviously bogus. I'll
correct this part of the patch, do better testing and post v3 on Monday.
Thanks to Andy for reporting.
--
Joe
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-08-22 1:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-20 19:44 [PATCH v2 0/2] objtool: altinstructions fix and cleanup Joe Lawrence
2021-08-20 19:44 ` [PATCH v2 1/2] objtool: make .altinstructions section entry size consistent Joe Lawrence
2021-08-20 19:44 ` [PATCH v2 2/2] objtool: remove redundant len value from struct section Joe Lawrence
2021-08-22 1:17 ` Joe Lawrence
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).