Re: [PATCH 3/3] perf tools: Fix use-after-free in help_unknown_cmd()
From: Namhyung Kim
Date: Tue Jul 01 2025 - 15:57:55 EST
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@xxxxxxxxxx> 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@xxxxxxxxxx>
>
> Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>
>
> 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