[PATCH V2 linux-next] perf test: Subtest Exclude disjoint subcmd names fails

Thomas Richter posted 1 patch 2 weeks, 2 days ago
tools/lib/subcmd/help.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
[PATCH V2 linux-next] perf test: Subtest Exclude disjoint subcmd names fails
Posted by Thomas Richter 2 weeks, 2 days ago
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@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
---
 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
Re: [PATCH V2 linux-next] perf test: Subtest Exclude disjoint subcmd names fails
Posted by Ian Rogers 2 weeks, 2 days ago
On Wed, Jan 21, 2026 at 12:24 AM Thomas Richter <tmricht@linux.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@kernel.org>
> Cc: Ian Rogers <irogers@google.com>
> Signed-off-by: Thomas Richter <tmricht@linux.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
>
Re: [PATCH V2 linux-next] perf test: Subtest Exclude disjoint subcmd names fails
Posted by Ian Rogers 2 weeks, 2 days ago
On Wed, Jan 21, 2026 at 8:12 AM Ian Rogers <irogers@google.com> wrote:
>
> On Wed, Jan 21, 2026 at 12:24 AM Thomas Richter <tmricht@linux.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@kernel.org>
> > Cc: Ian Rogers <irogers@google.com>
> > Signed-off-by: Thomas Richter <tmricht@linux.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.

Sorry, resending as I got Sri's email address wrong.
Ian

> 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
> >
Re: [PATCH V2 linux-next] perf test: Subtest Exclude disjoint subcmd names fails
Posted by Thomas Richter 2 weeks, 1 day ago
On 1/21/26 17:14, Ian Rogers wrote:
> On Wed, Jan 21, 2026 at 8:12 AM Ian Rogers <irogers@google.com> wrote:
>>
>> On Wed, Jan 21, 2026 at 12:24 AM Thomas Richter <tmricht@linux.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@kernel.org>
>>> Cc: Ian Rogers <irogers@google.com>
>>> Signed-off-by: Thomas Richter <tmricht@linux.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.
> 
> Sorry, resending as I got Sri's email address wrong.
> Ian
> 
Hi Ian, Jayaramappa,

I have looked at both patches and Jayaramappa's patch is already
in perf-tool-next and solves the issue too. I am fine with this one.
So lets simply drop mine.

Thanks Thomas
-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH

Vorsitzender des Aufsichtsrats: Wolfgang Wendt

Geschäftsführung: David Faller

Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294