linux-kernel - Re: [PATCH bpf-next v3 2/2] bpftool: Fix UAF in get_delegate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <226947cd-a28d-4c09-81cb-0fa0a21c7075@kernel.org>
Date: Thu, 18 Sep 2025 21:07:58 +0100
From: Quentin Monnet <qmo@...nel.org>
To: Tao Chen <chen.dylane@...ux.dev>, ast@...nel.org, daniel@...earbox.net,
 andrii@...nel.org, martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org,
 yonghong.song@...ux.dev, john.fastabend@...il.com, kpsingh@...nel.org,
 sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org
Cc: bpf@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v3 2/2] bpftool: Fix UAF in get_delegate_value

2025-09-18 20:09 UTC+0800 ~ Tao Chen <chen.dylane@...ux.dev>
> The return value ret pointer is pointing opts_copy, but opts_copy
> gets freed in get_delegate_value before return, fix this by free
> the mntent->mnt_opts strdup memory after show delegate value.
> 
> Fixes: 2d812311c2b2 ("bpftool: Add bpf_token show")
> Signed-off-by: Tao Chen <chen.dylane@...ux.dev>
> ---
>  tools/bpf/bpftool/token.c | 90 +++++++++++++++------------------------
>  1 file changed, 35 insertions(+), 55 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/token.c b/tools/bpf/bpftool/token.c
> index 82b829e44c8..20c4c78a8a8 100644
> --- a/tools/bpf/bpftool/token.c
> +++ b/tools/bpf/bpftool/token.c


> @@ -69,38 +73,29 @@ static void print_items_per_line(const char *input, int items_per_line)
>  		printf("%-20s", str);
>  		cnt++;
>  	}
> -
> -	free(strs);
>  }
>  
>  #define ITEMS_PER_LINE 4
>  static void show_token_info_plain(struct mntent *mntent)
>  {
> -	char *value;
> +	char *opts, *value;


Thank you! I just have style nits: can you move the declaration of
"opts" and "value" inside of the for loop, please? They're not used
outside of it.


>  
>  	printf("token_info  %s", mntent->mnt_dir);
>  
> -	printf("\n\tallowed_cmds:");
> -	value = get_delegate_value(mntent->mnt_opts, "delegate_cmds");
> -	print_items_per_line(value, ITEMS_PER_LINE);
> -
> -	printf("\n\tallowed_maps:");
> -	value = get_delegate_value(mntent->mnt_opts, "delegate_maps");
> -	print_items_per_line(value, ITEMS_PER_LINE);
> -
> -	printf("\n\tallowed_progs:");
> -	value = get_delegate_value(mntent->mnt_opts, "delegate_progs");
> -	print_items_per_line(value, ITEMS_PER_LINE);
> +	for (size_t i = 0; i < ARRAY_SIZE(sets); i++) {


And could you please move the declaration of variable "i" to the top of
the function, for consistency with the rest of the code?

Same comments for the JSON function.

Thanks,
Quentin