Re: [PATCH v2 2/3] powerpc/perf: Use the aggregate context switch values from vcpu struct
From: Gautam Menghani <hidden>
Date: 2026-08-14 12:15:54
Also in:
kvm, sashiko-reviews
On Thu, Aug 13, 2026 at 09:58:50AM +0000, sashiko-bot@kernel.org wrote:
quoted hunk
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: New issues: - [High] Task-bound PMU events cache a stale vCPU pointer, resulting in tracking the wrong vCPU. - [High] CPU-wide and system-wide PMU events are broken and silently report 0 if added when no guest is running. - [High] Uninitialized event state on task migration leaks stale vCPU pointers. - [Medium] Unbalanced disablement of the global hardware PMU counters. Pre-existing issues: - [Critical] Dereferencing `kvm_hstate.kvm_vcpu` leads to a Use-After-Free if the previously running VM has been destroyed. ----- Patch [2]: [PATCH v2 2/3] powerpc/perf: Use the aggregate context switch values from vcpu struct ---commit e328e1f6c8baf0140410c1983f559e87d1d0efdc Author: Gautam Menghani [off-list ref] powerpc/perf: Use the aggregate context switch values from vcpu struct This commit aims to fix inaccurate vpa-pmu counter reporting during vCPU migrations and when vCPUs are not running by fetching aggregate values from the vcpu struct instead of relying on the host cpu's PACA.quoted
diff --git a/arch/powerpc/perf/vpa-pmu.c b/arch/powerpc/perf/vpa-pmu.c --- a/arch/powerpc/perf/vpa-pmu.c +++ b/arch/powerpc/perf/vpa-pmu.c[ ... ]quoted
@@ -99,19 +99,19 @@ static unsigned long get_counter_data(struct perf_event *event) switch (config) { case L1_TO_L2_CS_LAT: if (event->attach_state & PERF_ATTACH_TASK) - data = kvmhv_get_l1_to_l2_cs_time_vcpu(); + data = vcpu->arch.l1_to_l2_cs;[Severity: Critical] This is a pre-existing issue, but does dereferencing the vcpu pointer here lead to a use-after-free if the previously running VM has been destroyed? When KVM exits a guest, it does not clear local_paca->kvm_hstate.kvm_vcpu, leaving a dangling pointer in the host's PACA. The PMU driver reads this dangling pointer in vpa_pmu_add() and permanently caches it in event->pmu_private. Subsequent calls to vpa_pmu_read() pass this cached pointer to get_counter_data(), which dereferences the freed memory here.quoted
else data = kvmhv_get_l1_to_l2_cs_time(); break;[ ... ]quoted
@@ -126,21 +126,33 @@ static int vpa_pmu_add(struct perf_event *event, int flags) { u64 data; + struct kvm_vcpu *vcpu; + vcpu = local_paca->kvm_hstate.kvm_vcpu; + if (!vcpu) + goto out;
A dangling pointer is stored, but will that cause a UAF? When tracing a particular tid/pid, the perf stat command exits if the pid (guest) is destroyed.
[Severity: High] Does this early exit break CPU-wide and system-wide profiling? If a user initiates a CPU-wide profiling session (e.g. perf stat -a) and no guest is actively running on that specific CPU at that moment, vcpu is NULL. The function exits early returning 0 without initializing event->pmu_private or event->hw.prev_count. Subsequent calls to vpa_pmu_read() check if (!vcpu) and immediately return, silently leaving the event counter permanently at 0.
The vpa-pmu driver is only meant to report context switch times of individual vCPUs. So reporting 0 when a vCPU is not running is expected behaviour.
[Severity: High] Could this early exit also leak state from a previous CPU when a task migrates? When a task with a vpa_pmu event migrates to a new CPU, perf core calls pmu->del on the old CPU and pmu->add on the new CPU. If vcpu is NULL on the new CPU, it returns 0 (success) but jumps to out without updating or clearing event->pmu_private. The event silently retains the old vcpu pointer and prev_count from the previous CPU, resulting in corrupted profiling data calculated against an unrelated CPU's context.
Yes, valid point.
[Severity: Medium] Are the global hardware PMU counters skipped here on early bailout, leading to an unbalanced disablement later? If a user creates a PMU event on a CPU where no vCPU is active, this early exit skips calling kvmhv_set_l2_counters_status(cpu, true). However, when the event is deleted or scheduled out, vpa_pmu_del() unconditionally calls kvmhv_set_l2_counters_status(cpu, false). This unbalanced behavior forcefully disables the hardware counters even when they were never enabled, which could break legitimate concurrent KVM tracing sessions.
Valid find - in vpa_pmu_del() the counters should be disabled only if they were enabled in vpa_pmu_add()
quoted
+ + event->pmu_private = vcpu;[Severity: High] Is it possible caching the vcpu pointer here results in tracking the wrong vCPU for task-bound events? When a user creates a task-bound vpa_pmu perf event, the core perf subsystem calls vpa_pmu_add() during perf_event_context_sched_in(). At this point, the task has been scheduled but has not yet invoked the KVM_RUN ioctl. Because of this, local_paca->kvm_hstate.kvm_vcpu does not point to the incoming task's vCPU; it contains a stale pointer to the last vCPU that executed on this physical host CPU. Caching this pointer permanently means the event tracks unrelated data.
Valid find, will fix
quoted
kvmhv_set_l2_counters_status(smp_processor_id(), true); - data = get_counter_data(event); + data = get_counter_data(vcpu, event); local64_set(&event->hw.prev_count, data); +out: return 0; }-- Sashiko AI review · https://sashiko.dev/#/patchset/20260813094535.10083-1-gautam@linux.ibm.com?part=2