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

[Philip Radford <philip.radford@arm.com>]

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@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@kernel.org>
> Reviewed-by: Peng Fan <peng.fan@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?

Regards,
Phil