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]>
