Hi drew, Thanks for the review and suggestion. I'll fix the alignment and send v4 shortly.
Best, Jiakai "Andrew Jones" <[email protected]>wrote: > On Tue, Mar 10, 2026 at 05:55:56AM +0000, Jiakai Xu wrote: > > When a guest invokes SBI_EXT_PMU_COUNTER_FW_READ or > > SBI_EXT_PMU_COUNTER_FW_READ_HI on a firmware counter that has not been > > configured via SBI_EXT_PMU_COUNTER_CFG_MATCH, the pmc->event_idx remains > > SBI_PMU_EVENT_IDX_INVALID (0xFFFFFFFF). get_event_code() extracts the > > lower 16 bits, yielding 0xFFFF (65535), which is then used to index into > > kvpmu->fw_event[]. Since fw_event is only RISCV_KVM_MAX_FW_CTRS (32) > > entries, this triggers an array-index-out-of-bounds: > > > > UBSAN: array-index-out-of-bounds in arch/riscv/kvm/vcpu_pmu.c:255:37 > > index 65535 is out of range for type 'kvm_fw_event [32]' > > > > Add a check for the known unconfigured case (SBI_PMU_EVENT_IDX_INVALID) > > and a WARN_ONCE guard for any unexpected out-of-bounds event codes, > > returning -EINVAL in both cases. > > > > Fixes: badc386869e2c ("RISC-V: KVM: Support firmware events") > > Fixes: 08fb07d6dcf71 ("RISC-V: KVM: Support 64 bit firmware counters on > > RV32") > > Signed-off-by: Jiakai Xu <[email protected]> > > Signed-off-by: Jiakai Xu <[email protected]> > > --- > > V2 -> V3: > > - Added check for SBI_PMU_EVENT_IDX_INVALID. > > - Added WARN_ONCE for unexpected out-of-bounds event codes. > > V1 -> V2: > > - Merged the fixes for pmu_ctr_read() and pmu_fw_ctr_read_hi() into a single > > commit. > > - Removed the pr_warn, simply returning -EINVAL instead. > > --- > > arch/riscv/kvm/vcpu_pmu.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c > > index e873430e596b..c87a7b0037cf 100644 > > --- a/arch/riscv/kvm/vcpu_pmu.c > > +++ b/arch/riscv/kvm/vcpu_pmu.c > > @@ -226,7 +226,14 @@ static int pmu_fw_ctr_read_hi(struct kvm_vcpu *vcpu, > > unsigned long cidx, > > if (pmc->cinfo.type != SBI_PMU_CTR_TYPE_FW) > > return -EINVAL; > > > > + if (pmc->event_idx == SBI_PMU_EVENT_IDX_INVALID) > > + return -EINVAL; > > + > > fevent_code = get_event_code(pmc->event_idx); > > + if (WARN_ONCE(fevent_code >= SBI_PMU_FW_MAX, > > + "Invalid firmware event code: %d\n", > > fevent_code)) > > Align under previous parameter, not the >= operator > > if (WARN_ONCE(fevent_code >= SBI_PMU_FW_MAX, > "Invalid firmware event code: %d\n", fevent_code)) > > > + return -EINVAL; > > + > > pmc->counter_val = kvpmu->fw_event[fevent_code].value; > > > > *out_val = pmc->counter_val >> 32; > > @@ -251,7 +258,14 @@ static int pmu_ctr_read(struct kvm_vcpu *vcpu, > > unsigned long cidx, > > pmc = &kvpmu->pmc[cidx]; > > > > if (pmc->cinfo.type == SBI_PMU_CTR_TYPE_FW) { > > + if (pmc->event_idx == SBI_PMU_EVENT_IDX_INVALID) > > + return -EINVAL; > > + > > fevent_code = get_event_code(pmc->event_idx); > > + if (WARN_ONCE(fevent_code >= SBI_PMU_FW_MAX, > > + "Invalid firmware event code: %d\n", > > fevent_code)) > > + return -EINVAL; > > + > > pmc->counter_val = kvpmu->fw_event[fevent_code].value; > > } else if (pmc->perf_event) { > > pmc->counter_val += perf_event_read_value(pmc->perf_event, > > &enabled, &running); > > -- > > 2.34.1 > > > > Otherwise, > > Reviewed-by: Andrew Jones <[email protected]>
