diff mbox series

[v3,1/2] RISC-V: KVM: Fix array out-of-bounds in pmu_ctr_read() and pmu_fw_ctr_read_hi()

Message ID 20260310055557.835531-2-xujiakai2025@iscas.ac.cn
State Superseded
Headers show
Series RISC-V: KVM: Fix array out-of-bounds in firmware counter reads | expand

Commit Message

Jiakai Xu March 10, 2026, 5:55 a.m. UTC 
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 <xujiakai2025@iscas.ac.cn>
Signed-off-by: Jiakai Xu <jiakaiPeanut@gmail.com>
---
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(+)

Comments

Andrew Jones March 11, 2026, 4:52 p.m. UTC | #1 
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 <xujiakai2025@iscas.ac.cn>
> Signed-off-by: Jiakai Xu <jiakaiPeanut@gmail.com>
> ---
> 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 <andrew.jones@oss.qualcomm.com>
Jiakai Xu March 12, 2026, 1:20 a.m. UTC | #2 
Hi drew,

Thanks for the review and suggestion. 
I'll fix the alignment and send v4 shortly.

Best,
Jiakai

&quot;Andrew Jones&quot; &lt;andrew.jones@oss.qualcomm.com&gt;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 <xujiakai2025@iscas.ac.cn>
> > Signed-off-by: Jiakai Xu <jiakaiPeanut@gmail.com>
> > ---
> > 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 <andrew.jones@oss.qualcomm.com>
diff mbox series

Patch

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))
+		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);