Re: [PATCH] perf tools: Fix segfault accessing sample_id xyarray

[Arnaldo Carvalho de Melo]

Em Wed, Apr 13, 2022 at 02:11:41PM -0700, Ian Rogers escreveu:
> On Wed, Apr 13, 2022 at 4:42 AM Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
> >
> > perf_evsel sample_id is an xyarray which can cause a segfault when
> > accessed beyond its size. e.g.
> >
> >  # perf record -e intel_pt// -C 1 sleep 1
> >  Segmentation fault (core dumped)
> >
> > That is happening because a dummy event is opened to capture text poke
> > events accoss all CPUs, however the mmap logic is allocating according
> > to the number of user_requested_cpus.
> 
> Nit: typo on 'accoss'

Fixed
 
> > In general, perf sometimes uses the evsel cpus to open events, and
> > sometimes the evlist user_requested_cpus. However, it is not necessary
> > to determine which case is which because the opened event file
> > descriptors are also in an xyarray, the size of whch can be used
> > to correctly allocate the size of the sample_id xyarray, because there
> > is one ID per file descriptor. Note, in the affected code path,
> > perf_evsel fd array is subsequently used to get the file descriptor for
> > the mmap, so it makes sense for the xyarrays to be the same size there.
> >
> > Fixes: 246eba8e9041c4 ("perf tools: Add support for PERF_RECORD_TEXT_POKE")
> > Fixes: d1a177595b3a82 ("libperf: Adopt perf_evlist__mmap()/munmap() from tools/perf")
> > Cc: stable@xxxxxxxxxxxxxxx # 5.5+
> > Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> 
> Acked-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks, applied.

- Arnaldo

 
> Thanks,
> Ian
> 
> > ---
> >  tools/lib/perf/evlist.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> > index 1b15ba13c477..a09315538a30 100644
> > --- a/tools/lib/perf/evlist.c
> > +++ b/tools/lib/perf/evlist.c
> > @@ -577,7 +577,6 @@ int perf_evlist__mmap_ops(struct perf_evlist *evlist,
> >  {
> >         struct perf_evsel *evsel;
> >         const struct perf_cpu_map *cpus = evlist->user_requested_cpus;
> > -       const struct perf_thread_map *threads = evlist->threads;
> >
> >         if (!ops || !ops->get || !ops->mmap)
> >                 return -EINVAL;
> > @@ -589,7 +588,7 @@ int perf_evlist__mmap_ops(struct perf_evlist *evlist,
> >         perf_evlist__for_each_entry(evlist, evsel) {
> >                 if ((evsel->attr.read_format & PERF_FORMAT_ID) &&
> >                     evsel->sample_id == NULL &&
> > -                   perf_evsel__alloc_id(evsel, perf_cpu_map__nr(cpus), threads->nr) < 0)
> > +                   perf_evsel__alloc_id(evsel, evsel->fd->max_x, evsel->fd->max_y) < 0)
> >                         return -ENOMEM;
> >         }
> >
> > --
> > 2.25.1
> >

-- 

- Arnaldo