Re: [PATCH 05/11] firmware: arm_scmi: stop reading past PINCTRL_LIST_ASSOCIATIONS replies

[Cristian Marussi]

On Fri, Oct 30, 2026 at 04:58:36PM +0100, Clément Le Goffic wrote:
> The transport cuts a reply larger than the receive buffer, but the iterator
> still reads every identifier the reply announces. Use only the identifiers
> that were copied and ask again for the others.

Hi Clement,

thanks for these fixes...I'll start from this since it really intersects
my current work....

> 
> Fixes: 2145af01b9cb ("firmware: arm_scmi: Add basic support for SCMI v3.2 pincontrol protocol")
> Signed-off-by: Clément Le Goffic <clegoffic@xxxxxxxxxxxx>
> ---
>  drivers/firmware/arm_scmi/pinctrl.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/firmware/arm_scmi/pinctrl.c b/drivers/firmware/arm_scmi/pinctrl.c
> index 42cb1aef1fe1..69645ce73d9d 100644
> --- a/drivers/firmware/arm_scmi/pinctrl.c
> +++ b/drivers/firmware/arm_scmi/pinctrl.c
> @@ -261,10 +261,22 @@ static int iter_pinctrl_assoc_update_state(struct scmi_iterator_state *st,
>  					   const void *response, void *priv)
>  {
>  	const struct scmi_resp_pinctrl_list_assoc *r = response;
> +	unsigned int nr_fit = 0;
>  
>  	st->num_returned = RETURNED(r->flags);
>  	st->num_remaining = REMAINING(r->flags);
>  
> +	if (st->rx_len > sizeof(*r))
> +		nr_fit = (st->rx_len - sizeof(*r)) / sizeof(r->array[0]);

As Sashiko kindly pointed out to me a few other zillion times O_o in my
Telemetry series, there were a couple of missing sanity checks in the
iterators core so in my last series there is upfront a generic fix

https://lore.kernel.org/arm-scmi/avLTkAtQQSzpgu2A@pluto/T/#me258191dd55db99d53b570be7c51901c31bc70cf

...so we could at least avoid to check here and everywhere else in SCMI
iterators that you have at least a minimum size messages to work on.

> +
> +	if (st->num_returned > nr_fit) {
> +		if (!nr_fit)
> +			return -EPROTO;

My core fix above will discard the iteration upfront in an equivalent way
to this check togateher with the rx_len check above If I get this wrong...BUT..

> +
> +		st->num_remaining += st->num_returned - nr_fit;
> +		st->num_returned = nr_fit;

...why are you fixing and carry on ?
I dont feel that you can safely proceed at this point by just fixing the
returned and the remaining...

...the check is of course needed, but it should be an hard check

	if (st->rx_len < (sizeof(*r) + sizeof(r->array[0]) * st->num_returned))   
                return -EINVAL;

...because something is clearly wrong...indeed, learning from my current
telemetry madness the above scenario could be due not only to a buggy FW
BUT more easily to a misconfigured transport...if the transport size is
misconfigured (as it is mostly NOT negotiated, simply configured) any
message long enough will be silently truncated, so the fact especially
that you guess remaining it can happen to be completely out of sync with
the platform side...

Are you fixing an existent known FW bug here ? in that case I would opt
for a quirk...

Thanks,
Cristian