Re: [PATCHv2 bpf] libbpf: Prevent double close of btf objects

[Jiri Olsa <olsajiri@gmail.com>]

On Wed, Apr 15, 2026 at 02:54:46PM +0200, Jiri Olsa wrote:
> On Wed, Apr 15, 2026 at 12:35:59PM +0100, Alan Maguire wrote:
> > On 15/04/2026 09:24, Jiri Olsa wrote:
> > > Sashiko found possible double close of btf object fd [1],
> > > which happens when strdup in load_module_btfs fails at which
> > > point the obj->btf_module_cnt is already incremented.
> > >
> > 
> > nit: maybe the subject should be something like
> > 
> > libbpf: Prevent double close, leak of btf objects
> > 
> > ...to reflect the leak fix too?
> 
> so the btf object leak is introduced by moving the obj->btf_module_cnt++
> below, because before that it was released in bpf_object_post_load_cleanup
> even when the strdup failed
> 
> now if strdup fails, obj->btf_module_cnt is not incremented yet and
> bpf_object_post_load_cleanup won't release it

nah you're right, there's the other possible btf leak when
libbpf_ensure_mem fails.. I'll mention that in next version

thanks,
jirka

> 
> >  
> > > The error path close btf fd and so does later cleanup code in
> > > bpf_object_post_load_cleanup function.
> > > 
> > > Incrementing obj->btf_module_cnt only if there's no failure
> > > and releasing btf object in error path.
> > > 
> > > Fixes: 91abb4a6d79d ("libbpf: Support attachment of BPF tracing programs to kernel modules")
> > > [1] https://sashiko.dev/#/patchset/20260324081846.2334094-1-jolsa%40kernel.org
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > 
> > Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> > 
> > > ---
> > > v2 changes:
> > > - squashed patches together, because they depend on each other [ci]
> > > - added Fixes tag [ci]
> > > 
> > >  tools/lib/bpf/libbpf.c | 7 +++++--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index 8b0c3246097f..79d9607d26e2 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -5806,7 +5806,6 @@ static int load_module_btfs(struct bpf_object *obj)
> > >  {
> > >  	struct bpf_btf_info info;
> > >  	struct module_btf *mod_btf;
> > > -	struct btf *btf;
> > >  	char name[64];
> > >  	__u32 id = 0, len;
> > >  	int err, fd;
> > > @@ -5825,6 +5824,8 @@ static int load_module_btfs(struct bpf_object *obj)
> > >  		return 0;
> > >  
> > >  	while (true) {
> > > +		struct btf *btf = NULL;
> > > +
> > >  		err = bpf_btf_get_next_id(id, &id);
> > >  		if (err && errno == ENOENT)
> > >  			return 0;
> > > @@ -5878,7 +5879,7 @@ static int load_module_btfs(struct bpf_object *obj)
> > >  		if (err)
> > >  			goto err_out;
> > >  
> > > -		mod_btf = &obj->btf_modules[obj->btf_module_cnt++];
> > > +		mod_btf = &obj->btf_modules[obj->btf_module_cnt];
> > >  
> > >  		mod_btf->btf = btf;
> > >  		mod_btf->id = id;
> > > @@ -5888,9 +5889,11 @@ static int load_module_btfs(struct bpf_object *obj)
> > >  			err = -ENOMEM;
> > >  			goto err_out;
> > >  		}
> > > +		obj->btf_module_cnt++;
> > >  		continue;
> > >  
> > >  err_out:
> > > +		btf__free(btf);
> > >  		close(fd);
> > >  		return err;
> > >  	}
> > 
> > the err_out label within the loop is a bit confusing, needed more now I guess
> > since btf is in loop scope. only way round it would be to haul btf back up
> > to global scope and explicitly NULL it at the start of each iteration. That way 
> > we could break out of the loop to do error handling. Not a big deal though.
> 
> I guess you mean something like below (untested).. looks better, will check
> 
> thanks,
> jirka
> 
> 
> ---
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 8b0c3246097f..3a80a018fc7d 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -5852,11 +5852,12 @@ static int load_module_btfs(struct bpf_object *obj)
>  		info.name = ptr_to_u64(name);
>  		info.name_len = sizeof(name);
>  
> +		btf = NULL;
>  		err = bpf_btf_get_info_by_fd(fd, &info, &len);
>  		if (err) {
>  			err = -errno;
>  			pr_warn("failed to get BTF object #%d info: %s\n", id, errstr(err));
> -			goto err_out;
> +			break;
>  		}
>  
>  		/* ignore non-module BTFs */
> @@ -5870,15 +5871,15 @@ static int load_module_btfs(struct bpf_object *obj)
>  		if (err) {
>  			pr_warn("failed to load module [%s]'s BTF object #%d: %s\n",
>  				name, id, errstr(err));
> -			goto err_out;
> +			break;
>  		}
>  
>  		err = libbpf_ensure_mem((void **)&obj->btf_modules, &obj->btf_module_cap,
>  					sizeof(*obj->btf_modules), obj->btf_module_cnt + 1);
>  		if (err)
> -			goto err_out;
> +			break;
>  
> -		mod_btf = &obj->btf_modules[obj->btf_module_cnt++];
> +		mod_btf = &obj->btf_modules[obj->btf_module_cnt];
>  
>  		mod_btf->btf = btf;
>  		mod_btf->id = id;
> @@ -5886,16 +5887,16 @@ static int load_module_btfs(struct bpf_object *obj)
>  		mod_btf->name = strdup(name);
>  		if (!mod_btf->name) {
>  			err = -ENOMEM;
> -			goto err_out;
> +			break;
>  		}
> -		continue;
> +		obj->btf_module_cnt++;
> +	}
>  
> -err_out:
> +	if (err) {
> +		btf__free(btf);
>  		close(fd);
> -		return err;
>  	}
> -
> -	return 0;
> +	return err;
>  }
>  
>  static struct bpf_core_cand_list *