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

On Fri, May 22, 2026 at 09:56:32AM +0200, Geert Uytterhoeven wrote:
> Hi Sudeep,
> 
> On Thu, 21 May 2026 at 18:26, Sudeep Holla <[email protected]> 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.
> > >
> > > Fix this by returning "unknown" for invalid domains, like
> > > scmi_reset_name_get() does.
> > >
> > > Fixes: 76a6550990e296a7 ("firmware: arm_scmi: add initial support for power protocol")
> > > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > > ---
> > >  drivers/firmware/arm_scmi/power.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/firmware/arm_scmi/power.c b/drivers/firmware/arm_scmi/power.c
> > > index 3aa84ceb6d2bab68..4a7215e02dec035d 100644
> > > --- a/drivers/firmware/arm_scmi/power.c
> > > +++ b/drivers/firmware/arm_scmi/power.c
> > > @@ -204,8 +204,12 @@ scmi_power_name_get(const struct scmi_protocol_handle *ph,
> > >                   u32 domain)
> > >  {
> > >       struct scmi_power_info *pi = ph->get_priv(ph);
> > > -     struct power_dom_info *dom = pi->dom_info + domain;
> > > +     struct power_dom_info *dom;
> > > +
> > > +     if (domain >= pi->num_domains)
> > > +             return "unknown";
> >
> > The only user of this function must not call it for domain >= pi->num_domains.
> > However, I am thinking if it is bit inconsistent within SCMI core now. I like
> > the way pinmux/ctl handles this as I don't like the alternative for this
> > (i.e. ERRPTR(-EINVAL or something)). Worst case if this ever causes issue
> > we can change the signature of the scmi_{power,reset}_name_get to follow
> > something like pinmux and update the users. Thoughts ? Happy to apply this
> > for now.
> 
> You mean returning an int error code using return statements, and
> returning the objects using passed function pointers?
> 

Yes I was thinking so, scmi pinmux seem to follow that.

> Depends on the number of returned objects: if it's just one (e.g. a
> name or info pointer), then the valid pointer/error pointer idiom is
> very common in Linux.
> 

Makes sense, I have applied it now. Thanks!

-- 
Regards,
Sudeep