tools/lib/subcmd/help.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
When there is no exclusion occurring from the cmds list - for example -
cmds contains ["read-vdso32"] and excludes contains ["archive"] - the
main loop completes with ci == cj == 0. In the original code the loop
processing the remaining elements in the list was conditional:
if (ci != cj) { ...}
So we end up in the assertion loop since ci < cmds->cnt and we
incorrectly try to assert the list elements to be NULL and fail with
the following error
help.c:104: exclude_cmds: Assertion `cmds->names[ci] == NULL' failed.
Fix this by moving the if (ci != cj) check inside of a broader loop.
If ci != cj, left shift the list elements, as before, and then
unconditionally advance the ci and cj indicies which also covers the
ci == cj case.
Fixes: 1fdf938168c4d26f ("perf tools: Fix use-after-free in help_unknown_cmd()")
Signed-off-by: Sri Jayaramappa <sjayaram@akamai.com>
---
tools/lib/subcmd/help.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/tools/lib/subcmd/help.c b/tools/lib/subcmd/help.c
index ddaeb4eb3e24..db94aa685b73 100644
--- a/tools/lib/subcmd/help.c
+++ b/tools/lib/subcmd/help.c
@@ -97,11 +97,13 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
ei++;
}
}
- if (ci != cj) {
- while (ci < cmds->cnt) {
- cmds->names[cj++] = cmds->names[ci];
- cmds->names[ci++] = NULL;
+ while (ci < cmds->cnt) {
+ if (ci != cj) {
+ cmds->names[cj] = cmds->names[ci];
+ cmds->names[ci] = NULL;
}
+ ci++;
+ cj++;
}
for (ci = cj; ci < cmds->cnt; ci++)
assert(cmds->names[ci] == NULL);
--
2.34.1On Tue, Dec 2, 2025 at 1:40 PM Sri Jayaramappa <sjayaram@akamai.com> wrote:
>
> When there is no exclusion occurring from the cmds list - for example -
> cmds contains ["read-vdso32"] and excludes contains ["archive"] - the
> main loop completes with ci == cj == 0. In the original code the loop
> processing the remaining elements in the list was conditional:
>
> if (ci != cj) { ...}
>
> So we end up in the assertion loop since ci < cmds->cnt and we
> incorrectly try to assert the list elements to be NULL and fail with
> the following error
>
> help.c:104: exclude_cmds: Assertion `cmds->names[ci] == NULL' failed.
>
> Fix this by moving the if (ci != cj) check inside of a broader loop.
> If ci != cj, left shift the list elements, as before, and then
> unconditionally advance the ci and cj indicies which also covers the
> ci == cj case.
>
> Fixes: 1fdf938168c4d26f ("perf tools: Fix use-after-free in help_unknown_cmd()")
>
> Signed-off-by: Sri Jayaramappa <sjayaram@akamai.com>
Thanks Sri! Guilherme reported a related issue and I think your fix covers it:
https://lore.kernel.org/lkml/aTXQw9dtP5df7LmP@gentoo.org/
I came up with the following test based on your commit message, could
you validate it covers the case for your fix:
```
diff --git a/tools/perf/tests/subcmd-help.c b/tools/perf/tests/subcmd-help.c
index 2280b4c0e5e7..9da96a16fd20 100644
--- a/tools/perf/tests/subcmd-help.c
+++ b/tools/perf/tests/subcmd-help.c
@@ -95,10 +95,36 @@ static int test__exclude_cmdnames(struct
test_suite *test __maybe_unused,
return TEST_OK;
}
+static int test__exclude_cmdnames_no_overlap(struct test_suite *test
__maybe_unused,
+ int subtest __maybe_unused)
+{
+ struct cmdnames cmds1 = {};
+ struct cmdnames cmds2 = {};
+
+ add_cmdname(&cmds1, "read-vdso32", 11);
+ add_cmdname(&cmds2, "archive", 7);
+
+ TEST_ASSERT_VAL("invalid original size", cmds1.cnt == 1);
+ TEST_ASSERT_VAL("invalid original size", cmds2.cnt == 1);
+
+ exclude_cmds(&cmds1, &cmds2);
+
+ TEST_ASSERT_VAL("invalid excluded size", cmds1.cnt == 1);
+ TEST_ASSERT_VAL("invalid excluded size", cmds2.cnt == 1);
+
+ TEST_ASSERT_VAL("cannot find cmd", is_in_cmdlist(&cmds1,
"read-vdso32") == 1);
+ TEST_ASSERT_VAL("wrong cmd", is_in_cmdlist(&cmds1, "archive") == 0);
+
+ clean_cmdnames(&cmds1);
+ clean_cmdnames(&cmds2);
+ return TEST_OK;
+}
+
static struct test_case tests__subcmd_help[] = {
TEST_CASE("Load subcmd names", load_cmdnames),
TEST_CASE("Uniquify subcmd names", uniq_cmdnames),
TEST_CASE("Exclude duplicate subcmd names", exclude_cmdnames),
+ TEST_CASE("Exclude disjoint subcmd names", exclude_cmdnames_no_overlap),
{ .name = NULL, }
};
```
If you build perf you can run the test like:
```
$ perf test -v help
68: libsubcmd help tests :
68.1: Load subcmd names : Ok
68.2: Uniquify subcmd names : Ok
68.3: Exclude duplicate subcmd names : Ok
68.4: Exclude disjoint subcmd names : Ok
```
Perhaps you can think of other values for the test so we don't run
into more issues.
Thanks,
Ian
> ---
> tools/lib/subcmd/help.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/tools/lib/subcmd/help.c b/tools/lib/subcmd/help.c
> index ddaeb4eb3e24..db94aa685b73 100644
> --- a/tools/lib/subcmd/help.c
> +++ b/tools/lib/subcmd/help.c
> @@ -97,11 +97,13 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
> ei++;
> }
> }
> - if (ci != cj) {
> - while (ci < cmds->cnt) {
> - cmds->names[cj++] = cmds->names[ci];
> - cmds->names[ci++] = NULL;
> + while (ci < cmds->cnt) {
> + if (ci != cj) {
> + cmds->names[cj] = cmds->names[ci];
> + cmds->names[ci] = NULL;
> }
> + ci++;
> + cj++;
> }
> for (ci = cj; ci < cmds->cnt; ci++)
> assert(cmds->names[ci] == NULL);
> --
> 2.34.1
>
Hi Ian, Yes, this covers my case. I had thought through other possible scenarios before sending the patch, and I don’t see any additional issues. Best -Sri
On Tue, Dec 9, 2025 at 6:00 PM Sri Jayaramappa <sjayaram@akamai.com> wrote: > > Hi Ian, > > Yes, this covers my case. I had thought through other possible scenarios before sending the patch, and I don’t see any additional issues. Thanks Sri, any chance you could reply to the test e-mail with a "Reviewed-by: Sri Jayaramappa <sjayaram@akamai.com>" ? It makes it easier for patch managers to spot that the code is reviewed. Thanks! Ian
________________________________________ From: Ian Rogers <irogers@google.com> Sent: Wednesday, December 10, 2025 10:38 AM To: Jayaramappa, Srilakshmi Cc: acme@kernel.org; amadio@gentoo.org; Hunt, Joshua; linux-kernel@vger.kernel.org; linux-perf-users@vger.kernel.org; namhyung@kernel.org; peterz@infradead.org Subject: Re: [PATCH] libsubcmd: Fix null intersection case in exclude_cmds() !-------------------------------------------------------------------| This Message Is From an External Sender This message came from outside your organization. |-------------------------------------------------------------------! On Tue, Dec 9, 2025 at 6:00 PM Sri Jayaramappa <sjayaram@akamai.com> wrote: > > Hi Ian, > > Yes, this covers my case. I had thought through other possible scenarios before sending the patch, and I don’t see any additional issues. Thanks Sri, any chance you could reply to the test e-mail with a "Reviewed-by: Sri Jayaramappa <sjayaram@akamai.com>" ? It makes it easier for patch managers to spot that the code is reviewed. Thanks! Ian Absolutely! Just replied to the test email. And apologies for the weird email formatting on my previous message. Tried to use git send-email for the reply and it didn't do what I thought I was going to... Thanks -Sri
On Sun, Dec 7, 2025 at 2:16 PM Ian Rogers <irogers@google.com> wrote:
>
> On Tue, Dec 2, 2025 at 1:40 PM Sri Jayaramappa <sjayaram@akamai.com> wrote:
> >
> > When there is no exclusion occurring from the cmds list - for example -
> > cmds contains ["read-vdso32"] and excludes contains ["archive"] - the
> > main loop completes with ci == cj == 0. In the original code the loop
> > processing the remaining elements in the list was conditional:
> >
> > if (ci != cj) { ...}
> >
> > So we end up in the assertion loop since ci < cmds->cnt and we
> > incorrectly try to assert the list elements to be NULL and fail with
> > the following error
> >
> > help.c:104: exclude_cmds: Assertion `cmds->names[ci] == NULL' failed.
> >
> > Fix this by moving the if (ci != cj) check inside of a broader loop.
> > If ci != cj, left shift the list elements, as before, and then
> > unconditionally advance the ci and cj indicies which also covers the
> > ci == cj case.
> >
> > Fixes: 1fdf938168c4d26f ("perf tools: Fix use-after-free in help_unknown_cmd()")
> >
> > Signed-off-by: Sri Jayaramappa <sjayaram@akamai.com>
>
> Thanks Sri! Guilherme reported a related issue and I think your fix covers it:
> https://lore.kernel.org/lkml/aTXQw9dtP5df7LmP@gentoo.org/
>
> I came up with the following test based on your commit message, could
> you validate it covers the case for your fix:
> ```
> diff --git a/tools/perf/tests/subcmd-help.c b/tools/perf/tests/subcmd-help.c
> index 2280b4c0e5e7..9da96a16fd20 100644
> --- a/tools/perf/tests/subcmd-help.c
> +++ b/tools/perf/tests/subcmd-help.c
> @@ -95,10 +95,36 @@ static int test__exclude_cmdnames(struct
> test_suite *test __maybe_unused,
> return TEST_OK;
> }
>
> +static int test__exclude_cmdnames_no_overlap(struct test_suite *test
> __maybe_unused,
> + int subtest __maybe_unused)
> +{
> + struct cmdnames cmds1 = {};
> + struct cmdnames cmds2 = {};
> +
> + add_cmdname(&cmds1, "read-vdso32", 11);
> + add_cmdname(&cmds2, "archive", 7);
> +
> + TEST_ASSERT_VAL("invalid original size", cmds1.cnt == 1);
> + TEST_ASSERT_VAL("invalid original size", cmds2.cnt == 1);
> +
> + exclude_cmds(&cmds1, &cmds2);
> +
> + TEST_ASSERT_VAL("invalid excluded size", cmds1.cnt == 1);
> + TEST_ASSERT_VAL("invalid excluded size", cmds2.cnt == 1);
> +
> + TEST_ASSERT_VAL("cannot find cmd", is_in_cmdlist(&cmds1,
> "read-vdso32") == 1);
> + TEST_ASSERT_VAL("wrong cmd", is_in_cmdlist(&cmds1, "archive") == 0);
> +
> + clean_cmdnames(&cmds1);
> + clean_cmdnames(&cmds2);
> + return TEST_OK;
> +}
> +
> static struct test_case tests__subcmd_help[] = {
> TEST_CASE("Load subcmd names", load_cmdnames),
> TEST_CASE("Uniquify subcmd names", uniq_cmdnames),
> TEST_CASE("Exclude duplicate subcmd names", exclude_cmdnames),
> + TEST_CASE("Exclude disjoint subcmd names", exclude_cmdnames_no_overlap),
> { .name = NULL, }
> };
> ```
> If you build perf you can run the test like:
> ```
> $ perf test -v help
> 68: libsubcmd help tests :
> 68.1: Load subcmd names : Ok
> 68.2: Uniquify subcmd names : Ok
> 68.3: Exclude duplicate subcmd names : Ok
> 68.4: Exclude disjoint subcmd names : Ok
> ```
> Perhaps you can think of other values for the test so we don't run
> into more issues.
Running the test above I get the following before:
```
68.4: Exclude disjoint subcmd names:
--- start ---
test child forked, pid 1443868
perf: help.c:107: exclude_cmds: Assertion `cmds->names[ci] == NULL' failed.
---- unexpected signal (6) ----
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
Failed to read build ID for //anon
#0 0x55ed35f172de in child_test_sig_handler builtin-test.c:312
#1 0x7fea1d049df0 in __restore_rt libc_sigaction.c:0
#2 0x7fea1d09e95c in __pthread_kill_implementation pthread_kill.c:44
#3 0x7fea1d049cc2 in raise raise.c:27
#4 0x7fea1d0324ac in abort abort.c:81
#5 0x7fea1d032420 in __assert_perror_fail assert-perr.c:31
#6 0x55ed35ea1524 in exclude_cmds help.c:106
#7 0x55ed35f62e82 in test__exclude_cmdnames_no_overlap subcmd-help.c:112
#8 0x55ed35f17470 in run_test_child builtin-test.c:340
#9 0x55ed35ea5834 in start_command run-command.c:128
#10 0x55ed35f17ee3 in start_test builtin-test.c:546
#11 0x55ed35f1838d in __cmd_test builtin-test.c:648
#12 0x55ed35f18f2d in cmd_test builtin-test.c:850
#13 0x55ed35e94494 in run_builtin perf.c:349
#14 0x55ed35e9472c in handle_internal_command perf.c:401
#15 0x55ed35e94885 in run_argv perf.c:448
#16 0x55ed35e94bce in main perf.c:555
#17 0x7fea1d033ca8 in __libc_start_call_main libc_start_call_main.h:74
#18 0x7fea1d033d65 in __libc_start_main@@GLIBC_2.34 libc-start.c:128
#19 0x55ed35de4f41 in _start perf[52f41]
68.4: Exclude disjoint subcmd names : FAILED!
```
After the test passes and is address/leak sanitizer clean.
Tested-by: Ian Rogers <irogers@google.com>
I posted the test as a patch here:
https://lore.kernel.org/lkml/20251208172339.1445817-1-irogers@google.com/
Thanks,
Ian
> Thanks,
> Ian
>
> > ---
> > tools/lib/subcmd/help.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/lib/subcmd/help.c b/tools/lib/subcmd/help.c
> > index ddaeb4eb3e24..db94aa685b73 100644
> > --- a/tools/lib/subcmd/help.c
> > +++ b/tools/lib/subcmd/help.c
> > @@ -97,11 +97,13 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
> > ei++;
> > }
> > }
> > - if (ci != cj) {
> > - while (ci < cmds->cnt) {
> > - cmds->names[cj++] = cmds->names[ci];
> > - cmds->names[ci++] = NULL;
> > + while (ci < cmds->cnt) {
> > + if (ci != cj) {
> > + cmds->names[cj] = cmds->names[ci];
> > + cmds->names[ci] = NULL;
> > }
> > + ci++;
> > + cj++;
> > }
> > for (ci = cj; ci < cmds->cnt; ci++)
> > assert(cmds->names[ci] == NULL);
> > --
> > 2.34.1
> >
© 2016 - 2025 Red Hat, Inc.