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

[Ian Rogers]

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'

> 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,
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
>