Re: [PATCH 4/4] KVM: riscv: Fix Spectre-v1 in PMU counter access

[Radim Krčmář]

2026-02-26T15:19:01+01:00, Lukas Gerlach <lukas.gerlach@xxxxxxxx>:
> Guest-controlled counter indices received via SBI ecalls are used to
> index into the PMC array. Sanitize them with array_index_nospec()
> to prevent speculative out-of-bounds access.
>
> Similar to x86 commit 13c5183a4e64 ("KVM: x86: Protect MSR-based
> index computations in pmu.h from Spectre-v1/L1TF attacks").
>
> Fixes: 8f0153ecd3bf ("RISC-V: KVM: Add skeleton support for perf")
> Signed-off-by: Lukas Gerlach <lukas.gerlach@xxxxxxxx>
> ---
> diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c
> @@ -525,6 +528,7 @@ int kvm_riscv_vcpu_pmu_ctr_info(struct kvm_vcpu *vcpu, unsigned long cidx,
>  		return 0;
>  	}
>  
> +	cidx = array_index_nospec(cidx, RISCV_KVM_MAX_COUNTERS);

This one also covers a non-speculation bug, since the previous condition
used cidx > RISCV_KVM_MAX_COUNTER. :)  I'll send a patch for that.

I noticed a few other places where mis-speculation is possible,
see below; can you explain why they don't need protection?

Anyway, the series looks good,

Reviewed-by: Radim Krčmář <radim.krcmar@xxxxxxxxxxxxxxxx>

Thanks.


---
diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c
index 4d8d5e9aa53d..08301b6033f0 100644
--- a/arch/riscv/kvm/vcpu_pmu.c
+++ b/arch/riscv/kvm/vcpu_pmu.c
@@ -87,7 +87,7 @@ static void kvm_pmu_release_perf_event(struct kvm_pmc *pmc)
 
 static u64 kvm_pmu_get_perf_event_hw_config(u32 sbi_event_code)
 {
-	return hw_event_perf_map[sbi_event_code];
+	return hw_event_perf_map[array_index_nospec(sbi_event_code, SBI_PMU_HW_GENERAL_MAX)];
 }
 
 static u64 kvm_pmu_get_perf_event_cache_config(u32 sbi_event_code)
@@ -559,7 +559,7 @@ int kvm_riscv_vcpu_pmu_ctr_start(struct kvm_vcpu *vcpu, unsigned long ctr_base,
 	}
 	/* Start the counters that have been configured and requested by the guest */
 	for_each_set_bit(i, &ctr_mask, RISCV_MAX_COUNTERS) {
-		pmc_index = i + ctr_base;
+		pmc_index = array_index_nospec(i + ctr_base, RISCV_KVM_MAX_COUNTERS);
 		if (!test_bit(pmc_index, kvpmu->pmc_in_use))
 			continue;
 		/* The guest started the counter again. Reset the overflow status */
@@ -630,7 +630,7 @@ int kvm_riscv_vcpu_pmu_ctr_stop(struct kvm_vcpu *vcpu, unsigned long ctr_base,
 
 	/* Stop the counters that have been configured and requested by the guest */
 	for_each_set_bit(i, &ctr_mask, RISCV_MAX_COUNTERS) {
-		pmc_index = i + ctr_base;
+		pmc_index = array_index_nospec(i + ctr_base, RISCV_KVM_MAX_COUNTERS);
 		if (!test_bit(pmc_index, kvpmu->pmc_in_use))
 			continue;
 		pmc = &kvpmu->pmc[pmc_index];
@@ -761,6 +761,7 @@ int kvm_riscv_vcpu_pmu_ctr_cfg_match(struct kvm_vcpu *vcpu, unsigned long ctr_ba
 		}
 	}
 
+	ctr_idx = array_index_nospec(ctr_idx, RISCV_KVM_MAX_COUNTERS);
 	pmc = &kvpmu->pmc[ctr_idx];
 	pmc->idx = ctr_idx;