Re: [PATCH v2 1/2] perf/core: out-of-line and export perf_allow_cpu/tracepoint()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The variable `v` used to store the event index is inadvertently overwritten by the return value of `perf_allow_cpu()`.
--

commit 4a1ba4712c8f6d5b5cbf8536fff7c0deb0a5c8b7
Author: John Hubbard <jhubbard@nvidia.com>

perf/core: out-of-line and export perf_allow_cpu/tracepoint()

This commit moves perf_allow_cpu() and perf_allow_tracepoint() out of
the perf_event.h header and into kernel/events/core.c. It exports them 
with EXPORT_SYMBOL_GPL to allow modular code to access these permissions 
checks, matching the behavior of perf_allow_kernel(). It also provides 
stubs for when CONFIG_PERF_EVENTS is disabled.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 6d1f8bad7e1c5..735e502beb966 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c

[ ... ]

> @@ -14691,6 +14691,24 @@ int perf_allow_kernel(void)
>  }
>  EXPORT_SYMBOL_GPL(perf_allow_kernel);
>  
> +int perf_allow_cpu(void)
> +{
> +	if (sysctl_perf_event_paranoid > 0 && !perfmon_capable())
> +		return -EACCES;
> +
> +	return security_perf_event_open(PERF_SECURITY_CPU);
> +}

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the usage
of perf_allow_cpu(), I noticed a pre-existing issue in the p4 PMU code.

Does this overwrite the event index v? Looking at p4_validate_raw_event()
in arch/x86/events/intel/p4.c, v is used to index into p4_event_bind_map,
but it appears to be overwritten by the return value of perf_allow_cpu():

arch/x86/events/intel/p4.c:p4_validate_raw_event() {
    ...
	if (p4_ht_active() && p4_event_bind_map[v].shared) {
		v = perf_allow_cpu();
		if (v)
			return v;
	}

	/* ESCR EventMask bits may be invalid */
	emask = p4_config_unpack_escr(event->attr.config) & P4_ESCR_EVENTMASK_MASK;
	if (emask & ~p4_event_bind_map[v].escr_emask)
		return -EINVAL;
    ...
}

If perf_allow_cpu() returns 0 for success, does this mean v becomes 0, and
the subsequent check uses p4_event_bind_map[0].escr_emask instead of the
intended event mask?

Could this allow users to bypass proper event mask checks and program invalid
hardware ESCR bits?

> +EXPORT_SYMBOL_GPL(perf_allow_cpu);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523013326.129491-1-jhubbard@nvidia.com?part=1