| 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
| ||
|
Message-ID: <aWahrUD2yzJSMyR-@x1>
Date: Tue, 13 Jan 2026 16:49:01 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Sri Jayaramappa <sjayaram@...mai.com>,
Guilherme Amadio <amadio@...too.org>,
Namhyung Kim <namhyung@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
Joshua Hunt <johunt@...mai.com>
Subject: Re: [PATCH] libsubcmd: Fix null intersection case in exclude_cmds()
On Mon, Dec 08, 2025 at 09:26:49AM -0800, Ian Rogers wrote:
> On Sun, Dec 7, 2025 at 2:16 PM Ian Rogers <irogers@...gle.com> wrote:
> >
> > On Tue, Dec 2, 2025 at 1:40 PM Sri Jayaramappa <sjayaram@...mai.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@...mai.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@...gle.com>
>
> I posted the test as a patch here:
> https://lore.kernel.org/lkml/20251208172339.1445817-1-irogers@google.com/
So, finally this one is also applied, together with Guilherme's
Tested-by/Reviewed-by tags, etc.
- Arnaldo
Powered by blists - more mailing lists