Re: [PATCH v3 08/19] perf evsel: Don't dump probe events to PERF_TEST_ATTR

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf: fd leak in evsel__open_cpu() on missing feature fallback
--

commit 4b54c2ca3d287fefa4e358bd15bc06b547bbe8da
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf evsel: Don't dump probe events to PERF_TEST_ATTR

This commit prevents support probe events from being dumped into the
$PERF_TEST_ATTR event files. It introduces a support_probe flag to identify
events opened strictly to query supported features.

Link: https://lore.kernel.org/linux-perf-users/asWQvXRdqp_RK85c@google.com/

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index e61f309e93dcc..59318fef03cb1 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -3104,7 +3104,11 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
>  
>  			bpf_counter__install_pe(evsel, idx, fd);
>  
> -			if (unlikely(test_attr__enabled())) {
> +			/*
> +			 * Support probes aren't what was asked for, leave them
> +			 * out of the PERF_TEST_ATTR dump.
> +			 */
> +			if (unlikely(test_attr__enabled() && !evsel->support_probe)) {

[Severity: Medium]
Does this code leak file descriptors when a missing feature fallback is
triggered on a subsequent loop iteration?

If sys_perf_event_open() succeeds on a capable CPU and records the file
descriptor, but then fails on an incapable CPU, the code jumps to restart
the loop:

evsel__open_cpu() {
    ...
fallback_missing_features:
    evsel__disable_missing_features(evsel);
    ...
    for (idx = start_cpu_map_idx; idx < end_cpu_map_idx; idx++) {
        ...
        for (thread = 0; thread < nthreads; thread++) {
            ...
            fd = sys_perf_event_open(...);
            FD(evsel, idx, thread) = fd;
            if (fd < 0) {
                err = -errno;
                goto try_fallback;
            }
            ...
try_fallback:
    ...
    if (err == -EINVAL && evsel__detect_missing_features(evsel, cpu))
        goto fallback_missing_features;
}

When evsel__detect_missing_features() triggers the jump back to
fallback_missing_features, it appears the loop restarts from
start_cpu_map_idx without routing through out_close to clean up the
previously opened file descriptors. Can this leak the file descriptors when
the array entries are overwritten?

>  				test_attr__open(&evsel->core.attr, pid, cpu,
>  						fd, group_fd, evsel->open_flags);
>  			}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016001843.71978-1-acme@kernel.org?part=8