Re: [PATCH bpf-next v3 2/2] bpftool: Fix UAF in get_delegate_value

[Quentin Monnet]

2025-09-18 20:09 UTC+0800 ~ Tao Chen <chen.dylane@xxxxxxxxx>
> 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@xxxxxxxxx>
> ---
>  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