| Message ID | 609abfededc3664da891514fcd687990547b8be4.1726806756.git.tony.ambardar@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Improve .BTF_ids patching and alignment | expand |
On Fri, 2024-09-20 at 00:49 -0700, Tony Ambardar wrote: > Initially, the .BTF_ids section was created zero-filled and then patched > with BTF IDs by resolve_btfids on the build host. Patching was done in > native endianness and thus failed to work for cross-endian compile targets. > This was fixed in [1] by using libelf-based translation to output patched > data in target byte order. > > The addition of 8-byte BTF sets in [2] lead to .BTF_ids creation with both > target-endian values and zero-filled data to be later patched. This again > broke cross-endian compilation as the already-correct target-endian values > were translated on output by libelf [1]. The problem was worked around [3] > by manually converting BTF SET8 values to native endianness, so that final > libelf output translation yields data in target byte order. > > Simplify and make the code more robust against future changes like [2] by > employing libelf-based endian translation on both input and output, which > is typical of libelf usage. > > [1]: 61e8aeda9398 ("bpf: Fix libelf endian handling in resolv_btfids") > [2]: ef2c6f370a63 ("tools/resolve_btfids: Add support for 8-byte BTF sets") > [3]: 903fad439466 ("tools/resolve_btfids: Fix cross-compilation to non-host endianness") > > CC: Viktor Malik <vmalik@redhat.com> > Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com> > --- Acked-by: Eduard Zingerman <eddyz87@gmail.com> [...]
On Fri, Sep 20, 2024 at 12:49:11AM -0700, Tony Ambardar wrote: SNIP > +static int btfids_endian_fix(struct object *obj) > +{ > + Elf_Data *btfids = obj->efile.idlist; > + Elf *elf = obj->efile.elf; > + int file_byteorder; > + > + /* This should always succeed due to prior ELF checks */ > + file_byteorder = elf_getident(elf, NULL)[EI_DATA]; > + > + /* Set type to ensure endian translation occurs, and manually invoke > + * translation on input since .BTF_ids section as created disables it. > + */ > + btfids->d_type = ELF_T_WORD; > + if (gelf_xlatetom(elf, btfids, btfids, file_byteorder) == NULL) { > + pr_err("FAILED xlatetom .BTF_ids data: %s\n", elf_errmsg(-1)); > + return -1; > + } > + return 0; > +} > + > static int elf_collect(struct object *obj) > { > Elf_Scn *scn = NULL; > size_t shdrstrndx; > - GElf_Ehdr ehdr; > int idx = 0; > Elf *elf; > int fd; > @@ -361,13 +371,6 @@ static int elf_collect(struct object *obj) > return -1; > } > > - if (gelf_getehdr(obj->efile.elf, &ehdr) == NULL) { > - pr_err("FAILED cannot get ELF header: %s\n", > - elf_errmsg(-1)); > - return -1; > - } > - obj->efile.encoding = ehdr.e_ident[EI_DATA]; > - > /* > * Scan all the elf sections and look for save data > * from .BTF_ids section and symbols. > @@ -409,6 +412,8 @@ static int elf_collect(struct object *obj) > obj->efile.idlist = data; > obj->efile.idlist_shndx = idx; > obj->efile.idlist_addr = sh.sh_addr; > + if (btfids_endian_fix(obj)) > + return -1; nit, it'd be bit more clear to me without using the btfids_endian_fix function and just unwind it in here.. but anyway lgtm Acked-by: Jiri Olsa <jolsa@kernel.org> jirka > } else if (!strcmp(name, BTF_BASE_ELF_SEC)) { > /* If a .BTF.base section is found, do not resolve > * BTF ids relative to vmlinux; resolve relative > @@ -706,24 +711,6 @@ static int sets_patch(struct object *obj) > */ > BUILD_BUG_ON((u32 *)set8->pairs != &set8->pairs[0].id); > qsort(set8->pairs, set8->cnt, sizeof(set8->pairs[0]), cmp_id); > - > - /* > - * When ELF endianness does not match endianness of the > - * host, libelf will do the translation when updating > - * the ELF. This, however, corrupts SET8 flags which are > - * already in the target endianness. So, let's bswap > - * them to the host endianness and libelf will then > - * correctly translate everything. > - */ > - if (obj->efile.encoding != ELFDATANATIVE) { > - int i; > - > - set8->flags = bswap_32(set8->flags); > - for (i = 0; i < set8->cnt; i++) { > - set8->pairs[i].flags = > - bswap_32(set8->pairs[i].flags); > - } > - } > } > > pr_debug("sorting addr %5lu: cnt %6d [%s]\n", > @@ -748,9 +735,6 @@ static int symbols_patch(struct object *obj) > if (sets_patch(obj)) > return -1; > > - /* Set type to ensure endian translation occurs. */ > - obj->efile.idlist->d_type = ELF_T_WORD; > - > elf_flagdata(obj->efile.idlist, ELF_C_SET, ELF_F_DIRTY); > > err = elf_update(obj->efile.elf, ELF_C_WRITE); > -- > 2.34.1 >
On 9/20/24 09:49, Tony Ambardar wrote: > Initially, the .BTF_ids section was created zero-filled and then patched > with BTF IDs by resolve_btfids on the build host. Patching was done in > native endianness and thus failed to work for cross-endian compile targets. > This was fixed in [1] by using libelf-based translation to output patched > data in target byte order. > > The addition of 8-byte BTF sets in [2] lead to .BTF_ids creation with both > target-endian values and zero-filled data to be later patched. This again > broke cross-endian compilation as the already-correct target-endian values > were translated on output by libelf [1]. The problem was worked around [3] > by manually converting BTF SET8 values to native endianness, so that final > libelf output translation yields data in target byte order. > > Simplify and make the code more robust against future changes like [2] by > employing libelf-based endian translation on both input and output, which > is typical of libelf usage. > > [1]: 61e8aeda9398 ("bpf: Fix libelf endian handling in resolv_btfids") > [2]: ef2c6f370a63 ("tools/resolve_btfids: Add support for 8-byte BTF sets") > [3]: 903fad439466 ("tools/resolve_btfids: Fix cross-compilation to non-host endianness") > > CC: Viktor Malik <vmalik@redhat.com> > Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com> > --- Acked-by: Viktor Malik <vmalik@redhat.com> > tools/bpf/resolve_btfids/main.c | 60 ++++++++++++--------------------- > 1 file changed, 22 insertions(+), 38 deletions(-) > > diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c > index d54aaa0619df..9f1ab23ed014 100644 > --- a/tools/bpf/resolve_btfids/main.c > +++ b/tools/bpf/resolve_btfids/main.c > @@ -90,14 +90,6 @@ > > #define ADDR_CNT 100 > > -#if __BYTE_ORDER == __LITTLE_ENDIAN > -# define ELFDATANATIVE ELFDATA2LSB > -#elif __BYTE_ORDER == __BIG_ENDIAN > -# define ELFDATANATIVE ELFDATA2MSB > -#else > -# error "Unknown machine endianness!" > -#endif > - > struct btf_id { > struct rb_node rb_node; > char *name; > @@ -125,7 +117,6 @@ struct object { > int idlist_shndx; > size_t strtabidx; > unsigned long idlist_addr; > - int encoding; > } efile; > > struct rb_root sets; > @@ -325,11 +316,30 @@ static int compressed_section_fix(Elf *elf, Elf_Scn *scn, GElf_Shdr *sh) > return 0; > } > > +static int btfids_endian_fix(struct object *obj) > +{ > + Elf_Data *btfids = obj->efile.idlist; > + Elf *elf = obj->efile.elf; > + int file_byteorder; > + > + /* This should always succeed due to prior ELF checks */ > + file_byteorder = elf_getident(elf, NULL)[EI_DATA]; > + > + /* Set type to ensure endian translation occurs, and manually invoke > + * translation on input since .BTF_ids section as created disables it. > + */ > + btfids->d_type = ELF_T_WORD; > + if (gelf_xlatetom(elf, btfids, btfids, file_byteorder) == NULL) { > + pr_err("FAILED xlatetom .BTF_ids data: %s\n", elf_errmsg(-1)); > + return -1; > + } > + return 0; > +} > + > static int elf_collect(struct object *obj) > { > Elf_Scn *scn = NULL; > size_t shdrstrndx; > - GElf_Ehdr ehdr; > int idx = 0; > Elf *elf; > int fd; > @@ -361,13 +371,6 @@ static int elf_collect(struct object *obj) > return -1; > } > > - if (gelf_getehdr(obj->efile.elf, &ehdr) == NULL) { > - pr_err("FAILED cannot get ELF header: %s\n", > - elf_errmsg(-1)); > - return -1; > - } > - obj->efile.encoding = ehdr.e_ident[EI_DATA]; > - > /* > * Scan all the elf sections and look for save data > * from .BTF_ids section and symbols. > @@ -409,6 +412,8 @@ static int elf_collect(struct object *obj) > obj->efile.idlist = data; > obj->efile.idlist_shndx = idx; > obj->efile.idlist_addr = sh.sh_addr; > + if (btfids_endian_fix(obj)) > + return -1; > } else if (!strcmp(name, BTF_BASE_ELF_SEC)) { > /* If a .BTF.base section is found, do not resolve > * BTF ids relative to vmlinux; resolve relative > @@ -706,24 +711,6 @@ static int sets_patch(struct object *obj) > */ > BUILD_BUG_ON((u32 *)set8->pairs != &set8->pairs[0].id); > qsort(set8->pairs, set8->cnt, sizeof(set8->pairs[0]), cmp_id); > - > - /* > - * When ELF endianness does not match endianness of the > - * host, libelf will do the translation when updating > - * the ELF. This, however, corrupts SET8 flags which are > - * already in the target endianness. So, let's bswap > - * them to the host endianness and libelf will then > - * correctly translate everything. > - */ > - if (obj->efile.encoding != ELFDATANATIVE) { > - int i; > - > - set8->flags = bswap_32(set8->flags); > - for (i = 0; i < set8->cnt; i++) { > - set8->pairs[i].flags = > - bswap_32(set8->pairs[i].flags); > - } > - } > } > > pr_debug("sorting addr %5lu: cnt %6d [%s]\n", > @@ -748,9 +735,6 @@ static int symbols_patch(struct object *obj) > if (sets_patch(obj)) > return -1; > > - /* Set type to ensure endian translation occurs. */ > - obj->efile.idlist->d_type = ELF_T_WORD; > - > elf_flagdata(obj->efile.idlist, ELF_C_SET, ELF_F_DIRTY); > > err = elf_update(obj->efile.elf, ELF_C_WRITE);
diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c index d54aaa0619df..9f1ab23ed014 100644 --- a/tools/bpf/resolve_btfids/main.c +++ b/tools/bpf/resolve_btfids/main.c @@ -90,14 +90,6 @@ #define ADDR_CNT 100 -#if __BYTE_ORDER == __LITTLE_ENDIAN -# define ELFDATANATIVE ELFDATA2LSB -#elif __BYTE_ORDER == __BIG_ENDIAN -# define ELFDATANATIVE ELFDATA2MSB -#else -# error "Unknown machine endianness!" -#endif - struct btf_id { struct rb_node rb_node; char *name; @@ -125,7 +117,6 @@ struct object { int idlist_shndx; size_t strtabidx; unsigned long idlist_addr; - int encoding; } efile; struct rb_root sets; @@ -325,11 +316,30 @@ static int compressed_section_fix(Elf *elf, Elf_Scn *scn, GElf_Shdr *sh) return 0; } +static int btfids_endian_fix(struct object *obj) +{ + Elf_Data *btfids = obj->efile.idlist; + Elf *elf = obj->efile.elf; + int file_byteorder; + + /* This should always succeed due to prior ELF checks */ + file_byteorder = elf_getident(elf, NULL)[EI_DATA]; + + /* Set type to ensure endian translation occurs, and manually invoke + * translation on input since .BTF_ids section as created disables it. + */ + btfids->d_type = ELF_T_WORD; + if (gelf_xlatetom(elf, btfids, btfids, file_byteorder) == NULL) { + pr_err("FAILED xlatetom .BTF_ids data: %s\n", elf_errmsg(-1)); + return -1; + } + return 0; +} + static int elf_collect(struct object *obj) { Elf_Scn *scn = NULL; size_t shdrstrndx; - GElf_Ehdr ehdr; int idx = 0; Elf *elf; int fd; @@ -361,13 +371,6 @@ static int elf_collect(struct object *obj) return -1; } - if (gelf_getehdr(obj->efile.elf, &ehdr) == NULL) { - pr_err("FAILED cannot get ELF header: %s\n", - elf_errmsg(-1)); - return -1; - } - obj->efile.encoding = ehdr.e_ident[EI_DATA]; - /* * Scan all the elf sections and look for save data * from .BTF_ids section and symbols. @@ -409,6 +412,8 @@ static int elf_collect(struct object *obj) obj->efile.idlist = data; obj->efile.idlist_shndx = idx; obj->efile.idlist_addr = sh.sh_addr; + if (btfids_endian_fix(obj)) + return -1; } else if (!strcmp(name, BTF_BASE_ELF_SEC)) { /* If a .BTF.base section is found, do not resolve * BTF ids relative to vmlinux; resolve relative @@ -706,24 +711,6 @@ static int sets_patch(struct object *obj) */ BUILD_BUG_ON((u32 *)set8->pairs != &set8->pairs[0].id); qsort(set8->pairs, set8->cnt, sizeof(set8->pairs[0]), cmp_id); - - /* - * When ELF endianness does not match endianness of the - * host, libelf will do the translation when updating - * the ELF. This, however, corrupts SET8 flags which are - * already in the target endianness. So, let's bswap - * them to the host endianness and libelf will then - * correctly translate everything. - */ - if (obj->efile.encoding != ELFDATANATIVE) { - int i; - - set8->flags = bswap_32(set8->flags); - for (i = 0; i < set8->cnt; i++) { - set8->pairs[i].flags = - bswap_32(set8->pairs[i].flags); - } - } } pr_debug("sorting addr %5lu: cnt %6d [%s]\n", @@ -748,9 +735,6 @@ static int symbols_patch(struct object *obj) if (sets_patch(obj)) return -1; - /* Set type to ensure endian translation occurs. */ - obj->efile.idlist->d_type = ELF_T_WORD; - elf_flagdata(obj->efile.idlist, ELF_C_SET, ELF_F_DIRTY); err = elf_update(obj->efile.elf, ELF_C_WRITE);
Initially, the .BTF_ids section was created zero-filled and then patched with BTF IDs by resolve_btfids on the build host. Patching was done in native endianness and thus failed to work for cross-endian compile targets. This was fixed in [1] by using libelf-based translation to output patched data in target byte order. The addition of 8-byte BTF sets in [2] lead to .BTF_ids creation with both target-endian values and zero-filled data to be later patched. This again broke cross-endian compilation as the already-correct target-endian values were translated on output by libelf [1]. The problem was worked around [3] by manually converting BTF SET8 values to native endianness, so that final libelf output translation yields data in target byte order. Simplify and make the code more robust against future changes like [2] by employing libelf-based endian translation on both input and output, which is typical of libelf usage. [1]: 61e8aeda9398 ("bpf: Fix libelf endian handling in resolv_btfids") [2]: ef2c6f370a63 ("tools/resolve_btfids: Add support for 8-byte BTF sets") [3]: 903fad439466 ("tools/resolve_btfids: Fix cross-compilation to non-host endianness") CC: Viktor Malik <vmalik@redhat.com> Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com> --- tools/bpf/resolve_btfids/main.c | 60 ++++++++++++--------------------- 1 file changed, 22 insertions(+), 38 deletions(-)