| Message ID | 20241122214431.292196-2-eddyz87@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | btf_encoder: handle .BTF_ids section endianness | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Fri, Nov 22, 2024 at 01:44:31PM -0800, Eduard Zingerman wrote: > btf_encoder__tag_kfuncs() reads .BTF_ids section to identify a set of > kfuncs present in the ELF file being processed. > This section consists of: > - arrays of uint32_t elements; > - arrays of records with the following structure: > struct btf_id_and_flag { > uint32_t id; > uint32_t flags; > }; > > When endianness of a binary operated by pahole differs from the host > system's endianness, these fields require byte-swapping before use. > Currently, this byte-swapping does not occur, resulting in kfuncs not > being marked with declaration tags. > > This commit resolves the issue by introducing an endianness conversion > step for the .BTF_ids section data before the main processing stage. > Since the ELF file is opened in O_RDONLY mode, gelf_xlatetom() > cannot be used for endianness conversion. > Instead, a new type is introduced: > > struct local_elf_data { > void *d_buf; > size_t d_size; > int64_t d_off; > bool owns_buf; > }; > > This structure is populated from the Elf_Data object representing > the .BTF_ids section. When byte-swapping is required, a local copy > of d_buf is created. > > Cc: Alan Maguire <alan.maguire@oracle.com> > Cc: Daniel Xu <dxu@dxuuu.xyz> > Cc: Jiri Olsa <olsajiri@gmail.com> > Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com> > Cc: Vadim Fedorenko <vadfed@meta.com> > Fixes: 72e88f29c6f7 ("pahole: Inject kfunc decl tags into BTF") > Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev> > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> Acked-by: Jiri Olsa <jolsa@kernel.org> thanks, jirka > --- > btf_encoder.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 59 insertions(+), 6 deletions(-) > > diff --git a/btf_encoder.c b/btf_encoder.c > index e1adddf..06d4a61 100644 > --- a/btf_encoder.c > +++ b/btf_encoder.c > @@ -33,6 +33,7 @@ > #include <stdint.h> > #include <search.h> /* for tsearch(), tfind() and tdestroy() */ > #include <pthread.h> > +#include <byteswap.h> > > #define BTF_IDS_SECTION ".BTF_ids" > #define BTF_ID_FUNC_PFX "__BTF_ID__func__" > @@ -145,6 +146,14 @@ struct btf_kfunc_set_range { > uint64_t end; > }; > > +/* Like Elf_Data, but when there is a need to change the data read from ELF */ > +struct local_elf_data { > + void *d_buf; > + size_t d_size; > + int64_t d_off; > + bool owns_buf; > +}; > + > static LIST_HEAD(encoders); > static pthread_mutex_t encoders__lock = PTHREAD_MUTEX_INITIALIZER; > > @@ -1681,7 +1690,8 @@ out: > } > > /* Returns if `sym` points to a kfunc set */ > -static int is_sym_kfunc_set(GElf_Sym *sym, const char *name, Elf_Data *idlist, size_t idlist_addr) > +static int is_sym_kfunc_set(GElf_Sym *sym, const char *name, struct local_elf_data *idlist, > + size_t idlist_addr) > { > void *ptr = idlist->d_buf; > struct btf_id_set8 *set; > @@ -1847,13 +1857,52 @@ static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer * > return 0; > } > > +/* If byte order of 'elf' differs from current byte order, convert the data->d_buf. > + * ELF file is opened in a readonly mode, so data->d_buf cannot be modified in place. > + * Instead, allocate a new buffer if modification is necessary. > + */ > +static int convert_idlist_endianness(Elf *elf, Elf_Data *src, struct local_elf_data *dst) > +{ > + int byteorder, i; > + char *elf_ident; > + uint32_t *tmp; > + > + dst->d_size = src->d_size; > + dst->d_off = src->d_off; > + elf_ident = elf_getident(elf, NULL); > + if (elf_ident == NULL) { > + fprintf(stderr, "Cannot get ELF identification from header\n"); > + return -EINVAL; > + } > + byteorder = elf_ident[EI_DATA]; > + if ((BYTE_ORDER == LITTLE_ENDIAN && byteorder == ELFDATA2LSB) > + || (BYTE_ORDER == BIG_ENDIAN && byteorder == ELFDATA2MSB)) { > + dst->d_buf = src->d_buf; > + dst->owns_buf = false; > + return 0; > + } > + tmp = malloc(src->d_size); > + if (tmp == NULL) { > + fprintf(stderr, "Cannot allocate %lu bytes of memory\n", src->d_size); > + return -ENOMEM; > + } > + memcpy(tmp, src->d_buf, src->d_size); > + dst->d_buf = tmp; > + dst->owns_buf = true; > + > + /* .BTF_ids sections consist of u32 objects */ > + for (i = 0; i < dst->d_size / sizeof(uint32_t); i++) > + tmp[i] = bswap_32(tmp[i]); > + return 0; > +} > + > static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) > { > const char *filename = encoder->source_filename; > struct gobuffer btf_kfunc_ranges = {}; > + struct local_elf_data idlist = {}; > struct gobuffer btf_funcs = {}; > Elf_Data *symbols = NULL; > - Elf_Data *idlist = NULL; > Elf_Scn *symscn = NULL; > int symbols_shndx = -1; > size_t idlist_addr = 0; > @@ -1918,7 +1967,9 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) > } else if (!strcmp(secname, BTF_IDS_SECTION)) { > idlist_shndx = i; > idlist_addr = shdr.sh_addr; > - idlist = data; > + err = convert_idlist_endianness(elf, data, &idlist); > + if (err < 0) > + goto out; > } > } > > @@ -1960,7 +2011,7 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) > continue; > > name = elf_strptr(elf, strtabidx, sym.st_name); > - if (!is_sym_kfunc_set(&sym, name, idlist, idlist_addr)) > + if (!is_sym_kfunc_set(&sym, name, &idlist, idlist_addr)) > continue; > > range.start = sym.st_value; > @@ -2003,13 +2054,13 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) > if (ranges[j].start <= addr && addr < ranges[j].end) { > found = true; > off = addr - idlist_addr; > - if (off < 0 || off + sizeof(*pair) > idlist->d_size) { > + if (off < 0 || off + sizeof(*pair) > idlist.d_size) { > fprintf(stderr, "%s: kfunc '%s' offset outside section '%s'\n", > __func__, func, BTF_IDS_SECTION); > free(func); > goto out; > } > - pair = idlist->d_buf + off; > + pair = idlist.d_buf + off; > break; > } > } > @@ -2031,6 +2082,8 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) > out: > __gobuffer__delete(&btf_funcs); > __gobuffer__delete(&btf_kfunc_ranges); > + if (idlist.owns_buf) > + free(idlist.d_buf); > if (elf) > elf_end(elf); > if (fd != -1) > -- > 2.47.0 >
On Sat, Nov 23, 2024 at 02:24:56PM +0100, Jiri Olsa wrote: > On Fri, Nov 22, 2024 at 01:44:31PM -0800, Eduard Zingerman wrote: > > btf_encoder__tag_kfuncs() reads .BTF_ids section to identify a set of > > kfuncs present in the ELF file being processed. > > This section consists of: > > - arrays of uint32_t elements; > > - arrays of records with the following structure: > > struct btf_id_and_flag { > > uint32_t id; > > uint32_t flags; > > }; > > > > When endianness of a binary operated by pahole differs from the host > > system's endianness, these fields require byte-swapping before use. > > Currently, this byte-swapping does not occur, resulting in kfuncs not > > being marked with declaration tags. > > > > This commit resolves the issue by introducing an endianness conversion > > step for the .BTF_ids section data before the main processing stage. > > Since the ELF file is opened in O_RDONLY mode, gelf_xlatetom() > > cannot be used for endianness conversion. > > Instead, a new type is introduced: > > > > struct local_elf_data { > > void *d_buf; > > size_t d_size; > > int64_t d_off; > > bool owns_buf; > > }; > > > > This structure is populated from the Elf_Data object representing > > the .BTF_ids section. When byte-swapping is required, a local copy > > of d_buf is created. > > > > Cc: Alan Maguire <alan.maguire@oracle.com> > > Cc: Daniel Xu <dxu@dxuuu.xyz> > > Cc: Jiri Olsa <olsajiri@gmail.com> > > Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com> > > Cc: Vadim Fedorenko <vadfed@meta.com> > > Fixes: 72e88f29c6f7 ("pahole: Inject kfunc decl tags into BTF") > > Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev> > > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> > > Acked-by: Jiri Olsa <jolsa@kernel.org> Looks ok to me as well, only byte swaps when needed, so affects only cross BTF encoding. Alan, have you looked at this as well? I think I saw instructions in one of the messages in this thread to get hold of a vmlinux for s390 and test it. Right? One extra question: this solves the BTF encoder case, the loader already supported loading BTF from a different endianness, right? Lemme check. cus__load_btf() cu->little_endian = btf__endianness(btf) == BTF_LITTLE_ENDIAN; enum btf_endianness btf__endianness(const struct btf *btf) { if (is_host_big_endian()) return btf->swapped_endian ? BTF_LITTLE_ENDIAN : BTF_BIG_ENDIAN; else return btf->swapped_endian ? BTF_BIG_ENDIAN : BTF_LITTLE_ENDIAN; } So we have parts of BTF byte swapping happening in libbpf and with this patch, parts of it done in pahole, have you tought about doing this in libbpf instead? - Arnaldo > > --- > > btf_encoder.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++----- > > 1 file changed, 59 insertions(+), 6 deletions(-) > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > index e1adddf..06d4a61 100644 > > --- a/btf_encoder.c > > +++ b/btf_encoder.c > > @@ -33,6 +33,7 @@ > > #include <stdint.h> > > #include <search.h> /* for tsearch(), tfind() and tdestroy() */ > > #include <pthread.h> > > +#include <byteswap.h> > > > > #define BTF_IDS_SECTION ".BTF_ids" > > #define BTF_ID_FUNC_PFX "__BTF_ID__func__" > > @@ -145,6 +146,14 @@ struct btf_kfunc_set_range { > > uint64_t end; > > }; > > > > +/* Like Elf_Data, but when there is a need to change the data read from ELF */ > > +struct local_elf_data { > > + void *d_buf; > > + size_t d_size; > > + int64_t d_off; > > + bool owns_buf; > > +}; > > + > > static LIST_HEAD(encoders); > > static pthread_mutex_t encoders__lock = PTHREAD_MUTEX_INITIALIZER; > > > > @@ -1681,7 +1690,8 @@ out: > > } > > > > /* Returns if `sym` points to a kfunc set */ > > -static int is_sym_kfunc_set(GElf_Sym *sym, const char *name, Elf_Data *idlist, size_t idlist_addr) > > +static int is_sym_kfunc_set(GElf_Sym *sym, const char *name, struct local_elf_data *idlist, > > + size_t idlist_addr) > > { > > void *ptr = idlist->d_buf; > > struct btf_id_set8 *set; > > @@ -1847,13 +1857,52 @@ static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer * > > return 0; > > } > > > > +/* If byte order of 'elf' differs from current byte order, convert the data->d_buf. > > + * ELF file is opened in a readonly mode, so data->d_buf cannot be modified in place. > > + * Instead, allocate a new buffer if modification is necessary. > > + */ > > +static int convert_idlist_endianness(Elf *elf, Elf_Data *src, struct local_elf_data *dst) > > +{ > > + int byteorder, i; > > + char *elf_ident; > > + uint32_t *tmp; > > + > > + dst->d_size = src->d_size; > > + dst->d_off = src->d_off; > > + elf_ident = elf_getident(elf, NULL); > > + if (elf_ident == NULL) { > > + fprintf(stderr, "Cannot get ELF identification from header\n"); > > + return -EINVAL; > > + } > > + byteorder = elf_ident[EI_DATA]; > > + if ((BYTE_ORDER == LITTLE_ENDIAN && byteorder == ELFDATA2LSB) > > + || (BYTE_ORDER == BIG_ENDIAN && byteorder == ELFDATA2MSB)) { > > + dst->d_buf = src->d_buf; > > + dst->owns_buf = false; > > + return 0; > > + } > > + tmp = malloc(src->d_size); > > + if (tmp == NULL) { > > + fprintf(stderr, "Cannot allocate %lu bytes of memory\n", src->d_size); > > + return -ENOMEM; > > + } > > + memcpy(tmp, src->d_buf, src->d_size); > > + dst->d_buf = tmp; > > + dst->owns_buf = true; > > + > > + /* .BTF_ids sections consist of u32 objects */ > > + for (i = 0; i < dst->d_size / sizeof(uint32_t); i++) > > + tmp[i] = bswap_32(tmp[i]); > > + return 0; > > +} > > + > > static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) > > { > > const char *filename = encoder->source_filename; > > struct gobuffer btf_kfunc_ranges = {}; > > + struct local_elf_data idlist = {}; > > struct gobuffer btf_funcs = {}; > > Elf_Data *symbols = NULL; > > - Elf_Data *idlist = NULL; > > Elf_Scn *symscn = NULL; > > int symbols_shndx = -1; > > size_t idlist_addr = 0; > > @@ -1918,7 +1967,9 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) > > } else if (!strcmp(secname, BTF_IDS_SECTION)) { > > idlist_shndx = i; > > idlist_addr = shdr.sh_addr; > > - idlist = data; > > + err = convert_idlist_endianness(elf, data, &idlist); > > + if (err < 0) > > + goto out; > > } > > } > > > > @@ -1960,7 +2011,7 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) > > continue; > > > > name = elf_strptr(elf, strtabidx, sym.st_name); > > - if (!is_sym_kfunc_set(&sym, name, idlist, idlist_addr)) > > + if (!is_sym_kfunc_set(&sym, name, &idlist, idlist_addr)) > > continue; > > > > range.start = sym.st_value; > > @@ -2003,13 +2054,13 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) > > if (ranges[j].start <= addr && addr < ranges[j].end) { > > found = true; > > off = addr - idlist_addr; > > - if (off < 0 || off + sizeof(*pair) > idlist->d_size) { > > + if (off < 0 || off + sizeof(*pair) > idlist.d_size) { > > fprintf(stderr, "%s: kfunc '%s' offset outside section '%s'\n", > > __func__, func, BTF_IDS_SECTION); > > free(func); > > goto out; > > } > > - pair = idlist->d_buf + off; > > + pair = idlist.d_buf + off; > > break; > > } > > } > > @@ -2031,6 +2082,8 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) > > out: > > __gobuffer__delete(&btf_funcs); > > __gobuffer__delete(&btf_kfunc_ranges); > > + if (idlist.owns_buf) > > + free(idlist.d_buf); > > if (elf) > > elf_end(elf); > > if (fd != -1) > > -- > > 2.47.0 > >
diff --git a/btf_encoder.c b/btf_encoder.c index e1adddf..06d4a61 100644 --- a/btf_encoder.c +++ b/btf_encoder.c @@ -33,6 +33,7 @@ #include <stdint.h> #include <search.h> /* for tsearch(), tfind() and tdestroy() */ #include <pthread.h> +#include <byteswap.h> #define BTF_IDS_SECTION ".BTF_ids" #define BTF_ID_FUNC_PFX "__BTF_ID__func__" @@ -145,6 +146,14 @@ struct btf_kfunc_set_range { uint64_t end; }; +/* Like Elf_Data, but when there is a need to change the data read from ELF */ +struct local_elf_data { + void *d_buf; + size_t d_size; + int64_t d_off; + bool owns_buf; +}; + static LIST_HEAD(encoders); static pthread_mutex_t encoders__lock = PTHREAD_MUTEX_INITIALIZER; @@ -1681,7 +1690,8 @@ out: } /* Returns if `sym` points to a kfunc set */ -static int is_sym_kfunc_set(GElf_Sym *sym, const char *name, Elf_Data *idlist, size_t idlist_addr) +static int is_sym_kfunc_set(GElf_Sym *sym, const char *name, struct local_elf_data *idlist, + size_t idlist_addr) { void *ptr = idlist->d_buf; struct btf_id_set8 *set; @@ -1847,13 +1857,52 @@ static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer * return 0; } +/* If byte order of 'elf' differs from current byte order, convert the data->d_buf. + * ELF file is opened in a readonly mode, so data->d_buf cannot be modified in place. + * Instead, allocate a new buffer if modification is necessary. + */ +static int convert_idlist_endianness(Elf *elf, Elf_Data *src, struct local_elf_data *dst) +{ + int byteorder, i; + char *elf_ident; + uint32_t *tmp; + + dst->d_size = src->d_size; + dst->d_off = src->d_off; + elf_ident = elf_getident(elf, NULL); + if (elf_ident == NULL) { + fprintf(stderr, "Cannot get ELF identification from header\n"); + return -EINVAL; + } + byteorder = elf_ident[EI_DATA]; + if ((BYTE_ORDER == LITTLE_ENDIAN && byteorder == ELFDATA2LSB) + || (BYTE_ORDER == BIG_ENDIAN && byteorder == ELFDATA2MSB)) { + dst->d_buf = src->d_buf; + dst->owns_buf = false; + return 0; + } + tmp = malloc(src->d_size); + if (tmp == NULL) { + fprintf(stderr, "Cannot allocate %lu bytes of memory\n", src->d_size); + return -ENOMEM; + } + memcpy(tmp, src->d_buf, src->d_size); + dst->d_buf = tmp; + dst->owns_buf = true; + + /* .BTF_ids sections consist of u32 objects */ + for (i = 0; i < dst->d_size / sizeof(uint32_t); i++) + tmp[i] = bswap_32(tmp[i]); + return 0; +} + static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) { const char *filename = encoder->source_filename; struct gobuffer btf_kfunc_ranges = {}; + struct local_elf_data idlist = {}; struct gobuffer btf_funcs = {}; Elf_Data *symbols = NULL; - Elf_Data *idlist = NULL; Elf_Scn *symscn = NULL; int symbols_shndx = -1; size_t idlist_addr = 0; @@ -1918,7 +1967,9 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) } else if (!strcmp(secname, BTF_IDS_SECTION)) { idlist_shndx = i; idlist_addr = shdr.sh_addr; - idlist = data; + err = convert_idlist_endianness(elf, data, &idlist); + if (err < 0) + goto out; } } @@ -1960,7 +2011,7 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) continue; name = elf_strptr(elf, strtabidx, sym.st_name); - if (!is_sym_kfunc_set(&sym, name, idlist, idlist_addr)) + if (!is_sym_kfunc_set(&sym, name, &idlist, idlist_addr)) continue; range.start = sym.st_value; @@ -2003,13 +2054,13 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) if (ranges[j].start <= addr && addr < ranges[j].end) { found = true; off = addr - idlist_addr; - if (off < 0 || off + sizeof(*pair) > idlist->d_size) { + if (off < 0 || off + sizeof(*pair) > idlist.d_size) { fprintf(stderr, "%s: kfunc '%s' offset outside section '%s'\n", __func__, func, BTF_IDS_SECTION); free(func); goto out; } - pair = idlist->d_buf + off; + pair = idlist.d_buf + off; break; } } @@ -2031,6 +2082,8 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) out: __gobuffer__delete(&btf_funcs); __gobuffer__delete(&btf_kfunc_ranges); + if (idlist.owns_buf) + free(idlist.d_buf); if (elf) elf_end(elf); if (fd != -1)
btf_encoder__tag_kfuncs() reads .BTF_ids section to identify a set of kfuncs present in the ELF file being processed. This section consists of: - arrays of uint32_t elements; - arrays of records with the following structure: struct btf_id_and_flag { uint32_t id; uint32_t flags; }; When endianness of a binary operated by pahole differs from the host system's endianness, these fields require byte-swapping before use. Currently, this byte-swapping does not occur, resulting in kfuncs not being marked with declaration tags. This commit resolves the issue by introducing an endianness conversion step for the .BTF_ids section data before the main processing stage. Since the ELF file is opened in O_RDONLY mode, gelf_xlatetom() cannot be used for endianness conversion. Instead, a new type is introduced: struct local_elf_data { void *d_buf; size_t d_size; int64_t d_off; bool owns_buf; }; This structure is populated from the Elf_Data object representing the .BTF_ids section. When byte-swapping is required, a local copy of d_buf is created. Cc: Alan Maguire <alan.maguire@oracle.com> Cc: Daniel Xu <dxu@dxuuu.xyz> Cc: Jiri Olsa <olsajiri@gmail.com> Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com> Cc: Vadim Fedorenko <vadfed@meta.com> Fixes: 72e88f29c6f7 ("pahole: Inject kfunc decl tags into BTF") Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> --- btf_encoder.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 6 deletions(-)