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

[Titouan Ameline]

I was reading through the driver code and noticed that
acpm_prepare_xfer() only enters the

if (xfer->rxd) branch to set response = true
with no corresponding else to reset it to false.

Since seqnum slots are recycled, I traced what happens when a slot
previously used by a response-expecting transfer gets reused by a
fire-and-forget one

-> the stale true remains and the wrong branch is taken in acpm_get_rx().

Le lun. 27 avr. 2026 à 10:48, Tudor Ambarus <tudor.ambarus at linaro.org> a écrit :
>
> 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 at 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);
>