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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAP-5=fWjzR0WqD7RyDE66ChUQnt4_qwauEPiDsrhtL02u_zo4A@mail.gmail.com>
Date: Wed, 21 Jan 2026 08:14:22 -0800
From: Ian Rogers <irogers@...gle.com>
To: Thomas Richter <tmricht@...ux.ibm.com>, "Jayaramappa, Srilakshmi" <sjayaram@...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 8:12 AM Ian Rogers <irogers@...gle.com> wrote:
>
> 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.

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
> >