[PATCH bpf-next v3 1/2] bpftool: Add HELP_SPEC_OPTIONS in token.c

[PATCH bpf-next v3 1/2] bpftool: Add HELP_SPEC_OPTIONS in token.c

[Tao Chen]

$ ./bpftool token help

Usage: bpftool token { show | list }
       bpftool token help
       OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} }

Fixes: 2d812311c2b2 ("bpftool: Add bpf_token show")
Signed-off-by: Tao Chen <chen.dylane@linux.dev>
---
 tools/bpf/bpftool/token.c | 1 +
 1 file changed, 1 insertion(+)

Change list:
 v1 -> v2:
  - strdup(mntent->mnt_opts) once per cmd/map/prog and
    remove another strdrup/free in print_items_per_line
    in patch2.(Alexei)
  v1: https://lore.kernel.org/bpf/20250916054111.1151487-1-chen.dylane@linux.dev
 v2 -> v3:
  - Replace PRINT_DELEGATE_OPT* macros with functions.(Quentin)
  v2: https://lore.kernel.org/bpf/20250917034732.1185429-2-chen.dylane@linux.dev

diff --git a/tools/bpf/bpftool/token.c b/tools/bpf/bpftool/token.c
index 6312e662a12..82b829e44c8 100644
--- a/tools/bpf/bpftool/token.c
+++ b/tools/bpf/bpftool/token.c
@@ -206,6 +206,7 @@ static int do_help(int argc, char **argv)
 	fprintf(stderr,
 		"Usage: %1$s %2$s { show | list }\n"
 		"       %1$s %2$s help\n"
+		"       " HELP_SPEC_OPTIONS " }\n"
 		"\n"
 		"",
 		bin_name, argv[-2]);
-- 
2.48.1

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

[Tao Chen]

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@linux.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
@@ -20,6 +20,16 @@
 
 #define MOUNTS_FILE "/proc/mounts"
 
+struct {
+	const char *header;
+	const char *key;
+} sets[] = {
+	{"allowed_cmds", "delegate_cmds"},
+	{"allowed_maps", "delegate_maps"},
+	{"allowed_progs", "delegate_progs"},
+	{"allowed_attachs", "delegate_attachs"},
+};
+
 static bool has_delegate_options(const char *mnt_ops)
 {
 	return strstr(mnt_ops, "delegate_cmds") ||
@@ -28,15 +38,14 @@ static bool has_delegate_options(const char *mnt_ops)
 	       strstr(mnt_ops, "delegate_attachs");
 }
 
-static char *get_delegate_value(const char *opts, const char *key)
+static char *get_delegate_value(char *opts, const char *key)
 {
 	char *token, *rest, *ret = NULL;
-	char *opts_copy = strdup(opts);
 
-	if (!opts_copy)
+	if (!opts)
 		return NULL;
 
-	for (token = strtok_r(opts_copy, ",", &rest); token;
+	for (token = strtok_r(opts, ",", &rest); token;
 			token = strtok_r(NULL, ",", &rest)) {
 		if (strncmp(token, key, strlen(key)) == 0 &&
 		    token[strlen(key)] == '=') {
@@ -44,24 +53,19 @@ static char *get_delegate_value(const char *opts, const char *key)
 			break;
 		}
 	}
-	free(opts_copy);
 
 	return ret;
 }
 
-static void print_items_per_line(const char *input, int items_per_line)
+static void print_items_per_line(char *input, int items_per_line)
 {
-	char *str, *rest, *strs;
+	char *str, *rest;
 	int cnt = 0;
 
 	if (!input)
 		return;
 
-	strs = strdup(input);
-	if (!strs)
-		return;
-
-	for (str = strtok_r(strs, ":", &rest); str;
+	for (str = strtok_r(input, ":", &rest); str;
 			str = strtok_r(NULL, ":", &rest)) {
 		if (cnt % items_per_line == 0)
 			printf("\n\t  ");
@@ -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;
 
 	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++) {
+		printf("\n\t%s:", sets[i].header);
+		opts = strdup(mntent->mnt_opts);
+		value = get_delegate_value(opts, sets[i].key);
+		print_items_per_line(value, ITEMS_PER_LINE);
+		free(opts);
+	}
 
-	printf("\n\tallowed_attachs:");
-	value = get_delegate_value(mntent->mnt_opts, "delegate_attachs");
-	print_items_per_line(value, ITEMS_PER_LINE);
 	printf("\n");
 }
 
-static void split_json_array_str(const char *input)
+static void split_json_array_str(char *input)
 {
-	char *str, *rest, *strs;
+	char *str, *rest;
 
 	if (!input) {
 		jsonw_start_array(json_wtr);
@@ -108,43 +103,28 @@ static void split_json_array_str(const char *input)
 		return;
 	}
 
-	strs = strdup(input);
-	if (!strs)
-		return;
-
 	jsonw_start_array(json_wtr);
-	for (str = strtok_r(strs, ":", &rest); str;
+	for (str = strtok_r(input, ":", &rest); str;
 			str = strtok_r(NULL, ":", &rest)) {
 		jsonw_string(json_wtr, str);
 	}
 	jsonw_end_array(json_wtr);
-
-	free(strs);
 }
 
 static void show_token_info_json(struct mntent *mntent)
 {
-	char *value;
+	char *opts, *value;
 
 	jsonw_start_object(json_wtr);
-
 	jsonw_string_field(json_wtr, "token_info", mntent->mnt_dir);
 
-	jsonw_name(json_wtr, "allowed_cmds");
-	value = get_delegate_value(mntent->mnt_opts, "delegate_cmds");
-	split_json_array_str(value);
-
-	jsonw_name(json_wtr, "allowed_maps");
-	value = get_delegate_value(mntent->mnt_opts, "delegate_maps");
-	split_json_array_str(value);
-
-	jsonw_name(json_wtr, "allowed_progs");
-	value = get_delegate_value(mntent->mnt_opts, "delegate_progs");
-	split_json_array_str(value);
-
-	jsonw_name(json_wtr, "allowed_attachs");
-	value = get_delegate_value(mntent->mnt_opts, "delegate_attachs");
-	split_json_array_str(value);
+	for (size_t i = 0; i < ARRAY_SIZE(sets); i++) {
+		jsonw_name(json_wtr, sets[i].header);
+		opts = strdup(mntent->mnt_opts);
+		value = get_delegate_value(opts, sets[i].key);
+		split_json_array_str(value);
+		free(opts);
+	}
 
 	jsonw_end_object(json_wtr);
 }
-- 
2.48.1

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@linux.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@linux.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

Re: [PATCH bpf-next v3 1/2] bpftool: Add HELP_SPEC_OPTIONS in token.c

[Quentin Monnet]

2025-09-18 20:09 UTC+0800 ~ Tao Chen <chen.dylane@linux.dev>
> $ ./bpftool token help
> 
> Usage: bpftool token { show | list }
>        bpftool token help
>        OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} }
> 
> Fixes: 2d812311c2b2 ("bpftool: Add bpf_token show")
> Signed-off-by: Tao Chen <chen.dylane@linux.dev>

I acked this patch before.

Acked-by: Quentin Monnet <qmo@kernel.org>

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

[Tao Chen]

在 2025/9/19 04:07, Quentin Monnet 写道:
> 2025-09-18 20:09 UTC+0800 ~ Tao Chen <chen.dylane@linux.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@linux.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.
>

No problem, will change it in v4, thanks.

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

also will change it in v4, thanks.

> Same comments for the JSON function.
> 
> Thanks,
> Quentin
-- 
Best Regards
Tao Chen