lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 11 Jan 2021 13:37:14 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Yonghong Song <yhs@...com>
Cc:     Andrii Nakryiko <andrii@...nel.org>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>, Hao Luo <haoluo@...gle.com>
Subject: Re: [PATCH v2 bpf-next 6/7] libbpf: support kernel module ksym externs

On Sun, Jan 10, 2021 at 8:15 PM Yonghong Song <yhs@...com> wrote:
>
>
>
> On 1/8/21 2:09 PM, Andrii Nakryiko wrote:
> > Add support for searching for ksym externs not just in vmlinux BTF, but across
> > all module BTFs, similarly to how it's done for CO-RE relocations. Kernels
> > that expose module BTFs through sysfs are assumed to support new ldimm64
> > instruction extension with BTF FD provided in insn[1].imm field, so no extra
> > feature detection is performed.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@...nel.org>
> > ---
> >   tools/lib/bpf/libbpf.c | 47 +++++++++++++++++++++++++++---------------
> >   1 file changed, 30 insertions(+), 17 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 6ae748f6ea11..57559a71e4de 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -395,7 +395,8 @@ struct extern_desc {
> >                       unsigned long long addr;
> >
> >                       /* target btf_id of the corresponding kernel var. */
> > -                     int vmlinux_btf_id;
> > +                     int kernel_btf_obj_fd;
> > +                     int kernel_btf_id;
> >
> >                       /* local btf_id of the ksym extern's type. */
> >                       __u32 type_id;
> > @@ -6162,7 +6163,8 @@ bpf_object__relocate_data(struct bpf_object *obj, struct bpf_program *prog)
> >                       } else /* EXT_KSYM */ {
> >                               if (ext->ksym.type_id) { /* typed ksyms */
> >                                       insn[0].src_reg = BPF_PSEUDO_BTF_ID;
> > -                                     insn[0].imm = ext->ksym.vmlinux_btf_id;
> > +                                     insn[0].imm = ext->ksym.kernel_btf_id;
> > +                                     insn[1].imm = ext->ksym.kernel_btf_obj_fd;
> >                               } else { /* typeless ksyms */
> >                                       insn[0].imm = (__u32)ext->ksym.addr;
> >                                       insn[1].imm = ext->ksym.addr >> 32;
> > @@ -7319,7 +7321,8 @@ static int bpf_object__read_kallsyms_file(struct bpf_object *obj)
> >   static int bpf_object__resolve_ksyms_btf_id(struct bpf_object *obj)
> >   {
> >       struct extern_desc *ext;
> > -     int i, id;
> > +     struct btf *btf;
> > +     int i, j, id, btf_fd, err;
> >
> >       for (i = 0; i < obj->nr_extern; i++) {
> >               const struct btf_type *targ_var, *targ_type;
> > @@ -7331,8 +7334,22 @@ static int bpf_object__resolve_ksyms_btf_id(struct bpf_object *obj)
> >               if (ext->type != EXT_KSYM || !ext->ksym.type_id)
> >                       continue;
> >
> > -             id = btf__find_by_name_kind(obj->btf_vmlinux, ext->name,
> > -                                         BTF_KIND_VAR);
> > +             btf = obj->btf_vmlinux;
> > +             btf_fd = 0;
> > +             id = btf__find_by_name_kind(btf, ext->name, BTF_KIND_VAR);
> > +             if (id == -ENOENT) {
> > +                     err = load_module_btfs(obj);
> > +                     if (err)
> > +                             return err;
> > +
> > +                     for (j = 0; j < obj->btf_module_cnt; j++) {
> > +                             btf = obj->btf_modules[j].btf;
> > +                             btf_fd = obj->btf_modules[j].fd;
>
> Do we have possibility btf_fd == 0 here?

Extremely unlikely. But if we are really worried about 0 fd, we should
handle that in a centralized fashion in libbpf. I.e., for any
operation that can return FD, check if that FD is 0, and if yes, dup()
it. And then make everything call that helper. So in the context of
this patch I'm just ignoring such possibility.

>
> > +                             id = btf__find_by_name_kind(btf, ext->name, BTF_KIND_VAR);
> > +                             if (id != -ENOENT)
> > +                                     break;
> > +                     }
> > +             }
> >               if (id <= 0) {
> >                       pr_warn("extern (ksym) '%s': failed to find BTF ID in vmlinux BTF.\n",
> >                               ext->name);
> > @@ -7343,24 +7360,19 @@ static int bpf_object__resolve_ksyms_btf_id(struct bpf_object *obj)
> >               local_type_id = ext->ksym.type_id;
> >
> >               /* find target type_id */
> > -             targ_var = btf__type_by_id(obj->btf_vmlinux, id);
> > -             targ_var_name = btf__name_by_offset(obj->btf_vmlinux,
> > -                                                 targ_var->name_off);
> > -             targ_type = skip_mods_and_typedefs(obj->btf_vmlinux,
> > -                                                targ_var->type,
> > -                                                &targ_type_id);
> > +             targ_var = btf__type_by_id(btf, id);
> > +             targ_var_name = btf__name_by_offset(btf, targ_var->name_off);
> > +             targ_type = skip_mods_and_typedefs(btf, targ_var->type, &targ_type_id);
> >
> >               ret = bpf_core_types_are_compat(obj->btf, local_type_id,
> > -                                             obj->btf_vmlinux, targ_type_id);
> > +                                             btf, targ_type_id);
> >               if (ret <= 0) {
> >                       const struct btf_type *local_type;
> >                       const char *targ_name, *local_name;
> >
> >                       local_type = btf__type_by_id(obj->btf, local_type_id);
> > -                     local_name = btf__name_by_offset(obj->btf,
> > -                                                      local_type->name_off);
> > -                     targ_name = btf__name_by_offset(obj->btf_vmlinux,
> > -                                                     targ_type->name_off);
> > +                     local_name = btf__name_by_offset(obj->btf, local_type->name_off);
> > +                     targ_name = btf__name_by_offset(btf, targ_type->name_off);
> >
> >                       pr_warn("extern (ksym) '%s': incompatible types, expected [%d] %s %s, but kernel has [%d] %s %s\n",
> >                               ext->name, local_type_id,
> > @@ -7370,7 +7382,8 @@ static int bpf_object__resolve_ksyms_btf_id(struct bpf_object *obj)
> >               }
> >
> >               ext->is_set = true;
> > -             ext->ksym.vmlinux_btf_id = id;
> > +             ext->ksym.kernel_btf_obj_fd = btf_fd;
> > +             ext->ksym.kernel_btf_id = id;
> >               pr_debug("extern (ksym) '%s': resolved to [%d] %s %s\n",
> >                        ext->name, id, btf_kind_str(targ_var), targ_var_name);
> >       }
> >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ