Re: [PATCH v6 16/21] RISC-V: perf: Add legacy event encodings via sysfs

[Atish Patra <atish.patra@linux.dev>]

On 6/8/26 11:21 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The globally scoped pmu_ctr_list array is leaked if PMU counter discovery fails, compounded by a flawed error code cast.
> --
>
> commit 7d64e724e6eff197f10e62cdd709e07f0a5c9a7d
> Author: Atish Patra <atishp@rivosinc.com>
>
> RISC-V: perf: Add legacy event encodings via sysfs
>
> Define sysfs details for the legacy events so that any tool can
> parse these to understand the minimum set of legacy events
> supported by the platform. The sysfs entry will describe both event
> encoding and corresponding counter map so that an perf event can be
> programmed accordingly.
>
>> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
>> index 4b4f151a0744d..00b84b28117a8 100644
>> --- a/drivers/perf/riscv_pmu_sbi.c
>> +++ b/drivers/perf/riscv_pmu_sbi.c
>> @@ -130,7 +130,20 @@ static struct attribute_group riscv_cdeleg_pmu_format_group = {
>>   	.attrs = riscv_cdeleg_pmu_formats_attr,
>>   };
> [ ... ]
>
> [Severity: Medium]
> Does this code leak the pmu_ctr_list array if PMU counter discovery fails?
>
> In rvpmu_find_ctrs(), pmu_ctr_list is allocated via kcalloc(), but the function
> bails out without freeing it if num_sbi_counters or num_deleg_counters
> exceeds RISCV_MAX_COUNTERS:
>
> drivers/perf/riscv_pmu_sbi.c:rvpmu_find_ctrs() {
> ...
> 	pmu_ctr_list = kcalloc(RISCV_MAX_COUNTERS, sizeof(*pmu_ctr_list),
> 			       GFP_KERNEL);
> 	if (!pmu_ctr_list)
> 		return -ENOMEM;
>
> 	if (riscv_pmu_cdeleg_available_boot())
> 		num_deleg_counters = rvpmu_deleg_find_ctrs();
>
> 	/* This is required for firmware counters even if the above is true */
> 	if (riscv_pmu_sbi_available_boot())
> 		num_sbi_counters = rvpmu_sbi_find_num_ctrs();
>
> 	if (num_sbi_counters > RISCV_MAX_COUNTERS || num_deleg_counters > RISCV_MAX_COUNTERS)
> 		return -ENOSPC;
> ...
> }
>
> Also, could num_sbi_counters inadvertently capture negative error codes?
>
> rvpmu_sbi_find_num_ctrs() returns an int which can be a negative error
> code. Storing it in the u32 num_sbi_counters implicitly casts
> it to a very large positive value.
>
> This triggers the RISCV_MAX_COUNTERS check and causes the function to abort
> with -ENOSPC, losing the original error.
>
> The caller rvpmu_device_probe() then fails to clean up the leaked globally
> scoped pmu_ctr_list pointer during its error handling:
>
> drivers/perf/riscv_pmu_sbi.c:rvpmu_device_probe() {
> ...
> 	num_counters = rvpmu_find_ctrs();
> 	if (num_counters < 0) {
> 		pr_err("SBI PMU extension doesn't provide any counters\n");
> 		goto out_free;
> 	}
> ...
> out_free:
> 	kfree(pmu);
> 	return ret;
> }

valid points but same as issue raised in PATCH11. Fixed in v7.