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

[Andrew Jones]

On Tue, Jul 04, 2023 at 02:18:47AM +0200, Heinrich Schuchardt wrote:
> 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.
>

Yeah, scratch the cap/warn suggestion. Despite mhpm_count being detected,
not coded, for most platforms, the architecture states their can never be
more than 29. It's definitely a fail scenario if more are detected, or
coded.

Reviewed-by: Andrew Jones <ajones at ventanamicro.com>

Thanks,
drew