linux-kernel - Re: [PATCH 3/3] perf tools: Fix use-after-free in help_unknown

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: <aGQ9ucA3BH1OXzlf@google.com>
Date: Tue, 1 Jul 2025 12:57:45 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
	Kan Liang <kan.liang@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
	linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 3/3] perf tools: Fix use-after-free in help_unknown_cmd()

On Tue, Jul 01, 2025 at 09:03:46AM -0700, Ian Rogers wrote:
> On Mon, Jun 30, 2025 at 4:32 PM Namhyung Kim <namhyung@...nel.org> wrote:
> >
> > Currently perf aborts when it finds an invalid command.  I guess it
> > depends on the environment as I have some custom commands in the path.
> >
> >   $ perf bad-command
> >   perf: 'bad-command' is not a perf-command. See 'perf --help'.
> >   Aborted (core dumped)
> >
> > It's because the exclude_cmds() in libsubcmd has a use-after-free when
> > it removes some entries.  After copying one to another entry, it keeps
> > the pointer in the both position.  And the next copy operation will free
> > the later one but it's the same entry in the previous one.
> >
> > For example, let's say cmds = { A, B, C, D, E } and excludes = { B, E }.
> >
> >   ci  cj  ei   cmds-name  excludes
> >   -----------+--------------------
> >    0   0   0 |     A         B       :    cmp < 0, ci == cj
> >    1   1   0 |     B         B       :    cmp == 0
> >    2   1   1 |     C         E       :    cmp < 0, ci != cj
> >
> > At this point, it frees cmds->names[1] and cmds->names[1] is assigned to
> > cmds->names[2].
> >
> >    3   2   1 |     D         E       :    cmp < 0, ci != cj
> >
> > Now it frees cmds->names[2] but it's the same as cmds->names[1].  So
> > accessing cmds->names[1] will be invalid.
> >
> > This makes the subcmd tests succeed.
> >
> >   $ perf test subcmd
> >    69: libsubcmd help tests                                            :
> >    69.1: Load subcmd names                                             : Ok
> >    69.2: Uniquify subcmd names                                         : Ok
> >    69.3: Exclude duplicate subcmd names                                : Ok
> >
> > Fixes: 657a3efee43a ("libsubcmd: Avoid SEGV/use-after-free when commands aren't excluded")
> > Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> 
> Reviewed-by: Ian Rogers <irogers@...gle.com>
> 
> Fwiw, it seems a shame we're doing this and the code in git is drifting apart:
> 
> https://github.com/git/git/blob/83014dc05f6fc9275c0a02886cb428805abaf9e5/help.c#L204

Maybe we can send a fix for them too.

Thanks,
Namhyung