Re: [PATCH 07/15] perf/x86/intel: Reject SAMPLE_READ for no-counter-snapshot ACR events

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf/x86: Validation bypass for SAMPLE_READ on ACR events via software events
--

commit cc3bcd9a57c859b5993ae9c14bd06faec36a3c33
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>

perf/x86/intel: Reject SAMPLE_READ for no-counter-snapshot ACR events

This commit restricts the use of PERF_SAMPLE_READ for Auto Counter Reload (ACR) 
events to prevent perf from reporting invalid counts. It ensures that 
SAMPLE_READ is only permitted for PEBS-backed ACR events with counter snapshot 
support, as non-PEBS events can be auto-reloaded before software reads them.

> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 377ff39124206..3fc3795534bf7 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c

[ ... ]

> @@ -5184,6 +5239,10 @@ static int intel_pmu_hw_config(struct perf_event *event)
>  		if (hweight64(cause_mask) > hweight64(hybrid(pmu, acr_cause_mask64)) ||
>  		    num > hweight64(hybrid(event->pmu, acr_cntr_mask64)))
>  			return -EINVAL;
> +
> +		if (!intel_pmu_allow_acr_sample_read(event, has_sample_read))
> +			return -EINVAL;
> +

[Severity: Medium]
Does this validation fully prevent PERF_SAMPLE_READ from being applied to ACR
groups?

Because this check is inside intel_pmu_hw_config(), it is evaluated when the
x86 event is initialized. If a user creates an ACR hardware event group
initially without PERF_SAMPLE_READ, it will successfully pass this check.

What happens if the user subsequently creates a Software PMU event (like a
dummy tracepoint) with PERF_SAMPLE_READ set, and attaches it to the existing
ACR hardware group using the leader's group_fd?

Since perf_event_open() allows software events to be attached to hardware
groups without re-invoking the hardware PMU's validation callbacks for
existing members, it appears intel_pmu_hw_config() won't be re-evaluated
for the hardware events. 

When the appended software event overflows, won't perf_output_read_group()
read all siblings and still capture the imprecise auto-reloaded values of the
ACR hardware counters?

>  		/*
>  		 * In the second round, apply the counter-constraints for
>  		 * the events which can cause other events reload.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928074309.898043-1-dapeng1.mi@linux.intel.com?part=7