[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fXwPrR-gwPbD7CcY6_fKg=s0yfNWSasFyYVQxCDhKKm=A@mail.gmail.com>
Date: Wed, 21 Jan 2026 08:12:52 -0800
From: Ian Rogers <irogers@...gle.com>
To: Thomas Richter <tmricht@...ux.ibm.com>, jayaram@...mai.com
Cc: linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
linux-perf-users@...r.kernel.org, acme@...nel.org, namhyung@...nel.org,
agordeev@...ux.ibm.com, gor@...ux.ibm.com, sumanthk@...ux.ibm.com,
hca@...ux.ibm.com, japo@...ux.ibm.com
Subject: Re: [PATCH V2 linux-next] perf test: Subtest Exclude disjoint subcmd
names fails
On Wed, Jan 21, 2026 at 12:24 AM Thomas Richter <tmricht@...ux.ibm.com> wrote:
>
> V1 --> V2: Add linux next repository
> s/needed/unneeded/ in commit message
>
> The perf test case 'libsubcmd help tests', subtest
> 'Exclude disjoint subcmd names' fails all the time.
>
> Root case is a special case of sorted input which the exclude_cmds()
> in libsubcmd can not handle. Assume the following inputs:
> cmds = { X, Y, Z } and excludes = { A, B }.
>
> This leads to
> ci cj ei cmds-name excludes
> ----------|--------------------
> 0 0 0 | X A : cmp > 0, ei++
> 0 0 1 | X B : cmp > 0, ei++
>
> At this point, the loop is terminated due to ei == excludes->cnt.
> The for-loop now checks for trailing names which had to be deleted.
> But the first entry points to a name: cmds->names[0].name == "X"
> and this is a valid entry.
>
> This is the case when all commands listed in excludes are less than
> all commands listed in cmds.
> Only check for existing names when cmds list was changed, that is ci != cj.
>
> Also remove an unneeded if (cmp > 0).
>
> -
> Output before:
> # ./perf test -F 68
> 68.1: Load subcmd names : Ok
> 68.2: Uniquify subcmd names : Ok
> 68.3: Exclude duplicate subcmd names : Ok
> perf: help.c:112: exclude_cmds: Assertion `cmds->names[ci] == NULL' \
> failed.
> Aborted ./perf test -F 68
>
> Output after:
> # ./perf test -F 68
> 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
>
> Fixes: 1fdf938168c4 ("perf tools: Fix use-after-free in help_unknown_cmd()")
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Ian Rogers <irogers@...gle.com>
> Signed-off-by: Thomas Richter <tmricht@...ux.ibm.com>
Thanks Thomas!
I tried to apply this on a perf-tools-next tree but it fails. Looking
into the git logs I see on linux-next:
https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/tools/lib/subcmd/help.c
the last patch is:
2025-09-12 perf subcmd: avoid crash in exclude_cmds when excludes is empty
In perf-tools-next:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/tools/lib/subcmd/help.c?h=tmp.perf-tools-next
I see:
8 days libsubcmd: Fix null intersection case in exclude_cmds()
2025-09-12 perf subcmd: avoid crash in exclude_cmds when excludes is empty
The test I wrote was to give coverage for Sri Jayaramappa's fix:
https://lore.kernel.org/r/20251202213632.2873731-1-sjayaram@akamai.com
I wonder if we've put the test into linux-next but not Sri's fix, well
that's what it looks like to me.
Now that we have both your fix and Sri's fix, and they differ :-) I'm
wondering how to resolve the differences.
Thanks,
Ian
> ---
> tools/lib/subcmd/help.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tools/lib/subcmd/help.c b/tools/lib/subcmd/help.c
> index ddaeb4eb3e24..1ce5fe507687 100644
> --- a/tools/lib/subcmd/help.c
> +++ b/tools/lib/subcmd/help.c
> @@ -93,19 +93,19 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
> zfree(&cmds->names[ci]);
> ci++;
> ei++;
> - } else if (cmp > 0) {
> + } else {
> ei++;
> }
> }
> - if (ci != cj) {
> + if (ci != cj) { /* Verify cmds list only if it changed */
> while (ci < cmds->cnt) {
> cmds->names[cj++] = cmds->names[ci];
> cmds->names[ci++] = NULL;
> }
> + for (ci = cj; ci < cmds->cnt; ci++)
> + assert(!cmds->names[ci]);
> + cmds->cnt = cj;
> }
> - for (ci = cj; ci < cmds->cnt; ci++)
> - assert(cmds->names[ci] == NULL);
> - cmds->cnt = cj;
> }
>
> static void get_term_dimensions(struct winsize *ws)
> --
> 2.52.0
>
Powered by blists - more mailing lists