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

Powered by Openwall GNU/*/Linux Powered by OpenVZ