On Fri, Oct 4, 2019 at 7:05 AM John Fastabend <john.fastab...@gmail.com> 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 <andr...@fb.com>
> > ---
> >  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);
> >  }

Reply via email to