[PATCH RFC 01/10] KVM: x86/pmu: Acquire SRCU in pmc_is_event_allowed() to protect filter lookup

[Marco Elver]

Dereferencing kvm->arch.pmu_event_filter via srcu_dereference() requires
holding kvm->srcu to guard against concurrent filter replacement and
freeing by kvm_vm_ioctl_set_pmu_event_filter().

Counter reprogramming can reach pmc_is_event_allowed() without holding
kvm->srcu. Specifically, on AMD SVM, toggling EFER.SVME via KVM_SET_SREGS
or KVM_SET_SREGS2 triggers synchronous counter reprogramming outside of
any SRCU read-side critical section:

  kvm_vcpu_ioctl(KVM_SET_SREGS{,2})
    kvm_vcpu_ioctl_x86_set_sregs{,2}()
      __set_sregs_common()
        kvm_x86_call(set_efer)()
          svm_set_efer()
            svm_pmu_handle_nested_transition()
              __svm_pmu_handle_nested_transition(..., defer=false)
                __kvm_pmu_reprogram_counters()
                  kvm_pmu_handle_event()
                    reprogram_counter()
                      pmc_is_event_allowed()
                        srcu_dereference(kvm->arch.pmu_event_filter, &kvm->srcu)

If userspace concurrently updates the filter (KVM_SET_PMU_EVENT_FILTER),
a concurrent free and subsequent use-after-free is possible.

Protect filter lookups directly in pmc_is_event_allowed():
1. check rcu_access_pointer() first for the common fast path;
2. acquire guard(srcu)(&kvm->srcu) only when a filter is present;
3. drop redundant outer srcu_read_lock() in kvm_pmu_trigger_event().

Found with Clang context analysis.

Fixes: a02a25a65246 ("KVM: x86/pmu: Reprogram Host/Guest-Only counters on nested transitions")
Signed-off-by: Marco Elver <elver@xxxxxxxxxx>
---
 arch/x86/kvm/pmu.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index a7d60c8785cd..3ad1e696edca 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -536,6 +536,11 @@ static bool pmc_is_event_allowed(struct kvm_pmc *pmc)
 	struct kvm_x86_pmu_event_filter *filter;
 	struct kvm *kvm = pmc->vcpu->kvm;
 
+	if (!rcu_access_pointer(kvm->arch.pmu_event_filter))
+		return true;
+
+	guard(srcu)(&kvm->srcu);
+
 	filter = srcu_dereference(kvm->arch.pmu_event_filter, &kvm->srcu);
 	if (!filter)
 		return true;
@@ -1132,7 +1137,7 @@ static void kvm_pmu_trigger_event(struct kvm_vcpu *vcpu,
 	DECLARE_BITMAP(bitmap, X86_PMC_IDX_MAX);
 	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
 	struct kvm_pmc *pmc;
-	int i, idx;
+	int i;
 
 	BUILD_BUG_ON(sizeof(pmu->global_ctrl) * BITS_PER_BYTE != X86_PMC_IDX_MAX);
 
@@ -1145,14 +1150,12 @@ static void kvm_pmu_trigger_event(struct kvm_vcpu *vcpu,
 			     (unsigned long *)&pmu->global_ctrl, X86_PMC_IDX_MAX))
 		return;
 
-	idx = srcu_read_lock(&vcpu->kvm->srcu);
 	kvm_for_each_pmc(pmu, pmc, i, bitmap) {
 		if (!pmc_is_event_allowed(pmc) || !cpl_is_matched(pmc))
 			continue;
 
 		kvm_pmu_incr_counter(pmc);
 	}
-	srcu_read_unlock(&vcpu->kvm->srcu, idx);
 }
 
 void kvm_pmu_instruction_retired(struct kvm_vcpu *vcpu)
-- 
2.55.0.1003.g10538fe699-goog