Re: [PATCH v4 1/7] firmware: samsung: acpm: Fix cross-thread RX length corruption

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

On 5/4/26 9:36 PM, Krzysztof Kozlowski wrote:
> On 04/05/2026 12:15, Tudor Ambarus wrote:
>> Sashiko identified a cross-thread RX length corruption bug when
>> reviewing the thermal addition to ACPM [1].
>>
>> When multiple threads concurrently send IPC requests, the ACPM polling
>> mechanism can encounter responses belonging to other threads. To drain
>> the queue, the driver saves these concurrent responses into an internal
>> cache (`rx_data->cmd`) to be retrieved later by the owning thread.
>>
>> Previously, the driver incorrectly used `xfer->rxcnt` (the expected
>> receive length of the *current* polling thread) when copying data for
>> *other* threads into this cache. If the threads expected responses of
>> different lengths, this resulted in buffer underflows (leading to reads
>> of uninitialized memory) or potential buffer overflows.
>>
>> Fix this by replacing the boolean `response` flag in
>> `struct acpm_rx_data` with `rxcnt`, caching the exact expected receive
>> length for each specific transaction during transfer preparation. Use
>> this cached length when saving concurrent responses.
>>
>> Consequently, ensure that `xfer->rxcnt` is explicitly zeroed in driver
>> helpers (e.g., `acpm_dvfs_set_xfer`) for fire-and-forget messages to
>> prevent uninitialized stack garbage from being interpreted as a massive
>> expected receive length.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: a88927b534ba ("firmware: add Exynos ACPM protocol driver")
>> Closes: https://sashiko.dev/#/patchset/20260420-acpm-tmu-v3-0-3dc8e93f0b26%40linaro.org [1]
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> 
> I think parallel credits for Titouan Ameline would be suitable here.

I agree.

> If there is going to be new version, please also add:

There's going to be a new version, will add. I have to admit I'm
impressed by sashiko's review skills.

Cheers,
ta