* [PATCH bpf-next v2 1/2] bpftool: Add HELP_SPEC_OPTIONS in token.c
@ 2025-09-17 3:47 Tao Chen
2025-09-17 3:47 ` [PATCH bpf-next v2 2/2] bpftool: Fix UAF in get_delegate_value Tao Chen
0 siblings, 1 reply; 5+ messages in thread
From: Tao Chen @ 2025-09-17 3:47 UTC (permalink / raw)
To: qmo, ast, daniel, andrii, martin.lau, eddyz87, song,
yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa,
chen.dylane
Cc: bpf, linux-kernel
$ ./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")
Acked-by: Quentin Monnet <qmo@kernel.org>
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
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH bpf-next v2 2/2] bpftool: Fix UAF in get_delegate_value 2025-09-17 3:47 [PATCH bpf-next v2 1/2] bpftool: Add HELP_SPEC_OPTIONS in token.c Tao Chen @ 2025-09-17 3:47 ` Tao Chen 2025-09-17 16:30 ` Quentin Monnet 0 siblings, 1 reply; 5+ messages in thread From: Tao Chen @ 2025-09-17 3:47 UTC (permalink / raw) To: qmo, ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, chen.dylane Cc: bpf, linux-kernel 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 | 75 +++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 42 deletions(-) diff --git a/tools/bpf/bpftool/token.c b/tools/bpf/bpftool/token.c index 82b829e44c8..05bc76c7276 100644 --- a/tools/bpf/bpftool/token.c +++ b/tools/bpf/bpftool/token.c @@ -28,15 +28,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 +43,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 +63,39 @@ static void print_items_per_line(const char *input, int items_per_line) printf("%-20s", str); cnt++; } - - free(strs); } +#define PRINT_DELEGATE_OPT(opt_name) do { \ + char *opts, *value; \ + opts = strdup(mntent->mnt_opts); \ + value = get_delegate_value(opts, opt_name); \ + print_items_per_line(value, ITEMS_PER_LINE); \ + free(opts); \ +} 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"); } -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,39 @@ 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); } +#define PRINT_DELEGATE_OPT_JSON(opt_name) do { \ + char *opts, *value; \ + opts = strdup(mntent->mnt_opts); \ + value = get_delegate_value(opts, opt_name); \ + split_json_array_str(value); \ + free(opts); \ +} 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v2 2/2] bpftool: Fix UAF in get_delegate_value 2025-09-17 3:47 ` [PATCH bpf-next v2 2/2] bpftool: Fix UAF in get_delegate_value Tao Chen @ 2025-09-17 16:30 ` Quentin Monnet 2025-09-17 21:00 ` Andrii Nakryiko 2025-09-18 11:17 ` Tao Chen 0 siblings, 2 replies; 5+ messages in thread From: Quentin Monnet @ 2025-09-17 16:30 UTC (permalink / raw) To: Tao Chen, ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa Cc: bpf, linux-kernel 2025-09-17 11:47 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 | 75 +++++++++++++++++---------------------- > 1 file changed, 33 insertions(+), 42 deletions(-) > > diff --git a/tools/bpf/bpftool/token.c b/tools/bpf/bpftool/token.c > index 82b829e44c8..05bc76c7276 100644 > --- a/tools/bpf/bpftool/token.c > +++ b/tools/bpf/bpftool/token.c > @@ -28,15 +28,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 +43,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 +63,39 @@ static void print_items_per_line(const char *input, int items_per_line) > printf("%-20s", str); > cnt++; > } > - > - free(strs); > } > > +#define PRINT_DELEGATE_OPT(opt_name) do { \ > + char *opts, *value; \ > + opts = strdup(mntent->mnt_opts); \ > + value = get_delegate_value(opts, opt_name); \ > + print_items_per_line(value, ITEMS_PER_LINE); \ > + free(opts); \ > +} while (0) Thanks! The fix looks OK to me, but why do you need to have PRINT_DELEGATE_OPT*() as macros? Can't you just use functions instead? Quentin ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v2 2/2] bpftool: Fix UAF in get_delegate_value 2025-09-17 16:30 ` Quentin Monnet @ 2025-09-17 21:00 ` Andrii Nakryiko 2025-09-18 11:17 ` Tao Chen 1 sibling, 0 replies; 5+ messages in thread From: Andrii Nakryiko @ 2025-09-17 21:00 UTC (permalink / raw) To: Quentin Monnet Cc: Tao Chen, ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf, linux-kernel On Wed, Sep 17, 2025 at 9:30 AM Quentin Monnet <qmo@kernel.org> wrote: > > 2025-09-17 11:47 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 | 75 +++++++++++++++++---------------------- > > 1 file changed, 33 insertions(+), 42 deletions(-) > > > > diff --git a/tools/bpf/bpftool/token.c b/tools/bpf/bpftool/token.c > > index 82b829e44c8..05bc76c7276 100644 > > --- a/tools/bpf/bpftool/token.c > > +++ b/tools/bpf/bpftool/token.c > > @@ -28,15 +28,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 +43,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 +63,39 @@ static void print_items_per_line(const char *input, int items_per_line) > > printf("%-20s", str); > > cnt++; > > } > > - > > - free(strs); > > } > > > > +#define PRINT_DELEGATE_OPT(opt_name) do { \ > > + char *opts, *value; \ > > + opts = strdup(mntent->mnt_opts); \ > > + value = get_delegate_value(opts, opt_name); \ > > + print_items_per_line(value, ITEMS_PER_LINE); \ > > + free(opts); \ > > +} while (0) > > > Thanks! The fix looks OK to me, but why do you need to have > PRINT_DELEGATE_OPT*() as macros? Can't you just use functions instead? right, function or just a loop, that will allow to also avoid repeating jsonw_name or printf calls: struct { const char *header, key; } sets = { {"allowed_cmds", "delegate_cmds"}, {"allowed_maps", "delegate_maps"}, ... }; for (i = 0; i < ARRAY_SIZE(sets); i++) { ... use sets[i].header and sets[i].key ... } pw-bot: cr > > Quentin ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v2 2/2] bpftool: Fix UAF in get_delegate_value 2025-09-17 16:30 ` Quentin Monnet 2025-09-17 21:00 ` Andrii Nakryiko @ 2025-09-18 11:17 ` Tao Chen 1 sibling, 0 replies; 5+ messages in thread From: Tao Chen @ 2025-09-18 11:17 UTC (permalink / raw) To: Quentin Monnet, ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa Cc: bpf, linux-kernel 在 2025/9/18 00:30, Quentin Monnet 写道: > 2025-09-17 11:47 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 | 75 +++++++++++++++++---------------------- >> 1 file changed, 33 insertions(+), 42 deletions(-) >> >> diff --git a/tools/bpf/bpftool/token.c b/tools/bpf/bpftool/token.c >> index 82b829e44c8..05bc76c7276 100644 >> --- a/tools/bpf/bpftool/token.c >> +++ b/tools/bpf/bpftool/token.c >> @@ -28,15 +28,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 +43,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 +63,39 @@ static void print_items_per_line(const char *input, int items_per_line) >> printf("%-20s", str); >> cnt++; >> } >> - >> - free(strs); >> } >> >> +#define PRINT_DELEGATE_OPT(opt_name) do { \ >> + char *opts, *value; \ >> + opts = strdup(mntent->mnt_opts); \ >> + value = get_delegate_value(opts, opt_name); \ >> + print_items_per_line(value, ITEMS_PER_LINE); \ >> + free(opts); \ >> +} while (0) > > > Thanks! The fix looks OK to me, but why do you need to have > PRINT_DELEGATE_OPT*() as macros? Can't you just use functions instead? > > Quentin Well, actually, there is no special purpose about using macros, i will use functions as you and Andrri suggested, thanks. -- Best Regards Tao Chen ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-09-18 11:18 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-17 3:47 [PATCH bpf-next v2 1/2] bpftool: Add HELP_SPEC_OPTIONS in token.c Tao Chen 2025-09-17 3:47 ` [PATCH bpf-next v2 2/2] bpftool: Fix UAF in get_delegate_value Tao Chen 2025-09-17 16:30 ` Quentin Monnet 2025-09-17 21:00 ` Andrii Nakryiko 2025-09-18 11:17 ` Tao Chen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.