Re: [PATCH] firmware: arm_scmi: Fix OOB in scmi_power_name_get()

[Cristian Marussi <cristian.marussi@arm.com>]

On Fri, May 15, 2026 at 02:00:24PM +0200, Geert Uytterhoeven wrote:
> Hi Cristian,
> 
> On Fri, 15 May 2026 at 13:46, Cristian Marussi <cristian.marussi@arm.com> wrote:
> > On Fri, May 15, 2026 at 01:29:27PM +0200, Geert Uytterhoeven wrote:
> > > On Fri, 15 May 2026 at 12:28, Dan Carpenter <error27@gmail.com> wrote:
> > > > On Fri, May 15, 2026 at 11:59:15AM +0200, Geert Uytterhoeven wrote:
> > > > > scmi_power_name_get() does not validate the domain number passed by the
> > > > > external caller, which may lead to an out-of-bounds access.
> > > >
> > > > Is an external caller an out of tree caller?  So far as I can see this
> > >
> > > I meant a caller outside drivers/firmware/arm_scmi/.
> > >
> > > > is only called by scmi_pm_domain_probe().
> > > >
> > > >         scmi_pd->name = power_ops->name_get(ph, i);
> > > >
> > > > where i < num_domains.
> > >
> > > You are right. But this seems to be only API implementation in
> > > drivers/firmware/arm_scmi/ that does not validate the passed domain
> > > number.
> >
> > Yes we tend to validate protocol operations calls even if apparently
> > safe from teh caller perspective...indeed I have this fixed locally
> > since ages in an horrible patch, that does a lot more, and that I
> > never posted :P
> >
> > Usually, if it is worth, we also build an internal domain get helper to
> > reuse across the protocol unit...but here really there are only 2 call-sites.
> >
> > What I am not sure is what to return: "unknown" is safer as of now than NULL
> > for sure, but really, what happened is NOT that the name was "unknown" (which
> > by itself would be out-of-spec behaviour) it is more that the whole domain that
> > was referred to that was invalid and NOT existent...
> >
> > ....mmm I suppose we are opening another can of worms here :P
> 
> Like scmi_perf_info_get() returning ERR_PTR(-EINVAL) instead of NULL,
> and scmi_perf_domain_probe() never checking the return value anyway?

...oh probably more than that...and related vendor FW that already exploits
these missing checks here and there to arbitrarily skip domains and return
out-of-spec non-contigous sets of domains becasue they cannot bother to
implement properly the spec (or they have simply forked their codebase from
an old drop and never updated it again...)...so that any kernel-side fix
you made along the road carries the risk of breaking something and a string
of possibly needed quirks...

Cheers,
Cristian