Re: [PATCH] firmware: samsung: fix stale response flag in acpm_prepare_xfer()

[Tudor Ambarus <tudor.ambarus@linaro.org>]

Hi,

Thanks for the patch!

On 4/27/26 12:02 AM, Titouan Ameline de Cadeville wrote:
> acpm_prepare_xfer() only ever set rx_data->response to true, never
> false. A reused sequence number slot could therefore inherit a stale
> true from a previous transfer that expected a response, causing
> acpm_get_rx() to enter the response-copy path for a fire-and-forget
> transfer whose rxd is NULL.
> 
> Unconditionally assign the correct boolean value so the slot is fully
> reset on every reuse.
> 

How did you find this?

Sashiko identified this too when reviewing the ACPM thermal patches.
I sent some fixes last week, where this bug is squashed as well:

https://lore.kernel.org/linux-samsung-soc/20260423-acpm-fixes-sashiko-reports-v1-0-2217b790925e@linaro.org/T/#m1c32aa4c84ea7e3909bc8fe7599585b71e95d8b5


Thanks!
ta

> Fixes: a88927b534ba ("firmware: add Exynos ACPM protocol driver")
> Signed-off-by: Titouan Ameline de Cadeville <titouan.ameline@gmail.com>
> ---
>  drivers/firmware/samsung/exynos-acpm.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c
> index 16c46ed60837..2fee6bb60efc 100644
> --- a/drivers/firmware/samsung/exynos-acpm.c
> +++ b/drivers/firmware/samsung/exynos-acpm.c
> @@ -380,8 +380,7 @@ static void acpm_prepare_xfer(struct acpm_chan *achan,
>  	/* Clear data for upcoming responses */
>  	rx_data = &achan->rx_data[achan->seqnum - 1];
>  	memset(rx_data->cmd, 0, sizeof(*rx_data->cmd) * rx_data->n_cmd);
> -	if (xfer->rxd)
> -		rx_data->response = true;
> +	rx_data->response = !!xfer->rxd;
>  
>  	/* Flag the index based on seqnum. (seqnum: 1~63, bitmap: 0~62) */
>  	set_bit(achan->seqnum - 1, achan->bitmap_seqnum);