[PATCH v2] libperf: Handle an evsel without an fd array in enable/disable

[TAKANO Mitsuhiro <takano32@gmail.com>]

A group led by a tool PMU event is opened with the members' group_fd set
to the leader's file descriptor.  For a tool event that descriptor comes
from open("/proc/stat"), not from perf_event_open(), so a member the
kernel refuses is marked unsupported; perf stat then releases the fd
array of that member while the evsel stays in the group.

Enabling the group goes through evsel__enable_cpu(), which enables the
members of a mixed PMU group from userspace because the kernel does not
do it along with the leader.  The member has no fd array left, so
perf_evsel__run_ioctl() reads its loop bound through xyarray__max_y()
and dereferences a NULL pointer:

  $ perf stat -a -e '{user_time,cycles}' -- sleep 0.05
  Segmentation fault

Guard the array in both places: FD() returns NULL when it is absent,
which perf_evsel__ioctl() and perf_evsel__read() already test for, and
xyarray__max_x()/max_y() return 0, so the enable and disable paths that
walk the array become no-ops; perf_evsel__ioctl() returns -1 for such an
evsel, as it already does for a closed fd.  The FD() guard alone is not
enough because that loop bound is read first; measured on the same tree,
rc=139 with the FD() change only and rc=0 with both.

Verified on an arm64 KVM guest (Neoverse-N1) with a perf built from this
tree.  '{user_time,cycles}' and '{system_time,cycles}' segfault on the
unpatched tree (rc=139) and exit 0 here, reporting user_time (resp.
system_time) and a '<not supported> cycles:u' line.  Real-PMU groups
such as '{cycles,instructions}', 'perf stat' with no event and '-r 2'
print identical output on both trees, and 'perf test -v 10' passes the
same 78 subtests.  The libperf tests report the same five test-evlist
failures on both trees: the events cannot be opened by this user
(perf_event_paranoid=4), and two cases cannot read the syscall tracepoint
id because /sys/kernel/debug is mounted root-only.

The events are still not counted: a member the kernel refused stays
unsupported.  Hang Zhengyang's follow-up opens these members with
group_fd = -1 so that they are counted.

Written with Hermes Agent, an LLM assistant, which made the change and
wrote this changelog.  I built perf with it, reproduced the segfault on
the unpatched tree and confirmed it is gone with the patch, and ran
'perf test' and the libperf tests on this box.

Fixes: 0b18bced5444 ("perf tool_pmu: Make tool PMU events respect enable/disable")
Link: https://lore.kernel.org/avQIOtt7Y67oXQem@z2
Suggested-by: Namhyung Kim <namhyung@kernel.org>
Cc: stable@vger.kernel.org
Signed-off-by: TAKANO Mitsuhiro <takano32@gmail.com>
---
v2: follow Namhyung's suggestion and fix it in libperf instead of
    skipping the members in tools/perf/util/evsel.c. The FD() guard
    alone leaves the crash in place, because the loop bound is read
    from the same NULL array through xyarray__max_y(), so the
    xyarray accessors get a NULL check as well. Measured on the same
    box with the FD() guard only: rc=139, with both: rc=0.

 tools/lib/perf/evsel.c                    | 3 ++-
 tools/lib/perf/include/internal/xyarray.h | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index f747c0bc692d..e23df3b2c1f1 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -57,7 +57,8 @@ void perf_evsel__delete(struct perf_evsel *evsel)
 }
 
 #define FD(_evsel, _cpu_map_idx, _thread)				\
-	((int *)xyarray__entry(_evsel->fd, _cpu_map_idx, _thread))
+	(_evsel->fd ? (int *)xyarray__entry(_evsel->fd, _cpu_map_idx, _thread) \
+		     : NULL)
 #define MMAP(_evsel, _cpu_map_idx, _thread)				\
 	(_evsel->mmap ? ((struct perf_mmap *) xyarray__entry(_evsel->mmap, _cpu_map_idx, _thread)) \
 		      : NULL)
diff --git a/tools/lib/perf/include/internal/xyarray.h b/tools/lib/perf/include/internal/xyarray.h
index f10af3da7b21..8bf76c12984e 100644
--- a/tools/lib/perf/include/internal/xyarray.h
+++ b/tools/lib/perf/include/internal/xyarray.h
@@ -32,12 +32,12 @@ static inline void *xyarray__entry(struct xyarray *xy, size_t x, size_t y)
 
 static inline int xyarray__max_y(struct xyarray *xy)
 {
-	return xy->max_y;
+	return xy ? xy->max_y : 0;
 }
 
 static inline int xyarray__max_x(struct xyarray *xy)
 {
-	return xy->max_x;
+	return xy ? xy->max_x : 0;
 }
 
 #endif /* __LIBPERF_INTERNAL_XYARRAY_H */

base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
-- 
2.53.0