| Message ID | a12e83308e11b15501aa3b9e927bc94139418ce3.1724976539.git.tony.ambardar@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | libbpf, selftests/bpf: Support cross-endian usage | expand |
On Fri, Aug 30, 2024 at 12:30 AM Tony Ambardar <tony.ambardar@gmail.com> wrote: > > Allow bpf_object__open() to access files of either endianness, and convert > included BPF programs to native byte-order in-memory for introspection. > Loading BPF objects of non-native byte-order is still disallowed however. > > Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com> > --- > tools/lib/bpf/libbpf.c | 49 +++++++++++++++++++++++++++------ > tools/lib/bpf/libbpf_internal.h | 11 ++++++++ > 2 files changed, 52 insertions(+), 8 deletions(-) > [...] > > + /* Validate ELF object endianness... */ > + if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB && > + ehdr->e_ident[EI_DATA] != ELFDATA2MSB) { > + err = -LIBBPF_ERRNO__ENDIAN; > + pr_warn("elf: '%s' has unknown byte order\n", obj->path); > + goto errout; > + } > + /* and preserve outside lifetime of bpf_object_open() */ what does it mean "preserve outside lifetime" ? > + obj->byteorder = ehdr->e_ident[EI_DATA]; > + > + > + why so many empty lines?.. > if (elf_getshdrstrndx(elf, &obj->efile.shstrndx)) { > pr_warn("elf: failed to get section names section index for %s: %s\n", > obj->path, elf_errmsg(-1)); [...] > err = bpf_object__elf_init(obj); > - err = err ? : bpf_object__check_endianness(obj); > err = err ? : bpf_object__elf_collect(obj); > err = err ? : bpf_object__collect_externs(obj); > err = err ? : bpf_object_fixup_btf(obj); > @@ -8500,6 +8529,10 @@ static int bpf_object_load(struct bpf_object *obj, int extra_log_level, const ch > > if (obj->gen_loader) > bpf_gen__init(obj->gen_loader, extra_log_level, obj->nr_programs, obj->nr_maps); nit: add {} around if, both sides should either have or not have {} > + else if (!is_native_endianness(obj)) { > + pr_warn("object '%s' is not native endianness\n", obj->name); "object '%s': load is not supported in non-native endianness\n" > + return libbpf_err(-LIBBPF_ERRNO__ENDIAN); > + } > > err = bpf_object_prepare_token(obj); > err = err ? : bpf_object__probe_loading(obj); [...]
On Fri, 2024-08-30 at 00:29 -0700, Tony Ambardar wrote: [...] > @@ -940,6 +942,21 @@ bpf_object__add_programs(struct bpf_object *obj, Elf_Data *sec_data, > return 0; > } > > +static void bpf_object_bswap_progs(struct bpf_object *obj) > +{ > + struct bpf_program *prog = obj->programs; > + struct bpf_insn *insn; > + int p, i; > + > + for (p = 0; p < obj->nr_programs; p++, prog++) { > + insn = prog->insns; > + for (i = 0; i < prog->insns_cnt; i++, insn++) > + bpf_insn_bswap(insn); > + pr_debug("prog '%s': converted %zu insns to native byte order\n", > + prog->name, prog->insns_cnt); Nit: pr_debug already printed available programs at this point, maybe move this call outside of both loops? > + } > +} > + > static const struct btf_member * > find_member_by_offset(const struct btf_type *t, __u32 bit_offset) > { [...]
On Fri, 2024-08-30 at 14:25 -0700, Andrii Nakryiko wrote: [...] > > err = bpf_object__elf_init(obj); > > - err = err ? : bpf_object__check_endianness(obj); > > err = err ? : bpf_object__elf_collect(obj); > > err = err ? : bpf_object__collect_externs(obj); > > err = err ? : bpf_object_fixup_btf(obj); > > @@ -8500,6 +8529,10 @@ static int bpf_object_load(struct bpf_object *obj, int extra_log_level, const ch > > > > if (obj->gen_loader) > > bpf_gen__init(obj->gen_loader, extra_log_level, obj->nr_programs, obj->nr_maps); > > nit: add {} around if, both sides should either have or not have {} > > > + else if (!is_native_endianness(obj)) { > > + pr_warn("object '%s' is not native endianness\n", obj->name); > > "object '%s': load is not supported in non-native endianness\n" > > > > + return libbpf_err(-LIBBPF_ERRNO__ENDIAN); > > + } Silly question: why load is allowed to proceed for non-native endianness when obj->gen_loader is set?
On Fri, Aug 30, 2024 at 02:25:54PM -0700, Andrii Nakryiko wrote: > On Fri, Aug 30, 2024 at 12:30 AM Tony Ambardar <tony.ambardar@gmail.com> wrote: > > > > Allow bpf_object__open() to access files of either endianness, and convert > > included BPF programs to native byte-order in-memory for introspection. > > Loading BPF objects of non-native byte-order is still disallowed however. > > > > Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com> > > --- > > tools/lib/bpf/libbpf.c | 49 +++++++++++++++++++++++++++------ > > tools/lib/bpf/libbpf_internal.h | 11 ++++++++ > > 2 files changed, 52 insertions(+), 8 deletions(-) > > > > [...] > > > > > + /* Validate ELF object endianness... */ > > + if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB && > > + ehdr->e_ident[EI_DATA] != ELFDATA2MSB) { > > + err = -LIBBPF_ERRNO__ENDIAN; > > + pr_warn("elf: '%s' has unknown byte order\n", obj->path); > > + goto errout; > > + } > > + /* and preserve outside lifetime of bpf_object_open() */ > > what does it mean "preserve outside lifetime" ? bpf_object_open() freed ELF data on exit but didn't zero obj->efile.ehdr, leading to unpredictable use-after-free problems in is_native_endianness(). This is part of the fix but should be clearer e.g. "save after ELF data freed...". Will update. > > > + obj->byteorder = ehdr->e_ident[EI_DATA]; > > + > > + > > + > > why so many empty lines?.. I'm blind? Fixed, thanks. > > > if (elf_getshdrstrndx(elf, &obj->efile.shstrndx)) { > > pr_warn("elf: failed to get section names section index for %s: %s\n", > > obj->path, elf_errmsg(-1)); > > [...] > > > err = bpf_object__elf_init(obj); > > - err = err ? : bpf_object__check_endianness(obj); > > err = err ? : bpf_object__elf_collect(obj); > > err = err ? : bpf_object__collect_externs(obj); > > err = err ? : bpf_object_fixup_btf(obj); > > @@ -8500,6 +8529,10 @@ static int bpf_object_load(struct bpf_object *obj, int extra_log_level, const ch > > > > if (obj->gen_loader) > > bpf_gen__init(obj->gen_loader, extra_log_level, obj->nr_programs, obj->nr_maps); > > nit: add {} around if, both sides should either have or not have {} > OK, done. > > + else if (!is_native_endianness(obj)) { > > + pr_warn("object '%s' is not native endianness\n", obj->name); > > "object '%s': load is not supported in non-native endianness\n" Clearer, will update. > > > > + return libbpf_err(-LIBBPF_ERRNO__ENDIAN); > > + } > > > > err = bpf_object_prepare_token(obj); > > err = err ? : bpf_object__probe_loading(obj); > > [...]
On Fri, Aug 30, 2024 at 06:16:25PM -0700, Eduard Zingerman wrote: > On Fri, 2024-08-30 at 00:29 -0700, Tony Ambardar wrote: > > [...] > > > @@ -940,6 +942,21 @@ bpf_object__add_programs(struct bpf_object *obj, Elf_Data *sec_data, > > return 0; > > } > > > > +static void bpf_object_bswap_progs(struct bpf_object *obj) > > +{ > > + struct bpf_program *prog = obj->programs; > > + struct bpf_insn *insn; > > + int p, i; > > + > > + for (p = 0; p < obj->nr_programs; p++, prog++) { > > + insn = prog->insns; > > + for (i = 0; i < prog->insns_cnt; i++, insn++) > > + bpf_insn_bswap(insn); > > + pr_debug("prog '%s': converted %zu insns to native byte order\n", > > + prog->name, prog->insns_cnt); > > Nit: pr_debug already printed available programs at this point, > maybe move this call outside of both loops? > Good point. Will update to summarize # of programs converted instead. > > + } > > +} > > + > > static const struct btf_member * > > find_member_by_offset(const struct btf_type *t, __u32 bit_offset) > > { > > [...] >
On Fri, Aug 30, 2024 at 06:26:10PM -0700, Eduard Zingerman wrote: > On Fri, 2024-08-30 at 14:25 -0700, Andrii Nakryiko wrote: > > [...] > > > > err = bpf_object__elf_init(obj); > > > - err = err ? : bpf_object__check_endianness(obj); > > > err = err ? : bpf_object__elf_collect(obj); > > > err = err ? : bpf_object__collect_externs(obj); > > > err = err ? : bpf_object_fixup_btf(obj); > > > @@ -8500,6 +8529,10 @@ static int bpf_object_load(struct bpf_object *obj, int extra_log_level, const ch > > > > > > if (obj->gen_loader) > > > bpf_gen__init(obj->gen_loader, extra_log_level, obj->nr_programs, obj->nr_maps); > > > > nit: add {} around if, both sides should either have or not have {} > > > > > + else if (!is_native_endianness(obj)) { > > > + pr_warn("object '%s' is not native endianness\n", obj->name); > > > > "object '%s': load is not supported in non-native endianness\n" > > > > > > > + return libbpf_err(-LIBBPF_ERRNO__ENDIAN); > > > + } > > Silly question: > why load is allowed to proceed for non-native endianness when obj->gen_loader is set? > Not silly, had similar questions. Having obj->gen_loader set means "light skeleton" is being generated, where it tries to eliminate dependency on libbpf by skeleton code. In this mode, the code doesn't load anything but instead tracks "what would libbpf do" so it can later write a pure BPF loader program. Alexei will correct me or elaborate as needed I hope. Unconditionally blocking on non-native endianness would break light skel.
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 0226d3b50709..aa52870b1967 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -694,6 +694,8 @@ struct bpf_object { /* Information when doing ELF related work. Only valid if efile.elf is not NULL */ struct elf_state efile; + unsigned char byteorder; + struct btf *btf; struct btf_ext *btf_ext; @@ -940,6 +942,21 @@ bpf_object__add_programs(struct bpf_object *obj, Elf_Data *sec_data, return 0; } +static void bpf_object_bswap_progs(struct bpf_object *obj) +{ + struct bpf_program *prog = obj->programs; + struct bpf_insn *insn; + int p, i; + + for (p = 0; p < obj->nr_programs; p++, prog++) { + insn = prog->insns; + for (i = 0; i < prog->insns_cnt; i++, insn++) + bpf_insn_bswap(insn); + pr_debug("prog '%s': converted %zu insns to native byte order\n", + prog->name, prog->insns_cnt); + } +} + static const struct btf_member * find_member_by_offset(const struct btf_type *t, __u32 bit_offset) { @@ -1506,6 +1523,7 @@ static void bpf_object__elf_finish(struct bpf_object *obj) elf_end(obj->efile.elf); obj->efile.elf = NULL; + obj->efile.ehdr = NULL; obj->efile.symbols = NULL; obj->efile.arena_data = NULL; @@ -1571,6 +1589,18 @@ static int bpf_object__elf_init(struct bpf_object *obj) goto errout; } + /* Validate ELF object endianness... */ + if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB && + ehdr->e_ident[EI_DATA] != ELFDATA2MSB) { + err = -LIBBPF_ERRNO__ENDIAN; + pr_warn("elf: '%s' has unknown byte order\n", obj->path); + goto errout; + } + /* and preserve outside lifetime of bpf_object_open() */ + obj->byteorder = ehdr->e_ident[EI_DATA]; + + + if (elf_getshdrstrndx(elf, &obj->efile.shstrndx)) { pr_warn("elf: failed to get section names section index for %s: %s\n", obj->path, elf_errmsg(-1)); @@ -1599,19 +1629,15 @@ static int bpf_object__elf_init(struct bpf_object *obj) return err; } -static int bpf_object__check_endianness(struct bpf_object *obj) +static bool is_native_endianness(struct bpf_object *obj) { #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ - if (obj->efile.ehdr->e_ident[EI_DATA] == ELFDATA2LSB) - return 0; + return obj->byteorder == ELFDATA2LSB; #elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ - if (obj->efile.ehdr->e_ident[EI_DATA] == ELFDATA2MSB) - return 0; + return obj->byteorder == ELFDATA2MSB; #else # error "Unrecognized __BYTE_ORDER__" #endif - pr_warn("elf: endianness mismatch in %s.\n", obj->path); - return -LIBBPF_ERRNO__ENDIAN; } static int @@ -3953,6 +3979,10 @@ static int bpf_object__elf_collect(struct bpf_object *obj) return -LIBBPF_ERRNO__FORMAT; } + /* change BPF program insns to native endianness for introspection */ + if (!is_native_endianness(obj)) + bpf_object_bswap_progs(obj); + /* sort BPF programs by section name and in-section instruction offset * for faster search */ @@ -7992,7 +8022,6 @@ static struct bpf_object *bpf_object_open(const char *path, const void *obj_buf, } err = bpf_object__elf_init(obj); - err = err ? : bpf_object__check_endianness(obj); err = err ? : bpf_object__elf_collect(obj); err = err ? : bpf_object__collect_externs(obj); err = err ? : bpf_object_fixup_btf(obj); @@ -8500,6 +8529,10 @@ static int bpf_object_load(struct bpf_object *obj, int extra_log_level, const ch if (obj->gen_loader) bpf_gen__init(obj->gen_loader, extra_log_level, obj->nr_programs, obj->nr_maps); + else if (!is_native_endianness(obj)) { + pr_warn("object '%s' is not native endianness\n", obj->name); + return libbpf_err(-LIBBPF_ERRNO__ENDIAN); + } err = bpf_object_prepare_token(obj); err = err ? : bpf_object__probe_loading(obj); diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h index 81d375015c2b..f32e3e8378a5 100644 --- a/tools/lib/bpf/libbpf_internal.h +++ b/tools/lib/bpf/libbpf_internal.h @@ -11,6 +11,7 @@ #include <stdlib.h> #include <limits.h> +#include <byteswap.h> #include <errno.h> #include <linux/err.h> #include <fcntl.h> @@ -621,6 +622,16 @@ static inline bool is_ldimm64_insn(struct bpf_insn *insn) return insn->code == (BPF_LD | BPF_IMM | BPF_DW); } +static inline void bpf_insn_bswap(struct bpf_insn *insn) +{ + __u8 tmp_reg = insn->dst_reg; + + insn->dst_reg = insn->src_reg; + insn->src_reg = tmp_reg; + insn->off = bswap_16(insn->off); + insn->imm = bswap_32(insn->imm); +} + /* Unconditionally dup FD, ensuring it doesn't use [0, 2] range. * Original FD is not closed or altered in any other way. * Preserves original FD value, if it's invalid (negative).
Allow bpf_object__open() to access files of either endianness, and convert included BPF programs to native byte-order in-memory for introspection. Loading BPF objects of non-native byte-order is still disallowed however. Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com> --- tools/lib/bpf/libbpf.c | 49 +++++++++++++++++++++++++++------ tools/lib/bpf/libbpf_internal.h | 11 ++++++++ 2 files changed, 52 insertions(+), 8 deletions(-)