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

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

[Jiri Olsa]

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.

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

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

[Alan Maguire]

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?
 
> 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.

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

[Jiri Olsa]

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

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

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

[Alan Maguire]

On 15/04/2026 13:54, 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
> 
>>  
>>> 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
>

looks good to me, but let's see what the bots say ;-) . Thanks!
 
> 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 *

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

[Jiri Olsa]

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 *