[PATCH bpf-next] libbpf: avoid NULL deref when initializing map BTF info

[PATCH bpf-next] libbpf: avoid NULL deref when initializing map BTF info

[Andrii Nakryiko]

If BPF object doesn't have an BTF info, don't attempt to search for BTF
types describing BPF map key or value layout.

Fixes: 262cfb74ffda ("libbpf: Init btf_{key,value}_type_id on internal map open")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 7526419e59e0..2efe9431c1ba 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4197,6 +4197,9 @@ static int bpf_map_find_btf_info(struct bpf_object *obj, struct bpf_map *map)
 	__u32 key_type_id = 0, value_type_id = 0;
 	int ret;
 
+	if (!obj->btf)
+		return -ENOENT;
+
 	/* if it's BTF-defined map, we don't need to search for type IDs.
 	 * For struct_ops map, it does not need btf_key_type_id and
 	 * btf_value_type_id.
-- 
2.30.2

Re: [PATCH bpf-next] libbpf: avoid NULL deref when initializing map BTF info

[Yonghong Song]

On 3/19/22 5:19 PM, Andrii Nakryiko wrote:
> If BPF object doesn't have an BTF info, don't attempt to search for BTF
> types describing BPF map key or value layout.
> 
> Fixes: 262cfb74ffda ("libbpf: Init btf_{key,value}_type_id on internal map open")
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

So without this patch, we may have segfault or other errors, right?
It would be good to specify that in the commit message.

The code change looks good to me.

Acked-by: Yonghong Song <yhs@fb.com>

> ---
>   tools/lib/bpf/libbpf.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 7526419e59e0..2efe9431c1ba 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4197,6 +4197,9 @@ static int bpf_map_find_btf_info(struct bpf_object *obj, struct bpf_map *map)
>   	__u32 key_type_id = 0, value_type_id = 0;
>   	int ret;
>   
> +	if (!obj->btf)
> +		return -ENOENT;
> +
>   	/* if it's BTF-defined map, we don't need to search for type IDs.
>   	 * For struct_ops map, it does not need btf_key_type_id and
>   	 * btf_value_type_id.

Re: [PATCH bpf-next] libbpf: avoid NULL deref when initializing map BTF info

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Sat, 19 Mar 2022 17:19:11 -0700 you wrote:
> If BPF object doesn't have an BTF info, don't attempt to search for BTF
> types describing BPF map key or value layout.
> 
> Fixes: 262cfb74ffda ("libbpf: Init btf_{key,value}_type_id on internal map open")
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  tools/lib/bpf/libbpf.c | 3 +++
>  1 file changed, 3 insertions(+)

Here is the summary with links:
  - [bpf-next] libbpf: avoid NULL deref when initializing map BTF info
    https://git.kernel.org/bpf/bpf-next/c/a8fee96202e2

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH bpf-next] libbpf: avoid NULL deref when initializing map BTF info

[Andrii Nakryiko]

On Sat, Mar 19, 2022 at 8:37 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 3/19/22 5:19 PM, Andrii Nakryiko wrote:
> > If BPF object doesn't have an BTF info, don't attempt to search for BTF
> > types describing BPF map key or value layout.
> >
> > Fixes: 262cfb74ffda ("libbpf: Init btf_{key,value}_type_id on internal map open")
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>
> So without this patch, we may have segfault or other errors, right?
> It would be good to specify that in the commit message.
>
> The code change looks good to me.
>
> Acked-by: Yonghong Song <yhs@fb.com>
>
> > ---
> >   tools/lib/bpf/libbpf.c | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 7526419e59e0..2efe9431c1ba 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -4197,6 +4197,9 @@ static int bpf_map_find_btf_info(struct bpf_object *obj, struct bpf_map *map)
> >       __u32 key_type_id = 0, value_type_id = 0;
> >       int ret;
> >
> > +     if (!obj->btf)
> > +             return -ENOENT;
> > +

yeah, it was SIGSEGV, which I only saw in libbpf CI on old 5.5 kernel.
I'll include a bit more specifics for future fixes, sorry.

> >       /* if it's BTF-defined map, we don't need to search for type IDs.
> >        * For struct_ops map, it does not need btf_key_type_id and
> >        * btf_value_type_id.