[PATCH 2/3] firmware: arm_scmi: imx: Validate MISC control response size

[Sudeep Holla]

On Thu, Oct 29, 2026 at 10:05:31PM +0000, Philip Radford wrote:
> On Wed, Oct 28, 2026 at 01:05:01PM +0000, Sudeep Holla wrote:
> > scmi_imx_misc_ctrl_get() uses @num to report the number of
> > values returned by firmware, but it did not retain the caller's value
> > as the capacity of @val.
> > 
> 
> Hi,
> 
> > A response with more values than the caller allocated was copied into
> > @val unchecked. For example, the MQS regmap callback supplies storage
> > for one value. Retain the input capacity and reject responses exceeding
> > it before copying, then update @num only after a successful copy.
> > 
> > Document the in/out @num contract of the exported helper.
> > 
> > Reported-by: Sashiko <sashiko-bot at kernel.org>
> > Closes: https://sashiko.dev/#/bug/linux-6897b44a-57fb-4964-95f8-447d0ce526e4
> > Fixes: 61c9f03e22fc ("firmware: arm_scmi: Add initial support for i.MX MISC protocol")
> > Signed-off-by: Sudeep Holla <sudeep.holla at kernel.org>
> > Reviewed-by: Peng Fan <peng.fan at nxp.com>
> > ---
> >  .../firmware/arm_scmi/vendors/imx/imx-sm-misc.c  | 16 ++++++++++++----
> >  include/linux/firmware/imx/sm.h                  |  6 ++++++
> >  2 files changed, 18 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/firmware/arm_scmi/vendors/imx/imx-sm-misc.c b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-misc.c
> > index 0c708b2dd496..f00f7acba845 100644
> > --- a/drivers/firmware/arm_scmi/vendors/imx/imx-sm-misc.c
> > +++ b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-misc.c
> > @@ -273,10 +273,16 @@ static int scmi_imx_misc_ctrl_get(const struct scmi_protocol_handle *ph,
> >  {
> >  	struct scmi_imx_misc_ctrl_get_out *out;
> >  	struct scmi_xfer *t;
> > +	u32 num_values, max_values;
> >  	int ret, i;
> >  	int max_msg_size = ph->hops->get_max_msg_size(ph);
> >  	int max_num = (max_msg_size - sizeof(*out)) / sizeof(__le32);
> >  
> > +	if (!num || !val || !*num)
> > +		return -EINVAL;
> > +
> > +	max_values = *num;
> > +
> >  	ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> >  	if (ret)
> >  		return ret;
> > @@ -290,16 +296,18 @@ static int scmi_imx_misc_ctrl_get(const struct scmi_protocol_handle *ph,
> >  	ret = ph->xops->do_xfer(ph, t);
> >  	if (!ret) {
> >  		out = t->rx.buf;
> > -		*num = le32_to_cpu(out->num);
> > +		num_values = le32_to_cpu(out->num);
> >  
> > -		if (*num >= max_num ||
> > -		    *num * sizeof(__le32) > t->rx.len - sizeof(__le32)) {
> > +		if (num_values > max_values || num_values >= max_num ||
> > +		    num_values * sizeof(__le32) > t->rx.len - sizeof(__le32)) {
> >  			ph->xops->xfer_put(ph, t);
> >  			return -EINVAL;
> >  		}
> 
> Why does num_values have to be less than max_num, can it not be equal to? Isn't max_num
> already smaller than max_msg_size due to the initialisation above?
> 

Good question, I didn't look at that logic as I blindly retained the existing
logic from `*num >= max_num` to `num_values >= max_num`.

The scmi_imx_misc_ctrl_eet() see to allow value equal to max_num. It could
have been overlooked when we reviewed initially.

Peng, thoughts ?

-- 
Regards,
Sudeep