Re: [PATCHv5] perf tool: fix dereferencing NULL al->maps

[Casey Chen]

On Wed, Jul 24, 2024 at 9:19 AM Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
>
> Hello,
>
> On Tue, Jul 23, 2024 at 6:01 PM Casey Chen <cachen@xxxxxxxxxxxxxxx> wrote:
> >
> > Ian / Namhyung,
> >
> > Could you take a look at the latest diff PATCHv5 ?
> >
> > Thanks,
> > Casey
> >
> > On Mon, Jul 22, 2024 at 2:15 PM Casey Chen <cachen@xxxxxxxxxxxxxxx> wrote:
> > >
> > > With 0dd5041c9a0e ("perf addr_location: Add init/exit/copy functions"),
> > > when cpumode is 3 (macro PERF_RECORD_MISC_HYPERVISOR),
> > > thread__find_map() could return with al->maps being NULL.
> > >
> > > The path below could add a callchain_cursor_node with NULL ms.maps.
> > >
> > > add_callchain_ip()
> > >   thread__find_symbol(.., &al)
> > >     thread__find_map(.., &al)   // al->maps becomes NULL
> > >   ms.maps = maps__get(al.maps)
> > >   callchain_cursor_append(..., &ms, ...)
> > >     node->ms.maps = maps__get(ms->maps)
> > >
> > > Then the path below would dereference NULL maps and get segfault.
> > >
> > > fill_callchain_info()
> > >   maps__machine(node->ms.maps);
> > >
> > > Fix it by checking if maps is NULL in fill_callchain_info().
> > >
> > > Signed-off-by: Casey Chen <cachen@xxxxxxxxxxxxxxx>
> > > Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>
>
> Acked-by: Namhyung Kim <namhyung@xxxxxxxxxx>
>
> Thanks,
> Namhyung
>
>
> > > ---
> > >  tools/perf/util/callchain.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> > > index 1730b852a947..6d075648d2cc 100644
> > > --- a/tools/perf/util/callchain.c
> > > +++ b/tools/perf/util/callchain.c
> > > @@ -1141,7 +1141,7 @@ int hist_entry__append_callchain(struct hist_entry *he, struct perf_sample *samp
> > >  int fill_callchain_info(struct addr_location *al, struct callchain_cursor_node *node,
> > >                         bool hide_unresolved)
> > >  {
> > > -       struct machine *machine = maps__machine(node->ms.maps);
> > > +       struct machine *machine = node->ms.maps ? maps__machine(node->ms.maps) : NULL;
> > >
> > >         maps__put(al->maps);
> > >         al->maps = maps__get(node->ms.maps);
> > > --
> > > 2.45.2
> > >

Thanks Namhyung.
I have another question. When will this patch get merged into master
branch or 6.6 release line ? Our benchmark systems depend on this fix
to do performance analysis. Currently, both our kernel and perf are on
6.6.9 and they build separately. We want to update perf hash without
patching it locally.

Casey