All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Dapeng Mi" <dapeng1.mi@linux.intel.com>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 1/2] perf/x86/intel: Clear stale ACR mask before updating new mask
Date: Mon, 13 Apr 2026 01:35:57 +0000	[thread overview]
Message-ID: <20260413013558.0EFE3C19424@smtp.kernel.org> (raw)
In-Reply-To: <20260413010157.535990-2-dapeng1.mi@linux.intel.com>

Sashiko AI review found 2 potential issue(s):
- [High] Incomplete Fix: Early return bypasses the newly added stale mask cleanup for subsequent events.
- [High] Missing bounds check on user-provided mask allows cross-event PMU interference (cross-wiring).
--

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

perf/x86/intel: Clear stale ACR mask before updating new mask

This commit clears the ACR mask before applying a new one to prevent
stale bits from persisting during event rescheduling. It ensures the
hardware state does not get polluted by previous configurations.

> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 4768236c054bb..58c236ce47474 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -3344,6 +3344,9 @@ static void intel_pmu_acr_late_setup(struct cpu_hw_events *cpuc)
>  			event = cpuc->event_list[j];
>  			if (event->group_leader != leader->group_leader)
>  				break;
> +
> +			/* Clear stale ACR mask first. */
> +			event->hw.config1 = 0;
>  			for_each_set_bit(idx, (unsigned long *)&event->attr.config2, X86_PMC_IDX_MAX) {
>  				if (i + idx >= cpuc->n_events ||
>  				    !is_acr_event_group(cpuc->event_list[i + idx]))

Could this validation check allow unprivileged users to cross-wire the PMU?

Since is_acr_event_group() only verifies that the target event belongs to any
ACR group, rather than the same group_leader, does this allow a user-provided
out-of-bounds bit in config2 to point to an independent, system-wide ACR event
running on the same CPU?

If so, it seems the hardware counter index of an unrelated privileged event
could be blindly copied into the user's hw.config1.

>  					return;

Does this early return bypass the newly added stale mask cleanup for subsequent
events?

Because attr.config2 is user-controlled, a user could supply a malformed mask
with an out-of-bounds bit to intentionally trigger this early return.

Since it returns from intel_pmu_acr_late_setup() completely, wouldn't this skip
the event->hw.config1 = 0 cleanup for all subsequent ACR events scheduled on the
same CPU, leaving them with a polluted hardware state?

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

  reply	other threads:[~2026-04-13  1:35 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-13  1:01 [PATCH 0/2] Fix 2 bugs for Intel auto counter reload (ACR) Dapeng Mi
2026-04-13  1:01 ` [PATCH 1/2] perf/x86/intel: Clear stale ACR mask before updating new mask Dapeng Mi
2026-04-13  1:35   ` sashiko-bot [this message]
2026-04-14  2:57     ` Mi, Dapeng
2026-04-13  1:01 ` [PATCH 2/2] perf/x86/intel: Disable PMI for self-reloaded ACR events Dapeng Mi
2026-04-13  1:19 ` [PATCH 0/2] Fix 2 bugs for Intel auto counter reload (ACR) Mi, Dapeng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260413013558.0EFE3C19424@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dapeng1.mi@linux.intel.com \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sashiko@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.