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

From: Arnaldo Carvalho de Melo
Date: Wed Apr 13 2022 - 21:23:42 EST


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