[PATCH] libbpf: potential NULL dereference in usdt_manager_attach_usdt()

[PATCH] libbpf: potential NULL dereference in usdt_manager_attach_usdt()

[Haowen Bai]

link could be null but still dereference bpf_link__destroy(&link->link)
and it will lead to a null pointer access.

Signed-off-by: Haowen Bai <baihaowen@meizu.com>
---
 tools/lib/bpf/usdt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
index 1bce2eab5e89..b02ebc4ba57c 100644
--- a/tools/lib/bpf/usdt.c
+++ b/tools/lib/bpf/usdt.c
@@ -996,7 +996,7 @@ struct bpf_link *usdt_manager_attach_usdt(struct usdt_manager *man, const struct
 	link = calloc(1, sizeof(*link));
 	if (!link) {
 		err = -ENOMEM;
-		goto err_out;
+		goto link_err;
 	}
 
 	link->usdt_man = man;
@@ -1072,7 +1072,7 @@ struct bpf_link *usdt_manager_attach_usdt(struct usdt_manager *man, const struct
 
 err_out:
 	bpf_link__destroy(&link->link);
-
+link_err:
 	free(targets);
 	hashmap__free(specs_hash);
 	if (elf)
-- 
2.7.4

Re: [PATCH] libbpf: potential NULL dereference in usdt_manager_attach_usdt()

[Andrii Nakryiko]

On Wed, Apr 6, 2022 at 7:38 PM Haowen Bai <baihaowen@meizu.com> wrote:
>
> link could be null but still dereference bpf_link__destroy(&link->link)
> and it will lead to a null pointer access.
>
> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
> ---
>  tools/lib/bpf/usdt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
> index 1bce2eab5e89..b02ebc4ba57c 100644
> --- a/tools/lib/bpf/usdt.c
> +++ b/tools/lib/bpf/usdt.c
> @@ -996,7 +996,7 @@ struct bpf_link *usdt_manager_attach_usdt(struct usdt_manager *man, const struct
>         link = calloc(1, sizeof(*link));
>         if (!link) {
>                 err = -ENOMEM;
> -               goto err_out;
> +               goto link_err;

this is not a complete fix because there are two more similar goto
err_out; above which you didn't fix. I think better fix is to just add
if (link) check before bpf_link__destroy(), which is what I did
locally when applying.


>         }
>
>         link->usdt_man = man;
> @@ -1072,7 +1072,7 @@ struct bpf_link *usdt_manager_attach_usdt(struct usdt_manager *man, const struct
>
>  err_out:
>         bpf_link__destroy(&link->link);
> -
> +link_err:
>         free(targets);
>         hashmap__free(specs_hash);
>         if (elf)
> --
> 2.7.4
>

Re: [PATCH] libbpf: potential NULL dereference in usdt_manager_attach_usdt()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Thu, 7 Apr 2022 10:38:17 +0800 you wrote:
> link could be null but still dereference bpf_link__destroy(&link->link)
> and it will lead to a null pointer access.
> 
> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
> ---
>  tools/lib/bpf/usdt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Here is the summary with links:
  - libbpf: potential NULL dereference in usdt_manager_attach_usdt()
    https://git.kernel.org/bpf/bpf-next/c/e58c5c971746

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

Re: [PATCH] libbpf: potential NULL dereference in usdt_manager_attach_usdt()

[baihaowen]

在 4/8/22 3:04 AM, Andrii Nakryiko 写道:
> On Wed, Apr 6, 2022 at 7:38 PM Haowen Bai <baihaowen@meizu.com> wrote:
>> link could be null but still dereference bpf_link__destroy(&link->link)
>> and it will lead to a null pointer access.
>>
>> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
>> ---
>>  tools/lib/bpf/usdt.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
>> index 1bce2eab5e89..b02ebc4ba57c 100644
>> --- a/tools/lib/bpf/usdt.c
>> +++ b/tools/lib/bpf/usdt.c
>> @@ -996,7 +996,7 @@ struct bpf_link *usdt_manager_attach_usdt(struct usdt_manager *man, const struct
>>         link = calloc(1, sizeof(*link));
>>         if (!link) {
>>                 err = -ENOMEM;
>> -               goto err_out;
>> +               goto link_err;
> this is not a complete fix because there are two more similar goto
> err_out; above which you didn't fix. I think better fix is to just add
> if (link) check before bpf_link__destroy(), which is what I did
> locally when applying.
>
>
>>         }
>>
>>         link->usdt_man = man;
>> @@ -1072,7 +1072,7 @@ struct bpf_link *usdt_manager_attach_usdt(struct usdt_manager *man, const struct
>>
>>  err_out:
>>         bpf_link__destroy(&link->link);
>> -
>> +link_err:
>>         free(targets);
>>         hashmap__free(specs_hash);
>>         if (elf)
>> --
>> 2.7.4
>>
Thank you for your kindness help. :)

-- 
Haowen Bai