* FAILED: patch "[PATCH] firmware: samsung: acpm: Fix false timeouts and" failed to apply to 6.18-stable tree
@ 2026-06-15 14:49 gregkh
2026-06-17 12:46 ` [PATCH 6.18.y 1/3] firmware: exynos-acpm: Count number of commands in acpm_xfer Sasha Levin
0 siblings, 1 reply; 4+ messages in thread
From: gregkh @ 2026-06-15 14:49 UTC (permalink / raw)
To: tudor.ambarus, krzk; +Cc: stable
The patch below does not apply to the 6.18-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.18.y
git checkout FETCH_HEAD
git cherry-pick -x c889b146478885344a220dd468e5a08de088cbc5
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2026061536-unchain-squabble-5291@gregkh' --subject-prefix 'PATCH 6.18.y' HEAD^..
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From c889b146478885344a220dd468e5a08de088cbc5 Mon Sep 17 00:00:00 2001
From: Tudor Ambarus <tudor.ambarus@linaro.org>
Date: Tue, 5 May 2026 13:13:02 +0000
Subject: [PATCH] firmware: samsung: acpm: Fix false timeouts and
Use-After-Free in polling
Sashiko identified severe races in the polling state machine [1].
In the ACPM driver's polling mode, threads waited for responses by
monitoring the globally shared 'bitmap_seqnum'. This caused false
timeouts because if a thread processed its response and freed the
sequence number, a concurrent TX thread could immediately reallocate
it before the polling thread woke up.
Additionally, the driver suffered from a cross-thread Use-After-Free
(UAF) preemption race. Previously, acpm_get_rx() cleared the sequence
number of whichever RX message it drained from the hardware queue. This
meant Thread A could globally free Thread B's sequence slot while
Thread B was asleep. A new Thread C could then steal the slot,
overwrite the buffer, and leave Thread B to wake up to corrupted state
or a timeout.
Fix this by rewriting the polling state machine:
1. Decouple polling from the global allocator by introducing a per-slot
'completed' flag, synchronized via smp_store_release() and
smp_load_acquire().
2. Strip acpm_get_saved_rx() out of acpm_get_rx() to make it a pure
queue-draining function. Introduce a 'native_match' boolean argument
which evaluates to true only if the thread natively processed its
own sequence number during the call. This explicitly informs the
polling loop whether it must retrieve its payload from the
cross-thread cache.
3. Centralize the cache fallback and sequence number free (clear_bit)
inside the polling loop. Crucially, the free operation now strictly
targets the thread's own TX sequence number (xfer->txd[0]), rather
than the drained RX sequence number. This enforces strict ownership:
a thread only ever frees its own allocated sequence slot, and only
at the exact moment it completes its poll, eliminating the UAF
window.
Furthermore, explicitly guard the 'native_match' assignment with an
if (rx_seqnum == tx_seqnum) check, even for zero-length (no payload)
responses. While an unguarded assignment wouldn't crash (because the
cache fallback acpm_get_saved_rx() safely returns early on zero-length
transfers) doing so would "lie" to the state machine. If a thread
drained the queue and found another thread's zero-length message,
setting native_match = true would falsely convince the polling loop
that it natively handled its own response. Maintaining a rigorous state
machine requires that native_match is only set when a thread explicitly
processes its own sequence number.
Cc: stable@vger.kernel.org
Fixes: a88927b534ba ("firmware: add Exynos ACPM protocol driver")
Closes: https://sashiko.dev/#/patchset/20260429-acpm-fixes-sashiko-reports-v3-0-47cf74ab09ad%40linaro.org [1]
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Link: https://patch.msgid.link/20260505-acpm-fixes-sashiko-reports-v5-5-43b5ee7f1674@linaro.org
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c
index 9766425a44ab..620880d8820a 100644
--- a/drivers/firmware/samsung/exynos-acpm.c
+++ b/drivers/firmware/samsung/exynos-acpm.c
@@ -105,11 +105,14 @@ struct acpm_queue {
* @cmd: pointer to where the data shall be saved.
* @n_cmd: number of 32-bit commands.
* @rxcnt: expected length of the response in 32-bit words.
+ * @completed: flag indicating if the firmware response has been fully
+ * processed.
*/
struct acpm_rx_data {
u32 *cmd;
size_t n_cmd;
size_t rxcnt;
+ bool completed;
};
#define ACPM_SEQNUM_MAX 64
@@ -204,26 +207,28 @@ static void acpm_get_saved_rx(struct acpm_chan *achan,
rx_seqnum = FIELD_GET(ACPM_PROTOCOL_SEQNUM, rx_data->cmd[0]);
- if (rx_seqnum == tx_seqnum) {
+ if (rx_seqnum == tx_seqnum)
memcpy(xfer->rxd, rx_data->cmd, xfer->rxcnt * sizeof(*xfer->rxd));
- clear_bit(rx_seqnum - 1, achan->bitmap_seqnum);
- }
}
/**
* acpm_get_rx() - get response from RX queue.
* @achan: ACPM channel info.
* @xfer: reference to the transfer to get response for.
+ * @native_match: pointer to a boolean set to true if the thread natively
+ * processed its own sequence number during this call.
*
* Return: 0 on success, -errno otherwise.
*/
-static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer)
+static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer,
+ bool *native_match)
{
u32 rx_front, rx_seqnum, tx_seqnum, seqnum;
const void __iomem *base, *addr;
struct acpm_rx_data *rx_data;
u32 i, val, mlen;
- bool rx_set = false;
+
+ *native_match = false;
guard(mutex)(&achan->rx_lock);
@@ -232,10 +237,8 @@ static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer)
tx_seqnum = FIELD_GET(ACPM_PROTOCOL_SEQNUM, xfer->txd[0]);
- if (i == rx_front) {
- acpm_get_saved_rx(achan, xfer, tx_seqnum);
+ if (i == rx_front)
return 0;
- }
base = achan->rx.base;
mlen = achan->mlen;
@@ -259,8 +262,13 @@ static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer)
if (rx_data->rxcnt) {
if (rx_seqnum == tx_seqnum) {
__ioread32_copy(xfer->rxd, addr, xfer->rxcnt);
- rx_set = true;
- clear_bit(seqnum, achan->bitmap_seqnum);
+ /*
+ * Signal completion to the polling thread.
+ * Pairs with smp_load_acquire() in polling
+ * loop.
+ */
+ smp_store_release(&rx_data->completed, true);
+ *native_match = true;
} else {
/*
* The RX data corresponds to another request.
@@ -270,9 +278,21 @@ static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer)
*/
__ioread32_copy(rx_data->cmd, addr,
rx_data->rxcnt);
+ /*
+ * Signal completion to the polling thread.
+ * Pairs with smp_load_acquire() in polling
+ * loop.
+ */
+ smp_store_release(&rx_data->completed, true);
}
} else {
- clear_bit(seqnum, achan->bitmap_seqnum);
+ /*
+ * Signal completion to the polling thread.
+ * Pairs with smp_load_acquire() in polling loop.
+ */
+ smp_store_release(&rx_data->completed, true);
+ if (rx_seqnum == tx_seqnum)
+ *native_match = true;
}
i = (i + 1) % achan->qlen;
@@ -281,13 +301,6 @@ static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer)
/* We saved all responses, mark RX empty. */
writel(rx_front, achan->rx.rear);
- /*
- * If the response was not in this iteration of the queue, check if the
- * RX data was previously saved.
- */
- if (!rx_set)
- acpm_get_saved_rx(achan, xfer, tx_seqnum);
-
return 0;
}
@@ -302,6 +315,7 @@ static int acpm_dequeue_by_polling(struct acpm_chan *achan,
const struct acpm_xfer *xfer)
{
struct device *dev = achan->acpm->dev;
+ bool native_match;
ktime_t timeout;
u32 seqnum;
int ret;
@@ -310,12 +324,25 @@ static int acpm_dequeue_by_polling(struct acpm_chan *achan,
timeout = ktime_add_us(ktime_get(), ACPM_POLL_TIMEOUT_US);
do {
- ret = acpm_get_rx(achan, xfer);
+ ret = acpm_get_rx(achan, xfer, &native_match);
if (ret)
return ret;
- if (!test_bit(seqnum - 1, achan->bitmap_seqnum))
+ /*
+ * Safely check if our specific transaction has been processed.
+ * smp_load_acquire prevents the CPU from speculatively
+ * executing subsequent instructions before the transaction is
+ * synchronized.
+ */
+ if (smp_load_acquire(&achan->rx_data[seqnum - 1].completed)) {
+ /* Retrieve payload if another thread cached it for us */
+ if (!native_match)
+ acpm_get_saved_rx(achan, xfer, seqnum);
+
+ /* Relinquish ownership of the sequence slot */
+ clear_bit(seqnum - 1, achan->bitmap_seqnum);
return 0;
+ }
/* Determined experimentally. */
udelay(20);
@@ -380,6 +407,7 @@ static void acpm_prepare_xfer(struct acpm_chan *achan,
/* Clear data for upcoming responses */
rx_data = &achan->rx_data[achan->seqnum - 1];
+ rx_data->completed = false;
memset(rx_data->cmd, 0, sizeof(*rx_data->cmd) * rx_data->n_cmd);
/* zero means no response expected */
rx_data->rxcnt = xfer->rxcnt;
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 6.18.y 1/3] firmware: exynos-acpm: Count number of commands in acpm_xfer
2026-06-15 14:49 FAILED: patch "[PATCH] firmware: samsung: acpm: Fix false timeouts and" failed to apply to 6.18-stable tree gregkh
@ 2026-06-17 12:46 ` Sasha Levin
2026-06-17 12:46 ` [PATCH 6.18.y 2/3] firmware: samsung: acpm: Fix cross-thread RX length corruption Sasha Levin
2026-06-17 12:46 ` [PATCH 6.18.y 3/3] firmware: samsung: acpm: Fix false timeouts and Use-After-Free in polling Sasha Levin
0 siblings, 2 replies; 4+ messages in thread
From: Sasha Levin @ 2026-06-17 12:46 UTC (permalink / raw)
To: stable; +Cc: Krzysztof Kozlowski, Tudor Ambarus, Krzysztof Kozlowski,
Sasha Levin
From: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
[ Upstream commit 00808ae2e679a97dccc5cf0ee4474ba1e2e8a21a ]
Struct acpm_xfer holds two buffers with u32 commands - rxd and txd - and
counts their size by rxlen and txlen. "len" suffix is here ambiguous,
so could mean length of the buffer or length of commands, and these are
not the same since each command is u32. Rename these to rxcnt and
txcnt, and change their usage to count the number of commands in each
buffer.
This will have a benefit of allowing to use __counted_by_ptr later.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Link: https://patch.msgid.link/20260219-firmare-acpm-counted-v2-2-e1f7389237d3@oss.qualcomm.com
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Stable-dep-of: c889b1464788 ("firmware: samsung: acpm: Fix false timeouts and Use-After-Free in polling")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/firmware/samsung/exynos-acpm-pmic.c | 14 +++++++-------
drivers/firmware/samsung/exynos-acpm.c | 14 +++++++-------
drivers/firmware/samsung/exynos-acpm.h | 4 ++--
3 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/firmware/samsung/exynos-acpm-pmic.c b/drivers/firmware/samsung/exynos-acpm-pmic.c
index 52e89d1b790f01..58edb3115d5fa3 100644
--- a/drivers/firmware/samsung/exynos-acpm-pmic.c
+++ b/drivers/firmware/samsung/exynos-acpm-pmic.c
@@ -63,8 +63,8 @@ static void acpm_pmic_set_xfer(struct acpm_xfer *xfer, u32 *cmd, size_t cmdlen,
{
xfer->txd = cmd;
xfer->rxd = cmd;
- xfer->txlen = cmdlen;
- xfer->rxlen = cmdlen;
+ xfer->txcnt = cmdlen;
+ xfer->rxcnt = cmdlen;
xfer->acpm_chan_id = acpm_chan_id;
}
@@ -86,7 +86,7 @@ int acpm_pmic_read_reg(struct acpm_handle *handle,
int ret;
acpm_pmic_init_read_cmd(cmd, type, reg, chan);
- acpm_pmic_set_xfer(&xfer, cmd, sizeof(cmd), acpm_chan_id);
+ acpm_pmic_set_xfer(&xfer, cmd, ARRAY_SIZE(cmd), acpm_chan_id);
ret = acpm_do_xfer(handle, &xfer);
if (ret)
@@ -119,7 +119,7 @@ int acpm_pmic_bulk_read(struct acpm_handle *handle,
return -EINVAL;
acpm_pmic_init_bulk_read_cmd(cmd, type, reg, chan, count);
- acpm_pmic_set_xfer(&xfer, cmd, sizeof(cmd), acpm_chan_id);
+ acpm_pmic_set_xfer(&xfer, cmd, ARRAY_SIZE(cmd), acpm_chan_id);
ret = acpm_do_xfer(handle, &xfer);
if (ret)
@@ -159,7 +159,7 @@ int acpm_pmic_write_reg(struct acpm_handle *handle,
int ret;
acpm_pmic_init_write_cmd(cmd, type, reg, chan, value);
- acpm_pmic_set_xfer(&xfer, cmd, sizeof(cmd), acpm_chan_id);
+ acpm_pmic_set_xfer(&xfer, cmd, ARRAY_SIZE(cmd), acpm_chan_id);
ret = acpm_do_xfer(handle, &xfer);
if (ret)
@@ -199,7 +199,7 @@ int acpm_pmic_bulk_write(struct acpm_handle *handle,
return -EINVAL;
acpm_pmic_init_bulk_write_cmd(cmd, type, reg, chan, count, buf);
- acpm_pmic_set_xfer(&xfer, cmd, sizeof(cmd), acpm_chan_id);
+ acpm_pmic_set_xfer(&xfer, cmd, ARRAY_SIZE(cmd), acpm_chan_id);
ret = acpm_do_xfer(handle, &xfer);
if (ret)
@@ -229,7 +229,7 @@ int acpm_pmic_update_reg(struct acpm_handle *handle,
int ret;
acpm_pmic_init_update_cmd(cmd, type, reg, chan, value, mask);
- acpm_pmic_set_xfer(&xfer, cmd, sizeof(cmd), acpm_chan_id);
+ acpm_pmic_set_xfer(&xfer, cmd, ARRAY_SIZE(cmd), acpm_chan_id);
ret = acpm_do_xfer(handle, &xfer);
if (ret)
diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c
index 6572cd7be9d177..07bb4af1ca5845 100644
--- a/drivers/firmware/samsung/exynos-acpm.c
+++ b/drivers/firmware/samsung/exynos-acpm.c
@@ -202,7 +202,7 @@ static void acpm_get_saved_rx(struct acpm_chan *achan,
rx_seqnum = FIELD_GET(ACPM_PROTOCOL_SEQNUM, rx_data->cmd[0]);
if (rx_seqnum == tx_seqnum) {
- memcpy(xfer->rxd, rx_data->cmd, xfer->rxlen);
+ memcpy(xfer->rxd, rx_data->cmd, xfer->rxcnt * sizeof(*xfer->rxd));
clear_bit(rx_seqnum - 1, achan->bitmap_seqnum);
}
}
@@ -255,8 +255,7 @@ static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer)
if (rx_data->response) {
if (rx_seqnum == tx_seqnum) {
- __ioread32_copy(xfer->rxd, addr,
- xfer->rxlen / 4);
+ __ioread32_copy(xfer->rxd, addr, xfer->rxcnt);
rx_set = true;
clear_bit(seqnum, achan->bitmap_seqnum);
} else {
@@ -266,8 +265,7 @@ static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer)
* clear yet the bitmap. It will be cleared
* after the response is copied to the request.
*/
- __ioread32_copy(rx_data->cmd, addr,
- xfer->rxlen / 4);
+ __ioread32_copy(rx_data->cmd, addr, xfer->rxcnt);
}
} else {
clear_bit(seqnum, achan->bitmap_seqnum);
@@ -422,7 +420,9 @@ int acpm_do_xfer(struct acpm_handle *handle, const struct acpm_xfer *xfer)
achan = &acpm->chans[xfer->acpm_chan_id];
- if (!xfer->txd || xfer->txlen > achan->mlen || xfer->rxlen > achan->mlen)
+ if (!xfer->txd ||
+ (xfer->txcnt * sizeof(*xfer->txd) > achan->mlen) ||
+ (xfer->rxcnt * sizeof(*xfer->rxd) > achan->mlen))
return -EINVAL;
if (!achan->poll_completion) {
@@ -445,7 +445,7 @@ int acpm_do_xfer(struct acpm_handle *handle, const struct acpm_xfer *xfer)
/* Write TX command. */
__iowrite32_copy(achan->tx.base + achan->mlen * tx_front,
- xfer->txd, xfer->txlen / 4);
+ xfer->txd, xfer->txcnt);
/* Advance TX front. */
writel(idx, achan->tx.front);
diff --git a/drivers/firmware/samsung/exynos-acpm.h b/drivers/firmware/samsung/exynos-acpm.h
index 6417550f89aa97..ac634ef9c6780e 100644
--- a/drivers/firmware/samsung/exynos-acpm.h
+++ b/drivers/firmware/samsung/exynos-acpm.h
@@ -10,8 +10,8 @@
struct acpm_xfer {
const u32 *txd;
u32 *rxd;
- size_t txlen;
- size_t rxlen;
+ size_t txcnt;
+ size_t rxcnt;
unsigned int acpm_chan_id;
};
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 6.18.y 2/3] firmware: samsung: acpm: Fix cross-thread RX length corruption
2026-06-17 12:46 ` [PATCH 6.18.y 1/3] firmware: exynos-acpm: Count number of commands in acpm_xfer Sasha Levin
@ 2026-06-17 12:46 ` Sasha Levin
2026-06-17 12:46 ` [PATCH 6.18.y 3/3] firmware: samsung: acpm: Fix false timeouts and Use-After-Free in polling Sasha Levin
1 sibling, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-06-17 12:46 UTC (permalink / raw)
To: stable
Cc: Tudor Ambarus, Titouan Ameline de Cadeville, Krzysztof Kozlowski,
Sasha Levin
From: Tudor Ambarus <tudor.ambarus@linaro.org>
[ Upstream commit f133bd4b5daf71bccdde0ad1a4f47fac76a6bfb1 ]
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]
Reported-by: Titouan Ameline de Cadeville <titouan.ameline@gmail.com>
Closes: https://lore.kernel.org/r/20260426210255.73674-1-titouan.ameline@gmail.com/
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Link: https://patch.msgid.link/20260505-acpm-fixes-sashiko-reports-v5-1-43b5ee7f1674@linaro.org
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Stable-dep-of: c889b1464788 ("firmware: samsung: acpm: Fix false timeouts and Use-After-Free in polling")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/firmware/samsung/exynos-acpm.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c
index 07bb4af1ca5845..742c0093250435 100644
--- a/drivers/firmware/samsung/exynos-acpm.c
+++ b/drivers/firmware/samsung/exynos-acpm.c
@@ -103,12 +103,12 @@ struct acpm_queue {
*
* @cmd: pointer to where the data shall be saved.
* @n_cmd: number of 32-bit commands.
- * @response: true if the client expects the RX data.
+ * @rxcnt: expected length of the response in 32-bit words.
*/
struct acpm_rx_data {
u32 *cmd;
size_t n_cmd;
- bool response;
+ size_t rxcnt;
};
#define ACPM_SEQNUM_MAX 64
@@ -196,7 +196,7 @@ static void acpm_get_saved_rx(struct acpm_chan *achan,
const struct acpm_rx_data *rx_data = &achan->rx_data[tx_seqnum - 1];
u32 rx_seqnum;
- if (!rx_data->response)
+ if (!rx_data->rxcnt)
return;
rx_seqnum = FIELD_GET(ACPM_PROTOCOL_SEQNUM, rx_data->cmd[0]);
@@ -253,7 +253,7 @@ static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer)
seqnum = rx_seqnum - 1;
rx_data = &achan->rx_data[seqnum];
- if (rx_data->response) {
+ if (rx_data->rxcnt) {
if (rx_seqnum == tx_seqnum) {
__ioread32_copy(xfer->rxd, addr, xfer->rxcnt);
rx_set = true;
@@ -265,7 +265,8 @@ static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer)
* clear yet the bitmap. It will be cleared
* after the response is copied to the request.
*/
- __ioread32_copy(rx_data->cmd, addr, xfer->rxcnt);
+ __ioread32_copy(rx_data->cmd, addr,
+ rx_data->rxcnt);
}
} else {
clear_bit(seqnum, achan->bitmap_seqnum);
@@ -377,8 +378,8 @@ 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;
+ /* zero means no response expected */
+ rx_data->rxcnt = xfer->rxcnt;
/* Flag the index based on seqnum. (seqnum: 1~63, bitmap: 0~62) */
set_bit(achan->seqnum - 1, achan->bitmap_seqnum);
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 6.18.y 3/3] firmware: samsung: acpm: Fix false timeouts and Use-After-Free in polling
2026-06-17 12:46 ` [PATCH 6.18.y 1/3] firmware: exynos-acpm: Count number of commands in acpm_xfer Sasha Levin
2026-06-17 12:46 ` [PATCH 6.18.y 2/3] firmware: samsung: acpm: Fix cross-thread RX length corruption Sasha Levin
@ 2026-06-17 12:46 ` Sasha Levin
1 sibling, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-06-17 12:46 UTC (permalink / raw)
To: stable; +Cc: Tudor Ambarus, Krzysztof Kozlowski, Sasha Levin
From: Tudor Ambarus <tudor.ambarus@linaro.org>
[ Upstream commit c889b146478885344a220dd468e5a08de088cbc5 ]
Sashiko identified severe races in the polling state machine [1].
In the ACPM driver's polling mode, threads waited for responses by
monitoring the globally shared 'bitmap_seqnum'. This caused false
timeouts because if a thread processed its response and freed the
sequence number, a concurrent TX thread could immediately reallocate
it before the polling thread woke up.
Additionally, the driver suffered from a cross-thread Use-After-Free
(UAF) preemption race. Previously, acpm_get_rx() cleared the sequence
number of whichever RX message it drained from the hardware queue. This
meant Thread A could globally free Thread B's sequence slot while
Thread B was asleep. A new Thread C could then steal the slot,
overwrite the buffer, and leave Thread B to wake up to corrupted state
or a timeout.
Fix this by rewriting the polling state machine:
1. Decouple polling from the global allocator by introducing a per-slot
'completed' flag, synchronized via smp_store_release() and
smp_load_acquire().
2. Strip acpm_get_saved_rx() out of acpm_get_rx() to make it a pure
queue-draining function. Introduce a 'native_match' boolean argument
which evaluates to true only if the thread natively processed its
own sequence number during the call. This explicitly informs the
polling loop whether it must retrieve its payload from the
cross-thread cache.
3. Centralize the cache fallback and sequence number free (clear_bit)
inside the polling loop. Crucially, the free operation now strictly
targets the thread's own TX sequence number (xfer->txd[0]), rather
than the drained RX sequence number. This enforces strict ownership:
a thread only ever frees its own allocated sequence slot, and only
at the exact moment it completes its poll, eliminating the UAF
window.
Furthermore, explicitly guard the 'native_match' assignment with an
if (rx_seqnum == tx_seqnum) check, even for zero-length (no payload)
responses. While an unguarded assignment wouldn't crash (because the
cache fallback acpm_get_saved_rx() safely returns early on zero-length
transfers) doing so would "lie" to the state machine. If a thread
drained the queue and found another thread's zero-length message,
setting native_match = true would falsely convince the polling loop
that it natively handled its own response. Maintaining a rigorous state
machine requires that native_match is only set when a thread explicitly
processes its own sequence number.
Cc: stable@vger.kernel.org
Fixes: a88927b534ba ("firmware: add Exynos ACPM protocol driver")
Closes: https://sashiko.dev/#/patchset/20260429-acpm-fixes-sashiko-reports-v3-0-47cf74ab09ad%40linaro.org [1]
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Link: https://patch.msgid.link/20260505-acpm-fixes-sashiko-reports-v5-5-43b5ee7f1674@linaro.org
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/firmware/samsung/exynos-acpm.c | 68 ++++++++++++++++++--------
1 file changed, 48 insertions(+), 20 deletions(-)
diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c
index 742c0093250435..eaa52aeede3772 100644
--- a/drivers/firmware/samsung/exynos-acpm.c
+++ b/drivers/firmware/samsung/exynos-acpm.c
@@ -104,11 +104,14 @@ struct acpm_queue {
* @cmd: pointer to where the data shall be saved.
* @n_cmd: number of 32-bit commands.
* @rxcnt: expected length of the response in 32-bit words.
+ * @completed: flag indicating if the firmware response has been fully
+ * processed.
*/
struct acpm_rx_data {
u32 *cmd;
size_t n_cmd;
size_t rxcnt;
+ bool completed;
};
#define ACPM_SEQNUM_MAX 64
@@ -201,26 +204,28 @@ static void acpm_get_saved_rx(struct acpm_chan *achan,
rx_seqnum = FIELD_GET(ACPM_PROTOCOL_SEQNUM, rx_data->cmd[0]);
- if (rx_seqnum == tx_seqnum) {
+ if (rx_seqnum == tx_seqnum)
memcpy(xfer->rxd, rx_data->cmd, xfer->rxcnt * sizeof(*xfer->rxd));
- clear_bit(rx_seqnum - 1, achan->bitmap_seqnum);
- }
}
/**
* acpm_get_rx() - get response from RX queue.
* @achan: ACPM channel info.
* @xfer: reference to the transfer to get response for.
+ * @native_match: pointer to a boolean set to true if the thread natively
+ * processed its own sequence number during this call.
*
* Return: 0 on success, -errno otherwise.
*/
-static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer)
+static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer,
+ bool *native_match)
{
u32 rx_front, rx_seqnum, tx_seqnum, seqnum;
const void __iomem *base, *addr;
struct acpm_rx_data *rx_data;
u32 i, val, mlen;
- bool rx_set = false;
+
+ *native_match = false;
guard(mutex)(&achan->rx_lock);
@@ -229,10 +234,8 @@ static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer)
tx_seqnum = FIELD_GET(ACPM_PROTOCOL_SEQNUM, xfer->txd[0]);
- if (i == rx_front) {
- acpm_get_saved_rx(achan, xfer, tx_seqnum);
+ if (i == rx_front)
return 0;
- }
base = achan->rx.base;
mlen = achan->mlen;
@@ -256,8 +259,13 @@ static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer)
if (rx_data->rxcnt) {
if (rx_seqnum == tx_seqnum) {
__ioread32_copy(xfer->rxd, addr, xfer->rxcnt);
- rx_set = true;
- clear_bit(seqnum, achan->bitmap_seqnum);
+ /*
+ * Signal completion to the polling thread.
+ * Pairs with smp_load_acquire() in polling
+ * loop.
+ */
+ smp_store_release(&rx_data->completed, true);
+ *native_match = true;
} else {
/*
* The RX data corresponds to another request.
@@ -267,9 +275,21 @@ static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer)
*/
__ioread32_copy(rx_data->cmd, addr,
rx_data->rxcnt);
+ /*
+ * Signal completion to the polling thread.
+ * Pairs with smp_load_acquire() in polling
+ * loop.
+ */
+ smp_store_release(&rx_data->completed, true);
}
} else {
- clear_bit(seqnum, achan->bitmap_seqnum);
+ /*
+ * Signal completion to the polling thread.
+ * Pairs with smp_load_acquire() in polling loop.
+ */
+ smp_store_release(&rx_data->completed, true);
+ if (rx_seqnum == tx_seqnum)
+ *native_match = true;
}
i = (i + 1) % achan->qlen;
@@ -278,13 +298,6 @@ static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer)
/* We saved all responses, mark RX empty. */
writel(rx_front, achan->rx.rear);
- /*
- * If the response was not in this iteration of the queue, check if the
- * RX data was previously saved.
- */
- if (!rx_set)
- acpm_get_saved_rx(achan, xfer, tx_seqnum);
-
return 0;
}
@@ -299,6 +312,7 @@ static int acpm_dequeue_by_polling(struct acpm_chan *achan,
const struct acpm_xfer *xfer)
{
struct device *dev = achan->acpm->dev;
+ bool native_match;
ktime_t timeout;
u32 seqnum;
int ret;
@@ -307,12 +321,25 @@ static int acpm_dequeue_by_polling(struct acpm_chan *achan,
timeout = ktime_add_us(ktime_get(), ACPM_POLL_TIMEOUT_US);
do {
- ret = acpm_get_rx(achan, xfer);
+ ret = acpm_get_rx(achan, xfer, &native_match);
if (ret)
return ret;
- if (!test_bit(seqnum - 1, achan->bitmap_seqnum))
+ /*
+ * Safely check if our specific transaction has been processed.
+ * smp_load_acquire prevents the CPU from speculatively
+ * executing subsequent instructions before the transaction is
+ * synchronized.
+ */
+ if (smp_load_acquire(&achan->rx_data[seqnum - 1].completed)) {
+ /* Retrieve payload if another thread cached it for us */
+ if (!native_match)
+ acpm_get_saved_rx(achan, xfer, seqnum);
+
+ /* Relinquish ownership of the sequence slot */
+ clear_bit(seqnum - 1, achan->bitmap_seqnum);
return 0;
+ }
/* Determined experimentally. */
udelay(20);
@@ -377,6 +404,7 @@ static void acpm_prepare_xfer(struct acpm_chan *achan,
/* Clear data for upcoming responses */
rx_data = &achan->rx_data[achan->seqnum - 1];
+ rx_data->completed = false;
memset(rx_data->cmd, 0, sizeof(*rx_data->cmd) * rx_data->n_cmd);
/* zero means no response expected */
rx_data->rxcnt = xfer->rxcnt;
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-06-17 12:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-15 14:49 FAILED: patch "[PATCH] firmware: samsung: acpm: Fix false timeouts and" failed to apply to 6.18-stable tree gregkh
2026-06-17 12:46 ` [PATCH 6.18.y 1/3] firmware: exynos-acpm: Count number of commands in acpm_xfer Sasha Levin
2026-06-17 12:46 ` [PATCH 6.18.y 2/3] firmware: samsung: acpm: Fix cross-thread RX length corruption Sasha Levin
2026-06-17 12:46 ` [PATCH 6.18.y 3/3] firmware: samsung: acpm: Fix false timeouts and Use-After-Free in polling Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox