On Fri, Oct 4, 2019 at 7:05 AM John Fastabend <[email protected]> wrote:
>
> Andrii Nakryiko wrote:
> > Kernel version enforcement for kprobes/kretprobes was removed from
> > 5.0 kernel in 6c4fc209fcf9 ("bpf: remove useless version check for prog
> > load").
> > Since then, BPF programs were specifying SEC("version") just to please
> > libbpf. We should stop enforcing this in libbpf, if even kernel doesn't
> > care. Furthermore, libbpf now will pre-populate current kernel version
> > of the host system, in case we are still running on old kernel.
> >
> > This patch also removes __bpf_object__open_xattr from libbpf.h, as
> > nothing in libbpf is relying on having it in that header. That function
> > was never exported as LIBBPF_API and even name suggests its internal
> > version. So this should be safe to remove, as it doesn't break ABI.
> >
> > Signed-off-by: Andrii Nakryiko <[email protected]>
> > ---
> > tools/lib/bpf/libbpf.c | 79 ++++++-------------
> > tools/lib/bpf/libbpf.h | 2 -
> > .../selftests/bpf/progs/test_attach_probe.c | 1 -
> > .../bpf/progs/test_get_stack_rawtp.c | 1 -
> > .../selftests/bpf/progs/test_perf_buffer.c | 1 -
> > .../selftests/bpf/progs/test_stacktrace_map.c | 1 -
> > 6 files changed, 22 insertions(+), 63 deletions(-)
>
> [...]
>
> > static struct bpf_object *
> > -__bpf_object__open(const char *path, void *obj_buf, size_t obj_buf_sz,
> > - bool needs_kver, int flags)
> > +__bpf_object__open(const char *path, const void *obj_buf, size_t
> > obj_buf_sz,
> > + int flags)
> > {
> > struct bpf_object *obj;
> > int err;
> > @@ -3617,7 +3585,6 @@ __bpf_object__open(const char *path, void *obj_buf,
> > size_t obj_buf_sz,
> > CHECK_ERR(bpf_object__probe_caps(obj), err, out);
> > CHECK_ERR(bpf_object__elf_collect(obj, flags), err, out);
>
> If we are not going to validate the section should we also skip collect'ing
> it?
Well, if user supplied version, we will parse and use it to override
out prepopulated one, so in that sense we do have validation.
But I think it's fine just to drop it altogether. Will do in v3.
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index e0276520171b..22a458cd602c 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1567,12 +1567,6 @@ static int bpf_object__elf_collect(struct bpf_object
> *obj, int flags)
> data->d_size);
> if (err)
> return err;
> - } else if (strcmp(name, "version") == 0) {
> - err = bpf_object__init_kversion(obj,
> - data->d_buf,
> - data->d_size);
> - if (err)
> - return err;
> } else if (strcmp(name, "maps") == 0) {
> obj->efile.maps_shndx = idx;
> } else if (strcmp(name, MAPS_ELF_SEC) == 0) {
>
>
> > CHECK_ERR(bpf_object__collect_reloc(obj), err, out);
> > - CHECK_ERR(bpf_object__validate(obj, needs_kver), err, out);
> >
> > bpf_object__elf_finish(obj);
> > return obj;
> > @@ -3626,8 +3593,8 @@ __bpf_object__open(const char *path, void *obj_buf,
> > size_t obj_buf_sz,
> > return ERR_PTR(err);
> > }
