| Message ID | 64f6372c75a44d5c8d00db5c5b7ca21aa3b8bd77.1706717857.git.vmalik@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | tools/resolve_btfids: fix cross-compilation to non-host endianness | expand |
On 1/31/24 5:24 PM, Viktor Malik wrote: > The .BTF_ids section is pre-filled with zeroed BTF ID entries during the > build and afterwards patched by resolve_btfids with correct values. > Since resolve_btfids always writes in host-native endianness, it relies > on libelf to do the translation when the target ELF is cross-compiled to > a different endianness (this was introduced in commit 61e8aeda9398 > ("bpf: Fix libelf endian handling in resolv_btfids")). > > Unfortunately, the translation will corrupt the flags fields of SET8 > entries because these were written during vmlinux compilation and are in > the correct endianness already. This will lead to numerous selftests > failures such as: > > $ sudo ./test_verifier 502 502 > #502/p sleepable fentry accept FAIL > Failed to load prog 'Invalid argument'! > bpf_fentry_test1 is not sleepable > verification time 34 usec > stack depth 0 > processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 > Summary: 0 PASSED, 0 SKIPPED, 1 FAILED > > Since it's not possible to instruct libelf to translate just certain > values, let's manually bswap the flags in resolve_btfids when needed, so > that libelf then translates everything correctly. > > Fixes: ef2c6f370a63 ("tools/resolve_btfids: Add support for 8-byte BTF sets") > Signed-off-by: Viktor Malik <vmalik@redhat.com> > --- > tools/bpf/resolve_btfids/main.c | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c > index 7badf1557e5c..d01603ef6283 100644 > --- a/tools/bpf/resolve_btfids/main.c > +++ b/tools/bpf/resolve_btfids/main.c > @@ -652,13 +652,23 @@ static int sets_patch(struct object *obj) > Elf_Data *data = obj->efile.idlist; > int *ptr = data->d_buf; > struct rb_node *next; > + GElf_Ehdr ehdr; > + int need_bswap; > + > + if (gelf_getehdr(obj->efile.elf, &ehdr) == NULL) { > + pr_err("FAILED cannot get ELF header: %s\n", > + elf_errmsg(-1)); > + return -1; > + } > + need_bswap = (__BYTE_ORDER == __LITTLE_ENDIAN) != > + (ehdr.e_ident[EI_DATA] == ELFDATA2LSB); > > next = rb_first(&obj->sets); > while (next) { > unsigned long addr, idx; > struct btf_id *id; > void *base; > - int cnt, size; > + int cnt, size, i; > > id = rb_entry(next, struct btf_id, rb_node); > addr = id->addr[0]; > @@ -686,6 +696,21 @@ static int sets_patch(struct object *obj) > base = set8->pairs; > cnt = set8->cnt; > size = sizeof(set8->pairs[0]); > + > + /* > + * 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 (need_bswap) { > + for (i = 0; i < cnt; i++) { > + set8->pairs[i].flags = > + bswap_32(set8->pairs[i].flags); > + } > + } > } > Could we improve that somewhat, e.g. gathering endianness could be moved into elf_collect() and the test could also be simplified (if I'm not missing sth) ? Like the below (not even compile-tested ...) : diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c index 7badf1557e5c..7b5f592fe79c 100644 --- a/tools/bpf/resolve_btfids/main.c +++ b/tools/bpf/resolve_btfids/main.c @@ -90,6 +90,14 @@ #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; @@ -117,6 +125,7 @@ struct object { int idlist_shndx; size_t strtabidx; unsigned long idlist_addr; + int encoding; } efile; struct rb_root sets; @@ -320,6 +329,7 @@ static int elf_collect(struct object *obj) { Elf_Scn *scn = NULL; size_t shdrstrndx; + GElf_Ehdr ehdr; int idx = 0; Elf *elf; int fd; @@ -351,6 +361,13 @@ 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. @@ -649,6 +666,7 @@ static int cmp_id(const void *pa, const void *pb) static int sets_patch(struct object *obj) { + bool need_bswap = obj->efile.encoding != ELFDATANATIVE; Elf_Data *data = obj->efile.idlist; int *ptr = data->d_buf; struct rb_node *next; @@ -658,7 +676,7 @@ static int sets_patch(struct object *obj) unsigned long addr, idx; struct btf_id *id; void *base; - int cnt, size; + int cnt, size, i; id = rb_entry(next, struct btf_id, rb_node); addr = id->addr[0]; @@ -686,6 +704,21 @@ static int sets_patch(struct object *obj) base = set8->pairs; cnt = set8->cnt; size = sizeof(set8->pairs[0]); + + /* + * 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 (need_bswap) { + for (i = 0; i < cnt; i++) { + set8->pairs[i].flags = + bswap_32(set8->pairs[i].flags); + } + } } pr_debug("sorting addr %5lu: cnt %6d [%s]\n",
On 2/1/24 17:36, Daniel Borkmann wrote: > On 1/31/24 5:24 PM, Viktor Malik wrote: >> The .BTF_ids section is pre-filled with zeroed BTF ID entries during the >> build and afterwards patched by resolve_btfids with correct values. >> Since resolve_btfids always writes in host-native endianness, it relies >> on libelf to do the translation when the target ELF is cross-compiled to >> a different endianness (this was introduced in commit 61e8aeda9398 >> ("bpf: Fix libelf endian handling in resolv_btfids")). >> >> Unfortunately, the translation will corrupt the flags fields of SET8 >> entries because these were written during vmlinux compilation and are in >> the correct endianness already. This will lead to numerous selftests >> failures such as: >> >> $ sudo ./test_verifier 502 502 >> #502/p sleepable fentry accept FAIL >> Failed to load prog 'Invalid argument'! >> bpf_fentry_test1 is not sleepable >> verification time 34 usec >> stack depth 0 >> processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 >> Summary: 0 PASSED, 0 SKIPPED, 1 FAILED >> >> Since it's not possible to instruct libelf to translate just certain >> values, let's manually bswap the flags in resolve_btfids when needed, so >> that libelf then translates everything correctly. >> >> Fixes: ef2c6f370a63 ("tools/resolve_btfids: Add support for 8-byte BTF sets") >> Signed-off-by: Viktor Malik <vmalik@redhat.com> >> --- >> tools/bpf/resolve_btfids/main.c | 27 ++++++++++++++++++++++++++- >> 1 file changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c >> index 7badf1557e5c..d01603ef6283 100644 >> --- a/tools/bpf/resolve_btfids/main.c >> +++ b/tools/bpf/resolve_btfids/main.c >> @@ -652,13 +652,23 @@ static int sets_patch(struct object *obj) >> Elf_Data *data = obj->efile.idlist; >> int *ptr = data->d_buf; >> struct rb_node *next; >> + GElf_Ehdr ehdr; >> + int need_bswap; >> + >> + if (gelf_getehdr(obj->efile.elf, &ehdr) == NULL) { >> + pr_err("FAILED cannot get ELF header: %s\n", >> + elf_errmsg(-1)); >> + return -1; >> + } >> + need_bswap = (__BYTE_ORDER == __LITTLE_ENDIAN) != >> + (ehdr.e_ident[EI_DATA] == ELFDATA2LSB); >> >> next = rb_first(&obj->sets); >> while (next) { >> unsigned long addr, idx; >> struct btf_id *id; >> void *base; >> - int cnt, size; >> + int cnt, size, i; >> >> id = rb_entry(next, struct btf_id, rb_node); >> addr = id->addr[0]; >> @@ -686,6 +696,21 @@ static int sets_patch(struct object *obj) >> base = set8->pairs; >> cnt = set8->cnt; >> size = sizeof(set8->pairs[0]); >> + >> + /* >> + * 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 (need_bswap) { >> + for (i = 0; i < cnt; i++) { >> + set8->pairs[i].flags = >> + bswap_32(set8->pairs[i].flags); >> + } >> + } >> } >> > > Could we improve that somewhat, e.g. gathering endianness could be moved into > elf_collect() and the test could also be simplified (if I'm not missing sth) ? > > Like the below (not even compile-tested ...) : Thanks for the suggestion, that looks nicer than my version. I'll use the below, it should work pretty much out of the box. Viktor > > diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c > index 7badf1557e5c..7b5f592fe79c 100644 > --- a/tools/bpf/resolve_btfids/main.c > +++ b/tools/bpf/resolve_btfids/main.c > @@ -90,6 +90,14 @@ > > #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; > @@ -117,6 +125,7 @@ struct object { > int idlist_shndx; > size_t strtabidx; > unsigned long idlist_addr; > + int encoding; > } efile; > > struct rb_root sets; > @@ -320,6 +329,7 @@ static int elf_collect(struct object *obj) > { > Elf_Scn *scn = NULL; > size_t shdrstrndx; > + GElf_Ehdr ehdr; > int idx = 0; > Elf *elf; > int fd; > @@ -351,6 +361,13 @@ 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. > @@ -649,6 +666,7 @@ static int cmp_id(const void *pa, const void *pb) > > static int sets_patch(struct object *obj) > { > + bool need_bswap = obj->efile.encoding != ELFDATANATIVE; > Elf_Data *data = obj->efile.idlist; > int *ptr = data->d_buf; > struct rb_node *next; > @@ -658,7 +676,7 @@ static int sets_patch(struct object *obj) > unsigned long addr, idx; > struct btf_id *id; > void *base; > - int cnt, size; > + int cnt, size, i; > > id = rb_entry(next, struct btf_id, rb_node); > addr = id->addr[0]; > @@ -686,6 +704,21 @@ static int sets_patch(struct object *obj) > base = set8->pairs; > cnt = set8->cnt; > size = sizeof(set8->pairs[0]); > + > + /* > + * 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 (need_bswap) { > + for (i = 0; i < cnt; i++) { > + set8->pairs[i].flags = > + bswap_32(set8->pairs[i].flags); > + } > + } > } > > pr_debug("sorting addr %5lu: cnt %6d [%s]\n",
Hi Viktor, On Wed, Jan 31, 2024 at 05:24:09PM +0100, Viktor Malik wrote: > The .BTF_ids section is pre-filled with zeroed BTF ID entries during the > build and afterwards patched by resolve_btfids with correct values. > Since resolve_btfids always writes in host-native endianness, it relies > on libelf to do the translation when the target ELF is cross-compiled to > a different endianness (this was introduced in commit 61e8aeda9398 > ("bpf: Fix libelf endian handling in resolv_btfids")). > > Unfortunately, the translation will corrupt the flags fields of SET8 > entries because these were written during vmlinux compilation and are in > the correct endianness already. This will lead to numerous selftests > failures such as: > > $ sudo ./test_verifier 502 502 > #502/p sleepable fentry accept FAIL > Failed to load prog 'Invalid argument'! > bpf_fentry_test1 is not sleepable > verification time 34 usec > stack depth 0 > processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 > Summary: 0 PASSED, 0 SKIPPED, 1 FAILED > > Since it's not possible to instruct libelf to translate just certain > values, let's manually bswap the flags in resolve_btfids when needed, so > that libelf then translates everything correctly. > > Fixes: ef2c6f370a63 ("tools/resolve_btfids: Add support for 8-byte BTF sets") > Signed-off-by: Viktor Malik <vmalik@redhat.com> > --- > tools/bpf/resolve_btfids/main.c | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c > index 7badf1557e5c..d01603ef6283 100644 > --- a/tools/bpf/resolve_btfids/main.c > +++ b/tools/bpf/resolve_btfids/main.c > @@ -652,13 +652,23 @@ static int sets_patch(struct object *obj) > Elf_Data *data = obj->efile.idlist; > int *ptr = data->d_buf; > struct rb_node *next; > + GElf_Ehdr ehdr; > + int need_bswap; > + > + if (gelf_getehdr(obj->efile.elf, &ehdr) == NULL) { > + pr_err("FAILED cannot get ELF header: %s\n", > + elf_errmsg(-1)); > + return -1; > + } > + need_bswap = (__BYTE_ORDER == __LITTLE_ENDIAN) != > + (ehdr.e_ident[EI_DATA] == ELFDATA2LSB); > > next = rb_first(&obj->sets); > while (next) { > unsigned long addr, idx; > struct btf_id *id; > void *base; > - int cnt, size; > + int cnt, size, i; > > id = rb_entry(next, struct btf_id, rb_node); > addr = id->addr[0]; > @@ -686,6 +696,21 @@ static int sets_patch(struct object *obj) > base = set8->pairs; > cnt = set8->cnt; > size = sizeof(set8->pairs[0]); > + > + /* > + * 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 (need_bswap) { > + for (i = 0; i < cnt; i++) { > + set8->pairs[i].flags = > + bswap_32(set8->pairs[i].flags); > + } Do we need this for btf_id_set8:flags as well? Didn't get a chance to look too deeply yet. Thanks, Daniel
On Fri, Feb 02, 2024 at 06:38:18PM -0700, Daniel Xu wrote: > Hi Viktor, > > On Wed, Jan 31, 2024 at 05:24:09PM +0100, Viktor Malik wrote: > > The .BTF_ids section is pre-filled with zeroed BTF ID entries during the > > build and afterwards patched by resolve_btfids with correct values. > > Since resolve_btfids always writes in host-native endianness, it relies > > on libelf to do the translation when the target ELF is cross-compiled to > > a different endianness (this was introduced in commit 61e8aeda9398 > > ("bpf: Fix libelf endian handling in resolv_btfids")). > > > > Unfortunately, the translation will corrupt the flags fields of SET8 > > entries because these were written during vmlinux compilation and are in > > the correct endianness already. This will lead to numerous selftests > > failures such as: > > > > $ sudo ./test_verifier 502 502 > > #502/p sleepable fentry accept FAIL > > Failed to load prog 'Invalid argument'! > > bpf_fentry_test1 is not sleepable > > verification time 34 usec > > stack depth 0 > > processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 > > Summary: 0 PASSED, 0 SKIPPED, 1 FAILED > > > > Since it's not possible to instruct libelf to translate just certain > > values, let's manually bswap the flags in resolve_btfids when needed, so > > that libelf then translates everything correctly. > > > > Fixes: ef2c6f370a63 ("tools/resolve_btfids: Add support for 8-byte BTF sets") > > Signed-off-by: Viktor Malik <vmalik@redhat.com> > > --- > > tools/bpf/resolve_btfids/main.c | 27 ++++++++++++++++++++++++++- > > 1 file changed, 26 insertions(+), 1 deletion(-) > > > > diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c > > index 7badf1557e5c..d01603ef6283 100644 > > --- a/tools/bpf/resolve_btfids/main.c > > +++ b/tools/bpf/resolve_btfids/main.c > > @@ -652,13 +652,23 @@ static int sets_patch(struct object *obj) > > Elf_Data *data = obj->efile.idlist; > > int *ptr = data->d_buf; > > struct rb_node *next; > > + GElf_Ehdr ehdr; > > + int need_bswap; > > + > > + if (gelf_getehdr(obj->efile.elf, &ehdr) == NULL) { > > + pr_err("FAILED cannot get ELF header: %s\n", > > + elf_errmsg(-1)); > > + return -1; > > + } > > + need_bswap = (__BYTE_ORDER == __LITTLE_ENDIAN) != > > + (ehdr.e_ident[EI_DATA] == ELFDATA2LSB); > > > > next = rb_first(&obj->sets); > > while (next) { > > unsigned long addr, idx; > > struct btf_id *id; > > void *base; > > - int cnt, size; > > + int cnt, size, i; > > > > id = rb_entry(next, struct btf_id, rb_node); > > addr = id->addr[0]; > > @@ -686,6 +696,21 @@ static int sets_patch(struct object *obj) > > base = set8->pairs; > > cnt = set8->cnt; > > size = sizeof(set8->pairs[0]); > > + > > + /* > > + * 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 (need_bswap) { > > + for (i = 0; i < cnt; i++) { > > + set8->pairs[i].flags = > > + bswap_32(set8->pairs[i].flags); > > + } > > Do we need this for btf_id_set8:flags as well? Didn't get a chance to > look too deeply yet. > > Thanks, > Daniel I ran some test and tried and validated Jiri's patch in https://lore.kernel.org/bpf/Zb6Jt30bNcNhM6zR@surya/ Manu
On Fri, Feb 02, 2024 at 06:38:18PM -0700, Daniel Xu wrote: > Hi Viktor, > > On Wed, Jan 31, 2024 at 05:24:09PM +0100, Viktor Malik wrote: > > The .BTF_ids section is pre-filled with zeroed BTF ID entries during the > > build and afterwards patched by resolve_btfids with correct values. > > Since resolve_btfids always writes in host-native endianness, it relies > > on libelf to do the translation when the target ELF is cross-compiled to > > a different endianness (this was introduced in commit 61e8aeda9398 > > ("bpf: Fix libelf endian handling in resolv_btfids")). > > > > Unfortunately, the translation will corrupt the flags fields of SET8 > > entries because these were written during vmlinux compilation and are in > > the correct endianness already. This will lead to numerous selftests > > failures such as: > > > > $ sudo ./test_verifier 502 502 > > #502/p sleepable fentry accept FAIL > > Failed to load prog 'Invalid argument'! > > bpf_fentry_test1 is not sleepable > > verification time 34 usec > > stack depth 0 > > processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 > > Summary: 0 PASSED, 0 SKIPPED, 1 FAILED > > > > Since it's not possible to instruct libelf to translate just certain > > values, let's manually bswap the flags in resolve_btfids when needed, so > > that libelf then translates everything correctly. > > > > Fixes: ef2c6f370a63 ("tools/resolve_btfids: Add support for 8-byte BTF sets") > > Signed-off-by: Viktor Malik <vmalik@redhat.com> > > --- > > tools/bpf/resolve_btfids/main.c | 27 ++++++++++++++++++++++++++- > > 1 file changed, 26 insertions(+), 1 deletion(-) > > > > diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c > > index 7badf1557e5c..d01603ef6283 100644 > > --- a/tools/bpf/resolve_btfids/main.c > > +++ b/tools/bpf/resolve_btfids/main.c > > @@ -652,13 +652,23 @@ static int sets_patch(struct object *obj) > > Elf_Data *data = obj->efile.idlist; > > int *ptr = data->d_buf; > > struct rb_node *next; > > + GElf_Ehdr ehdr; > > + int need_bswap; > > + > > + if (gelf_getehdr(obj->efile.elf, &ehdr) == NULL) { > > + pr_err("FAILED cannot get ELF header: %s\n", > > + elf_errmsg(-1)); > > + return -1; > > + } > > + need_bswap = (__BYTE_ORDER == __LITTLE_ENDIAN) != > > + (ehdr.e_ident[EI_DATA] == ELFDATA2LSB); > > > > next = rb_first(&obj->sets); > > while (next) { > > unsigned long addr, idx; > > struct btf_id *id; > > void *base; > > - int cnt, size; > > + int cnt, size, i; > > > > id = rb_entry(next, struct btf_id, rb_node); > > addr = id->addr[0]; > > @@ -686,6 +696,21 @@ static int sets_patch(struct object *obj) > > base = set8->pairs; > > cnt = set8->cnt; > > size = sizeof(set8->pairs[0]); > > + > > + /* > > + * 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 (need_bswap) { > > + for (i = 0; i < cnt; i++) { > > + set8->pairs[i].flags = > > + bswap_32(set8->pairs[i].flags); > > + } > > Do we need this for btf_id_set8:flags as well? Didn't get a chance to > look too deeply yet. ah did not this, right, looks like we need that jirka
On 2/3/24 19:48, Manu Bretelle wrote: > On Fri, Feb 02, 2024 at 06:38:18PM -0700, Daniel Xu wrote: >> Hi Viktor, >> >> On Wed, Jan 31, 2024 at 05:24:09PM +0100, Viktor Malik wrote: >>> The .BTF_ids section is pre-filled with zeroed BTF ID entries during the >>> build and afterwards patched by resolve_btfids with correct values. >>> Since resolve_btfids always writes in host-native endianness, it relies >>> on libelf to do the translation when the target ELF is cross-compiled to >>> a different endianness (this was introduced in commit 61e8aeda9398 >>> ("bpf: Fix libelf endian handling in resolv_btfids")). >>> >>> Unfortunately, the translation will corrupt the flags fields of SET8 >>> entries because these were written during vmlinux compilation and are in >>> the correct endianness already. This will lead to numerous selftests >>> failures such as: >>> >>> $ sudo ./test_verifier 502 502 >>> #502/p sleepable fentry accept FAIL >>> Failed to load prog 'Invalid argument'! >>> bpf_fentry_test1 is not sleepable >>> verification time 34 usec >>> stack depth 0 >>> processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 >>> Summary: 0 PASSED, 0 SKIPPED, 1 FAILED >>> >>> Since it's not possible to instruct libelf to translate just certain >>> values, let's manually bswap the flags in resolve_btfids when needed, so >>> that libelf then translates everything correctly. >>> >>> Fixes: ef2c6f370a63 ("tools/resolve_btfids: Add support for 8-byte BTF sets") >>> Signed-off-by: Viktor Malik <vmalik@redhat.com> >>> --- >>> tools/bpf/resolve_btfids/main.c | 27 ++++++++++++++++++++++++++- >>> 1 file changed, 26 insertions(+), 1 deletion(-) >>> >>> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c >>> index 7badf1557e5c..d01603ef6283 100644 >>> --- a/tools/bpf/resolve_btfids/main.c >>> +++ b/tools/bpf/resolve_btfids/main.c >>> @@ -652,13 +652,23 @@ static int sets_patch(struct object *obj) >>> Elf_Data *data = obj->efile.idlist; >>> int *ptr = data->d_buf; >>> struct rb_node *next; >>> + GElf_Ehdr ehdr; >>> + int need_bswap; >>> + >>> + if (gelf_getehdr(obj->efile.elf, &ehdr) == NULL) { >>> + pr_err("FAILED cannot get ELF header: %s\n", >>> + elf_errmsg(-1)); >>> + return -1; >>> + } >>> + need_bswap = (__BYTE_ORDER == __LITTLE_ENDIAN) != >>> + (ehdr.e_ident[EI_DATA] == ELFDATA2LSB); >>> >>> next = rb_first(&obj->sets); >>> while (next) { >>> unsigned long addr, idx; >>> struct btf_id *id; >>> void *base; >>> - int cnt, size; >>> + int cnt, size, i; >>> >>> id = rb_entry(next, struct btf_id, rb_node); >>> addr = id->addr[0]; >>> @@ -686,6 +696,21 @@ static int sets_patch(struct object *obj) >>> base = set8->pairs; >>> cnt = set8->cnt; >>> size = sizeof(set8->pairs[0]); >>> + >>> + /* >>> + * 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 (need_bswap) { >>> + for (i = 0; i < cnt; i++) { >>> + set8->pairs[i].flags = >>> + bswap_32(set8->pairs[i].flags); >>> + } >> >> Do we need this for btf_id_set8:flags as well? Didn't get a chance to >> look too deeply yet. >> >> Thanks, >> Daniel > > I ran some test and tried and validated Jiri's patch in > https://lore.kernel.org/bpf/Zb6Jt30bNcNhM6zR@surya/ Right, I'll include that patch. Thanks for testing! Viktor > > Manu >
diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c index 7badf1557e5c..d01603ef6283 100644 --- a/tools/bpf/resolve_btfids/main.c +++ b/tools/bpf/resolve_btfids/main.c @@ -652,13 +652,23 @@ static int sets_patch(struct object *obj) Elf_Data *data = obj->efile.idlist; int *ptr = data->d_buf; struct rb_node *next; + GElf_Ehdr ehdr; + int need_bswap; + + if (gelf_getehdr(obj->efile.elf, &ehdr) == NULL) { + pr_err("FAILED cannot get ELF header: %s\n", + elf_errmsg(-1)); + return -1; + } + need_bswap = (__BYTE_ORDER == __LITTLE_ENDIAN) != + (ehdr.e_ident[EI_DATA] == ELFDATA2LSB); next = rb_first(&obj->sets); while (next) { unsigned long addr, idx; struct btf_id *id; void *base; - int cnt, size; + int cnt, size, i; id = rb_entry(next, struct btf_id, rb_node); addr = id->addr[0]; @@ -686,6 +696,21 @@ static int sets_patch(struct object *obj) base = set8->pairs; cnt = set8->cnt; size = sizeof(set8->pairs[0]); + + /* + * 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 (need_bswap) { + for (i = 0; i < cnt; i++) { + set8->pairs[i].flags = + bswap_32(set8->pairs[i].flags); + } + } } pr_debug("sorting addr %5lu: cnt %6d [%s]\n",
The .BTF_ids section is pre-filled with zeroed BTF ID entries during the build and afterwards patched by resolve_btfids with correct values. Since resolve_btfids always writes in host-native endianness, it relies on libelf to do the translation when the target ELF is cross-compiled to a different endianness (this was introduced in commit 61e8aeda9398 ("bpf: Fix libelf endian handling in resolv_btfids")). Unfortunately, the translation will corrupt the flags fields of SET8 entries because these were written during vmlinux compilation and are in the correct endianness already. This will lead to numerous selftests failures such as: $ sudo ./test_verifier 502 502 #502/p sleepable fentry accept FAIL Failed to load prog 'Invalid argument'! bpf_fentry_test1 is not sleepable verification time 34 usec stack depth 0 processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 Summary: 0 PASSED, 0 SKIPPED, 1 FAILED Since it's not possible to instruct libelf to translate just certain values, let's manually bswap the flags in resolve_btfids when needed, so that libelf then translates everything correctly. Fixes: ef2c6f370a63 ("tools/resolve_btfids: Add support for 8-byte BTF sets") Signed-off-by: Viktor Malik <vmalik@redhat.com> --- tools/bpf/resolve_btfids/main.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)