[PATCH] use valid btf in bpf_program__set_attach_target(prog, 0, ...);

[PATCH] use valid btf in bpf_program__set_attach_target(prog, 0, ...);

[Luigi Rizzo]

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);
+	}
 
 	if (btf_id < 0)
 		return btf_id;
-- 
2.28.0.806.g8561365e88-goog

Re: [PATCH] use valid btf in bpf_program__set_attach_target(prog, 0, ...);

[Yonghong Song]

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;
> 

Re: [PATCH] use valid btf in bpf_program__set_attach_target(prog, 0, ...);

[Luigi Rizzo]

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