| Message ID | 20201005134100.302271-1-lrizzo@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | use valid btf in bpf_program__set_attach_target(prog, 0, ...); | expand |
On 10/5/20 6:41 AM, Luigi Rizzo wrote: > bpf_program__set_attach_target() will always fail with fd=0 (attach to a > kernel symbol) because obj->btf_vmlinux is NULL and there is no way to > set it. > > Fix this by explicitly calling libbpf_find_kernel_btf() in the function. > > Signed-off-by: Luigi Rizzo <lrizzo@google.com> > --- > tools/lib/bpf/libbpf.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index a4f55f8a460d..3a63db86666f 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -10352,10 +10352,13 @@ int bpf_program__set_attach_target(struct bpf_program *prog, > if (attach_prog_fd) > btf_id = libbpf_find_prog_btf_id(attach_func_name, > attach_prog_fd); > - else > - btf_id = __find_vmlinux_btf_id(prog->obj->btf_vmlinux, > - attach_func_name, > + else { > + struct btf *btf = libbpf_find_kernel_btf(); > + > + btf_id = __find_vmlinux_btf_id(btf, attach_func_name, > prog->expected_attach_type); > + btf__free(btf); > + } Doesn't feel this is right fix. In libbpf, we have API function libbpf_find_vmlinux_btf_id just doing the above. Also, could you point out why btf_vmlinux is NULL? Is this related to fd = 0? We probably should fix the problem there since what if other API functions like bpf_program__set_attach_target() wants btf_vmlinux. We should not duplicate this logic to other API functions. > > if (btf_id < 0) > return btf_id; >
On Mon, Oct 5, 2020 at 6:04 PM Yonghong Song <yhs@fb.com> wrote: > > > > On 10/5/20 6:41 AM, Luigi Rizzo wrote: > > bpf_program__set_attach_target() will always fail with fd=0 (attach to a > > kernel symbol) because obj->btf_vmlinux is NULL and there is no way to > > set it. > > > > Fix this by explicitly calling libbpf_find_kernel_btf() in the function. > > > > Signed-off-by: Luigi Rizzo <lrizzo@google.com> > > --- > > tools/lib/bpf/libbpf.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > > index a4f55f8a460d..3a63db86666f 100644 > > --- a/tools/lib/bpf/libbpf.c > > +++ b/tools/lib/bpf/libbpf.c > > @@ -10352,10 +10352,13 @@ int bpf_program__set_attach_target(struct bpf_program *prog, > > if (attach_prog_fd) > > btf_id = libbpf_find_prog_btf_id(attach_func_name, > > attach_prog_fd); > > - else > > - btf_id = __find_vmlinux_btf_id(prog->obj->btf_vmlinux, > > - attach_func_name, > > + else { > > + struct btf *btf = libbpf_find_kernel_btf(); > > + > > + btf_id = __find_vmlinux_btf_id(btf, attach_func_name, > > prog->expected_attach_type); > > + btf__free(btf); > > + } > > Doesn't feel this is right fix. In libbpf, we have API function > libbpf_find_vmlinux_btf_id > just doing the above. nice, that makes the fix even simpler, will post an update: - btf_id = __find_vmlinux_btf_id(prog->obj->btf_vmlinux, + btf_id = libbpf_find_vmlinux_btf_id(attach_func_name, > > Also, could you point out why btf_vmlinux is NULL? Is this related > to fd = 0? We probably > should fix the problem there since what if other API functions > like bpf_program__set_attach_target() wants btf_vmlinux. We should > not duplicate this logic to other API functions. As I mentioned in a previous post, I don't know how btf_vmlinux is meant to be used. To me it looks more like temporary storage for use in bpf_object__load_xattr() than a property of bpf_object (in that case it should be released in bpf_object__close() ). I'd probably fix the current bug now, and wait for the original author to communicate the intent and decide on followup changes. thanks luigi
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index a4f55f8a460d..3a63db86666f 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -10352,10 +10352,13 @@ int bpf_program__set_attach_target(struct bpf_program *prog, if (attach_prog_fd) btf_id = libbpf_find_prog_btf_id(attach_func_name, attach_prog_fd); - else - btf_id = __find_vmlinux_btf_id(prog->obj->btf_vmlinux, - attach_func_name, + else { + struct btf *btf = libbpf_find_kernel_btf(); + + btf_id = __find_vmlinux_btf_id(btf, attach_func_name, prog->expected_attach_type); + btf__free(btf); + } if (btf_id < 0) return btf_id;
bpf_program__set_attach_target() will always fail with fd=0 (attach to a kernel symbol) because obj->btf_vmlinux is NULL and there is no way to set it. Fix this by explicitly calling libbpf_find_kernel_btf() in the function. Signed-off-by: Luigi Rizzo <lrizzo@google.com> --- tools/lib/bpf/libbpf.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)