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

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

On 9/28/2026 4:04 PM, sashiko-bot@kernel.org wrote:
> 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?

Yes, it's a real risk. But it's hard to implement the check for last
software event in current framework. If the last event in the group is a
software event, then intel_pmu_hw_config() won't be involved to validate
the software event.

I suppose this could be just a corner case, we may leave this as a known
issue. Thanks.



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