Re: [PATCH 3/3] perf tools: Fix use-after-free in help_unknown_cmd()

[Namhyung Kim <namhyung@kernel.org>]

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@kernel.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@kernel.org>
> 
> Reviewed-by: Ian Rogers <irogers@google.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