| Message ID | 20210201172530.1141087-5-gprocida@google.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF |
| Headers | show |
| Series | BTF ELF writing changes | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Mon, Feb 1, 2021 at 9:26 AM Giuliano Procida <gprocida@google.com> wrote: > > This is to avoid misaligned access to BTF type structs when > memory-mapping ELF sections. > > Signed-off-by: Giuliano Procida <gprocida@google.com> > --- > libbtf.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/libbtf.c b/libbtf.c > index 048a873..ae99a93 100644 > --- a/libbtf.c > +++ b/libbtf.c > @@ -755,7 +755,13 @@ static int btf_elf__write(const char *filename, struct btf *btf) > * This actually happens in practice with vmlinux which has .strtab > * after .shstrtab, resulting in a (small) hole the size of the original > * .shstrtab. > + * > + * We'll align .BTF to 8 bytes to cater for all architectures. It'd be > + * nice if we could fetch this value from somewhere. The BTF > + * specification does not discuss alignment and its trailing string > + * table is not currently padded to any particular alignment. > */ > + const size_t btf_alignment = 8; > > /* > * First we look if there was already a .BTF section present and > @@ -847,8 +853,8 @@ static int btf_elf__write(const char *filename, struct btf *btf) > elf_flagdata(btf_data, ELF_C_SET, ELF_F_DIRTY); > > /* Update .BTF section in the SHT */ > - size_t new_btf_offset = high_water_mark; > - size_t new_btf_size = raw_btf_size; > + size_t new_btf_offset = roundup(high_water_mark, btf_alignment); > + size_t new_btf_size = roundup(raw_btf_size, btf_alignment); > GElf_Shdr btf_shdr_mem; > GElf_Shdr *btf_shdr = gelf_getshdr(btf_scn, &btf_shdr_mem); > if (!btf_shdr) { > @@ -856,6 +862,7 @@ static int btf_elf__write(const char *filename, struct btf *btf) > __func__, elf_errmsg(elf_errno())); > goto out; > } > + btf_shdr->sh_addralign = btf_alignment; if we set just this and let libelf do the layout, would libelf ensure 8-byte alignment of .BTF section inside the ELF file? > btf_shdr->sh_entsize = 0; > btf_shdr->sh_flags = SHF_ALLOC; > if (dot_btf_offset) > @@ -926,6 +933,7 @@ static int btf_elf__write(const char *filename, struct btf *btf) > pht[phnum].p_memsz = pht[phnum].p_filesz = btf_shdr->sh_size; > pht[phnum].p_vaddr = pht[phnum].p_paddr = 0; > pht[phnum].p_flags = PF_R; > + pht[phnum].p_align = btf_alignment; > void *phdr = gelf_newphdr(elf, phnum+1); > if (!phdr) { > fprintf(stderr, "%s: gelf_newphdr failed: %s\n", > -- > 2.30.0.365.g02bc693789-goog >
Hi. On Thu, 4 Feb 2021 at 04:10, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Mon, Feb 1, 2021 at 9:26 AM Giuliano Procida <gprocida@google.com> wrote: > > > > This is to avoid misaligned access to BTF type structs when > > memory-mapping ELF sections. > > > > Signed-off-by: Giuliano Procida <gprocida@google.com> > > --- > > libbtf.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/libbtf.c b/libbtf.c > > index 048a873..ae99a93 100644 > > --- a/libbtf.c > > +++ b/libbtf.c > > @@ -755,7 +755,13 @@ static int btf_elf__write(const char *filename, struct btf *btf) > > * This actually happens in practice with vmlinux which has .strtab > > * after .shstrtab, resulting in a (small) hole the size of the original > > * .shstrtab. > > + * > > + * We'll align .BTF to 8 bytes to cater for all architectures. It'd be > > + * nice if we could fetch this value from somewhere. The BTF > > + * specification does not discuss alignment and its trailing string > > + * table is not currently padded to any particular alignment. > > */ > > + const size_t btf_alignment = 8; > > > > /* > > * First we look if there was already a .BTF section present and > > @@ -847,8 +853,8 @@ static int btf_elf__write(const char *filename, struct btf *btf) > > elf_flagdata(btf_data, ELF_C_SET, ELF_F_DIRTY); > > > > /* Update .BTF section in the SHT */ > > - size_t new_btf_offset = high_water_mark; > > - size_t new_btf_size = raw_btf_size; > > + size_t new_btf_offset = roundup(high_water_mark, btf_alignment); > > + size_t new_btf_size = roundup(raw_btf_size, btf_alignment); > > GElf_Shdr btf_shdr_mem; > > GElf_Shdr *btf_shdr = gelf_getshdr(btf_scn, &btf_shdr_mem); > > if (!btf_shdr) { > > @@ -856,6 +862,7 @@ static int btf_elf__write(const char *filename, struct btf *btf) > > __func__, elf_errmsg(elf_errno())); > > goto out; > > } > > + btf_shdr->sh_addralign = btf_alignment; > > if we set just this and let libelf do the layout, would libelf ensure > 8-byte alignment of .BTF section inside the ELF file? > Yes. I'll follow up with a trivial patch that does that. > > btf_shdr->sh_entsize = 0; > > btf_shdr->sh_flags = SHF_ALLOC; > > if (dot_btf_offset) > > @@ -926,6 +933,7 @@ static int btf_elf__write(const char *filename, struct btf *btf) > > pht[phnum].p_memsz = pht[phnum].p_filesz = btf_shdr->sh_size; > > pht[phnum].p_vaddr = pht[phnum].p_paddr = 0; > > pht[phnum].p_flags = PF_R; > > + pht[phnum].p_align = btf_alignment; > > void *phdr = gelf_newphdr(elf, phnum+1); > > if (!phdr) { > > fprintf(stderr, "%s: gelf_newphdr failed: %s\n", > > -- > > 2.30.0.365.g02bc693789-goog > >
diff --git a/libbtf.c b/libbtf.c index 048a873..ae99a93 100644 --- a/libbtf.c +++ b/libbtf.c @@ -755,7 +755,13 @@ static int btf_elf__write(const char *filename, struct btf *btf) * This actually happens in practice with vmlinux which has .strtab * after .shstrtab, resulting in a (small) hole the size of the original * .shstrtab. + * + * We'll align .BTF to 8 bytes to cater for all architectures. It'd be + * nice if we could fetch this value from somewhere. The BTF + * specification does not discuss alignment and its trailing string + * table is not currently padded to any particular alignment. */ + const size_t btf_alignment = 8; /* * First we look if there was already a .BTF section present and @@ -847,8 +853,8 @@ static int btf_elf__write(const char *filename, struct btf *btf) elf_flagdata(btf_data, ELF_C_SET, ELF_F_DIRTY); /* Update .BTF section in the SHT */ - size_t new_btf_offset = high_water_mark; - size_t new_btf_size = raw_btf_size; + size_t new_btf_offset = roundup(high_water_mark, btf_alignment); + size_t new_btf_size = roundup(raw_btf_size, btf_alignment); GElf_Shdr btf_shdr_mem; GElf_Shdr *btf_shdr = gelf_getshdr(btf_scn, &btf_shdr_mem); if (!btf_shdr) { @@ -856,6 +862,7 @@ static int btf_elf__write(const char *filename, struct btf *btf) __func__, elf_errmsg(elf_errno())); goto out; } + btf_shdr->sh_addralign = btf_alignment; btf_shdr->sh_entsize = 0; btf_shdr->sh_flags = SHF_ALLOC; if (dot_btf_offset) @@ -926,6 +933,7 @@ static int btf_elf__write(const char *filename, struct btf *btf) pht[phnum].p_memsz = pht[phnum].p_filesz = btf_shdr->sh_size; pht[phnum].p_vaddr = pht[phnum].p_paddr = 0; pht[phnum].p_flags = PF_R; + pht[phnum].p_align = btf_alignment; void *phdr = gelf_newphdr(elf, phnum+1); if (!phdr) { fprintf(stderr, "%s: gelf_newphdr failed: %s\n",
This is to avoid misaligned access to BTF type structs when memory-mapping ELF sections. Signed-off-by: Giuliano Procida <gprocida@google.com> --- libbtf.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)