| Message ID | 20260226-kvm-riscv-spectre-v1-v1-4-5f930ea16691@cispa.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | KVM: riscv: Fix Spectre-v1 vulnerabilities in register access | expand |
2026-02-26T15:19:01+01:00, Lukas Gerlach <lukas.gerlach@cispa.de>: > 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@cispa.de> > --- > 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@oss.qualcomm.com> 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;
Thanks for the review! > 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. Nice catch, thanks for sending the fix. > I noticed a few other places where mis-speculation is possible, > see below; can you explain why they don't need protection? They do, I missed those. Will send a v2 incorporating all four sites. Thanks, Lukas
diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c index 4d8d5e9aa53d..fd891750c31f 100644 --- a/arch/riscv/kvm/vcpu_pmu.c +++ b/arch/riscv/kvm/vcpu_pmu.c @@ -10,6 +10,7 @@ #include <linux/errno.h> #include <linux/err.h> #include <linux/kvm_host.h> +#include <linux/nospec.h> #include <linux/perf/riscv_pmu.h> #include <asm/csr.h> #include <asm/kvm_vcpu_sbi.h> @@ -218,6 +219,7 @@ static int pmu_fw_ctr_read_hi(struct kvm_vcpu *vcpu, unsigned long cidx, return -EINVAL; } + cidx = array_index_nospec(cidx, RISCV_KVM_MAX_COUNTERS); pmc = &kvpmu->pmc[cidx]; if (pmc->cinfo.type != SBI_PMU_CTR_TYPE_FW) @@ -244,6 +246,7 @@ static int pmu_ctr_read(struct kvm_vcpu *vcpu, unsigned long cidx, return -EINVAL; } + cidx = array_index_nospec(cidx, RISCV_KVM_MAX_COUNTERS); pmc = &kvpmu->pmc[cidx]; if (pmc->cinfo.type == SBI_PMU_CTR_TYPE_FW) { @@ -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); retdata->out_val = kvpmu->pmc[cidx].cinfo.value; return 0;
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@cispa.de> --- arch/riscv/kvm/vcpu_pmu.c | 4 ++++ 1 file changed, 4 insertions(+)