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

[PATCH bpf-next 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(+)

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 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 strdup
a new buffer.

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

diff --git a/tools/bpf/bpftool/token.c b/tools/bpf/bpftool/token.c
index 82b829e44c8..c47256d8038 100644
--- a/tools/bpf/bpftool/token.c
+++ b/tools/bpf/bpftool/token.c
@@ -28,6 +28,12 @@ static bool has_delegate_options(const char *mnt_ops)
 	       strstr(mnt_ops, "delegate_attachs");
 }
 
+static void free_delegate_value(char *value)
+{
+	if (value)
+		free(value);
+}
+
 static char *get_delegate_value(const char *opts, const char *key)
 {
 	char *token, *rest, *ret = NULL;
@@ -40,7 +46,7 @@ static char *get_delegate_value(const char *opts, const char *key)
 			token = strtok_r(NULL, ",", &rest)) {
 		if (strncmp(token, key, strlen(key)) == 0 &&
 		    token[strlen(key)] == '=') {
-			ret = token + strlen(key) + 1;
+			ret = strdup(token + strlen(key) + 1);
 			break;
 		}
 	}
@@ -73,28 +79,29 @@ static void print_items_per_line(const char *input, int items_per_line)
 	free(strs);
 }
 
+#define PRINT_DELEGATE_OPT(opt_name) do {				\
+	char *value = get_delegate_value(mntent->mnt_opts, opt_name);	\
+	print_items_per_line(value, ITEMS_PER_LINE);			\
+	free_delegate_value(value);					\
+} while (0)
+
 #define ITEMS_PER_LINE 4
 static void show_token_info_plain(struct mntent *mntent)
 {
-	char *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);
+	PRINT_DELEGATE_OPT("delegate_cmds");
 
 	printf("\n\tallowed_maps:");
-	value = get_delegate_value(mntent->mnt_opts, "delegate_maps");
-	print_items_per_line(value, ITEMS_PER_LINE);
+	PRINT_DELEGATE_OPT("delegate_maps");
 
 	printf("\n\tallowed_progs:");
-	value = get_delegate_value(mntent->mnt_opts, "delegate_progs");
-	print_items_per_line(value, ITEMS_PER_LINE);
+	PRINT_DELEGATE_OPT("delegate_progs");
 
 	printf("\n\tallowed_attachs:");
-	value = get_delegate_value(mntent->mnt_opts, "delegate_attachs");
-	print_items_per_line(value, ITEMS_PER_LINE);
+	PRINT_DELEGATE_OPT("delegate_attachs");
 	printf("\n");
 }
 
@@ -122,29 +129,29 @@ static void split_json_array_str(const char *input)
 	free(strs);
 }
 
+#define PRINT_DELEGATE_OPT_JSON(opt_name) do {				\
+	char *value = get_delegate_value(mntent->mnt_opts, opt_name);	\
+	split_json_array_str(value);					\
+	free_delegate_value(value);					\
+} while (0)
+
 static void show_token_info_json(struct mntent *mntent)
 {
-	char *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);
+	PRINT_DELEGATE_OPT_JSON("delegate_cmds");
 
 	jsonw_name(json_wtr, "allowed_maps");
-	value = get_delegate_value(mntent->mnt_opts, "delegate_maps");
-	split_json_array_str(value);
+	PRINT_DELEGATE_OPT_JSON("delegate_maps");
 
 	jsonw_name(json_wtr, "allowed_progs");
-	value = get_delegate_value(mntent->mnt_opts, "delegate_progs");
-	split_json_array_str(value);
+	PRINT_DELEGATE_OPT_JSON("delegate_progs");
 
 	jsonw_name(json_wtr, "allowed_attachs");
-	value = get_delegate_value(mntent->mnt_opts, "delegate_attachs");
-	split_json_array_str(value);
+	PRINT_DELEGATE_OPT_JSON("delegate_attachs");
 
 	jsonw_end_object(json_wtr);
 }
-- 
2.48.1

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

[Alexei Starovoitov]

On Mon, Sep 15, 2025 at 10:42 PM Tao Chen <chen.dylane@linux.dev> wrote:
>
> The return value ret pointer is pointing opts_copy, but opts_copy
> gets freed in get_delegate_value before return, fix this by strdup
> a new buffer.
>
> Fixes: 2d812311c2b2 ("bpftool: Add bpf_token show")
> Signed-off-by: Tao Chen <chen.dylane@linux.dev>
> ---
>  tools/bpf/bpftool/token.c | 47 ++++++++++++++++++++++-----------------
>  1 file changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/tools/bpf/bpftool/token.c b/tools/bpf/bpftool/token.c
> index 82b829e44c8..c47256d8038 100644
> --- a/tools/bpf/bpftool/token.c
> +++ b/tools/bpf/bpftool/token.c
> @@ -28,6 +28,12 @@ static bool has_delegate_options(const char *mnt_ops)
>                strstr(mnt_ops, "delegate_attachs");
>  }
>
> +static void free_delegate_value(char *value)
> +{
> +       if (value)
> +               free(value);
> +}
> +
>  static char *get_delegate_value(const char *opts, const char *key)
>  {
>         char *token, *rest, *ret = NULL;
> @@ -40,7 +46,7 @@ static char *get_delegate_value(const char *opts, const char *key)
>                         token = strtok_r(NULL, ",", &rest)) {
>                 if (strncmp(token, key, strlen(key)) == 0 &&
>                     token[strlen(key)] == '=') {
> -                       ret = token + strlen(key) + 1;
> +                       ret = strdup(token + strlen(key) + 1);

Instead of adding more strdup-s
strdup(mntent->mnt_opts) once per cmd/map/prog and
remove another strdrup/free in print_items_per_line().

pw-bot: cr

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

[Quentin Monnet]

2025-09-16 13:41 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>


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

Thanks!

I'll review your v2 with Alexei's comment addressed for your second patch.

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

[Tao Chen]

在 2025/9/17 01:07, Alexei Starovoitov 写道:
> On Mon, Sep 15, 2025 at 10:42 PM Tao Chen <chen.dylane@linux.dev> wrote:
>>
>> The return value ret pointer is pointing opts_copy, but opts_copy
>> gets freed in get_delegate_value before return, fix this by strdup
>> a new buffer.
>>
>> Fixes: 2d812311c2b2 ("bpftool: Add bpf_token show")
>> Signed-off-by: Tao Chen <chen.dylane@linux.dev>
>> ---
>>   tools/bpf/bpftool/token.c | 47 ++++++++++++++++++++++-----------------
>>   1 file changed, 27 insertions(+), 20 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/token.c b/tools/bpf/bpftool/token.c
>> index 82b829e44c8..c47256d8038 100644
>> --- a/tools/bpf/bpftool/token.c
>> +++ b/tools/bpf/bpftool/token.c
>> @@ -28,6 +28,12 @@ static bool has_delegate_options(const char *mnt_ops)
>>                 strstr(mnt_ops, "delegate_attachs");
>>   }
>>
>> +static void free_delegate_value(char *value)
>> +{
>> +       if (value)
>> +               free(value);
>> +}
>> +
>>   static char *get_delegate_value(const char *opts, const char *key)
>>   {
>>          char *token, *rest, *ret = NULL;
>> @@ -40,7 +46,7 @@ static char *get_delegate_value(const char *opts, const char *key)
>>                          token = strtok_r(NULL, ",", &rest)) {
>>                  if (strncmp(token, key, strlen(key)) == 0 &&
>>                      token[strlen(key)] == '=') {
>> -                       ret = token + strlen(key) + 1;
>> +                       ret = strdup(token + strlen(key) + 1);
> 
> Instead of adding more strdup-s
> strdup(mntent->mnt_opts) once per cmd/map/prog and
> remove another strdrup/free in print_items_per_line().
> 
> pw-bot: cr

will remove it in v2, thanks.

-- 
Best Regards
Tao Chen