Re: [PATCH] perf/riscv: Guard PMU counter indices and user access

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] perf/riscv: zero counter count in pmu_sbi_get_ctrinfo() derefs ZERO_SIZE_PTR
--

commit 8efed03034c08f98dfc2b34e375ed58087ac3e08
Author: Zhao Gongyi <zhaogongyi@bytedance.com>

perf/riscv: Guard PMU counter indices and user access

This commit adds bounds validation on PMU counter indices before accessing
counter state or CSR mappings, preventing out-of-bounds accesses when a
disabled event is mmaped before hw.idx is assigned or when SBI returns
invalid counter values.

> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index 21a2078eab98a..ac0d43746b799 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
[ ... ]
> @@ -890,10 +917,18 @@ static int pmu_sbi_find_num_ctrs(void)
>  	struct sbiret ret;
>  
>  	ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_NUM_COUNTERS);
> -	if (!ret.error)
> -		return ret.value;
> -	else
> +	if (ret.error)
>  		return sbi_err_map_linux_errno(ret.error);
> +
> +	if (ret.value < 0)
> +		return -EINVAL;
> +	if (ret.value > RISCV_MAX_COUNTERS) {
> +		pr_info("SBI returned too many counters, limiting to %d\n",
> +			RISCV_MAX_COUNTERS);
> +		return RISCV_MAX_COUNTERS;
> +	}
> +
> +	return ret.value;
>  }
>  
>  static int pmu_sbi_get_ctrinfo(int nctr, unsigned long *mask)
> @@ -902,6 +937,9 @@ static int pmu_sbi_get_ctrinfo(int nctr, unsigned long *mask)
>  	int i, num_hw_ctr = 0, num_fw_ctr = 0;
>  	union sbi_pmu_ctr_info cinfo;
>  
> +	if (nctr < 0 || nctr > RISCV_MAX_COUNTERS)
> +		return -EINVAL;
> +
>  	pmu_ctr_list = kzalloc_objs(*pmu_ctr_list, nctr);
>  	if (!pmu_ctr_list)
>  		return -ENOMEM;

[Severity: High]
Should pmu_sbi_find_num_ctrs() and pmu_sbi_get_ctrinfo() reject zero
counters as well (ret.value <= 0 and nctr <= 0)?

If SBI_EXT_PMU_NUM_COUNTERS returns 0, pmu_sbi_find_num_ctrs() returns 0,
which passes the num_counters < 0 check in pmu_sbi_device_probe():

drivers/perf/riscv_pmu_sbi.c:pmu_sbi_device_probe() {
	num_counters = pmu_sbi_find_num_ctrs();
	if (num_counters < 0) {
		pr_err("SBI PMU extension doesn't provide any counters\n");
		goto out_free;
	}

	/* cache all the information about counters now */
	if (pmu_sbi_get_ctrinfo(num_counters, cmask))
		goto out_free;
}

In pmu_sbi_get_ctrinfo(), kzalloc_objs(*pmu_ctr_list, 0) returns
ZERO_SIZE_PTR, which is non-NULL, so the PMU is registered with
pmu_ctr_list set to ZERO_SIZE_PTR and an empty cmask.

When userspace later opens a non-sampling event (such as a PERF_TYPE_RAW
event), riscv_pmu_event_init() sets hwc->idx = -1 and calls
riscv_pmu_ctr_get_width_mask(), which falls back to counter index 0 and
dereferences ZERO_SIZE_PTR in pmu_sbi_ctr_get_width():

riscv_pmu_event_init()
  riscv_pmu_ctr_get_width_mask()
    if (!riscv_pmu_ctr_idx_valid(hwc->idx))
      cwidth = rvpmu->ctr_get_width(0);
        pmu_sbi_ctr_get_width(0)
          return pmu_ctr_list[idx].width;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103093709.81605-1-zhaogongyi@bytedance.com?part=1