[PATCH 1/1] lib: sbi_pmu: Avoid out of bounds access

[Heinrich Schuchardt]

On 03.07.23 16:50, Andrew Jones wrote:
> On Mon, Jul 03, 2023 at 03:43:18PM +0200, Heinrich Schuchardt wrote:
>> On a misconfigured system we could access phs->active_events[] out of
>> bounds. Check that num_hw_ctrs is less or equal SBI_PMU_HW_CTR_MAX.
>>
>> Addresses-Coverity-ID: 1566113 ("Out-of-bounds read")
>> Addresses-Coverity-ID: 1566114 ("Out-of-bounds write")
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
>> ---
>>   lib/sbi/sbi_pmu.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
>> index c73e6ef..7213a53 100644
>> --- a/lib/sbi/sbi_pmu.c
>> +++ b/lib/sbi/sbi_pmu.c
>> @@ -933,6 +933,8 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot)
>>   
>>   		/* mcycle & minstret is available always */
>>   		num_hw_ctrs = sbi_hart_mhpm_count(scratch) + 3;
>> +		if (num_hw_ctrs > SBI_PMU_HW_CTR_MAX)
>> +			return SBI_EINVAL;
>>   		total_ctrs = num_hw_ctrs + SBI_PMU_FW_CTR_MAX;
>>   	}
>>   
>> -- 
>> 2.40.1
>>
> 
> Should we instead cap and warn in hart_detect_features()?

Thanks for reviewing.

By default U-Boot invokes OpenSBI with SBI_SCRATCH_NO_BOOT_PRINTS (to 
save a few ms boot time). You might never see the warning.

mhpm_count is hard coded, e.g.

platform/generic/allwinner/sun20i-d1.c:275:
hfeatures->mhpm_count = 29;

If this value is too high, the coding needs to be changed.

Best regards

Heinrich