FAILED: patch "[PATCH] firmware: samsung: acpm: Fix missing LKMM barriers in" failed to apply to 7.0-stable tree
[email protected]
maintainerThe patch below does not apply to the 7.0-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 <[email protected]>.
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-7.0.y
git checkout FETCH_HEAD
git cherry-pick -x bf296f83a3ddab1ab875edc4e8862cb10553064f
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026061551-lung-clutter-0849@gregkh' --subject-prefix 'PATCH 7.0.y' HEAD^..
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From bf296f83a3ddab1ab875edc4e8862cb10553064f Mon Sep 17 00:00:00 2001
From: Tudor Ambarus <[email protected]>
Date: Tue, 5 May 2026 13:13:03 +0000
Subject: [PATCH] firmware: samsung: acpm: Fix missing LKMM barriers in
sequence allocator
Sashiko identified memory ordering races in [1].
The ACPM driver uses a globally shared 'bitmap_seqnum' to track available sequence numbers. Even though threads now strictly free their own sequence numbers, the allocation and freeing of these bits across concurrent threads are effectively lockless operations and require explicit LKMM memory barriers.
Previously, the driver used plain bitwise operators (test_bit, set_bit, clear_bit), which lack ordering guarantees. This creates two race conditions on weakly ordered architectures like ARM64:
1. Polling Release Violation: The polling thread copies its payload and calls clear_bit(). Without a release barrier, the CPU can reorder the memory operations, making the cleared bit globally visible before the payload reads have fully completed.
2. TX Acquire Violation: The TX thread loops on test_bit(), calls set_bit(), and then wipes the payload buffer via memset(). Without an acquire barrier, the CPU can speculatively execute the memset() before the bit is safely and formally claimed.
If these reorderings overlap, a new TX thread can claim the sequence number and overwrite the buffer while the original polling thread is still actively reading from it.
Fix this by upgrading the bitwise operators. Wrap the TX allocation in test_and_set_bit_lock() to establish formal LKMM Acquire semantics, and pair it with clear_bit_unlock() in the polling path to enforce Release semantics.
drivers/firmware/samsung/exynos-acpm.c +11 -7
@@ -7,7 +7,7 @@#include <linux/bitfield.h> #include <linux/bitmap.h> -#include <linux/bits.h> +#include <linux/bitops.h> #include <linux/cleanup.h> #include <linux/container_of.h> #include <linux/delay.h>
@@ -340,7 +340,7 @@ static int acpm_dequeue_by_polling(struct acpm_chan *achan,acpm_get_saved_rx (achan, xfer, seqnum);/* Relinquish ownership of the sequence slot */ -clear_bit (seqnum -1 , achan->bitmap_seqnum); +clear_bit_unlock (seqnum -1 , achan->bitmap_seqnum);return 0 ; }
@@ -397,11 +397,18 @@ static void acpm_prepare_xfer(struct acpm_chan *achan,struct acpm_rx_data *rx_data;u32 *txd = (u32 *)xfer->txd; -/* Prevent chan->seqnum from being re-used */ +/* +* Prevent chan->seqnum from being re-used. +* test_and_set_bit_lock() provides formal LKMM Acquire semantics. +* It pairs with the RX thread's clear_bit_unlock() to ensure the CPU +* does not speculatively execute the rx_data buffer wipe (memset) +* before the sequence number is safely claimed. +*/ do {if (++achan->seqnum ==ACPM_SEQNUM_MAX ) achan->seqnum =1 ; - }while (test_bit (achan->seqnum -1 , achan->bitmap_seqnum)); +/* Flag the index based on seqnum. (seqnum: 1~63, bitmap: 0~62) */ + }while (test_and_set_bit_lock (achan->seqnum -1 , achan->bitmap_seqnum)); txd[0 ] |=FIELD_PREP (ACPM_PROTOCOL_SEQNUM , achan->seqnum);
@@ -411,9 +418,6 @@ static void acpm_prepare_xfer(struct acpm_chan *achan,memset (rx_data->cmd,0 ,sizeof (*rx_data->cmd) * rx_data->n_cmd);/* 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); }/**
Sasha Levin
maintainerPATCH 1/5 firmware: exynos-acpm: Count number of commands in acpm_xfer
From: Krzysztof Kozlowski <[email protected]>
[ 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.
Stable-dep-of: bf296f83a3dd ("firmware: samsung: acpm: Fix missing LKMM barriers in sequence allocator")
--- drivers/firmware/samsung/exynos-acpm-dvfs.c | 9 +++++---- drivers/firmware/samsung/exynos-acpm-pmic.c | 14 +++++++------- drivers/firmware/samsung/exynos-acpm.c | 14 +++++++------- drivers/firmware/samsung/exynos-acpm.h | 4 ++-- 4 files changed, 21 insertions(+), 20 deletions(-)
drivers/firmware/samsung/exynos-acpm-dvfs.c +5 -4
@@ -5,6 +5,7 @@* Copyright 2025 Linaro Ltd. */ +#include <linux/array_size.h> #include <linux/bitfield.h> #include <linux/firmware/samsung/exynos-acpm-protocol.h> #include <linux/ktime.h>
@@ -25,11 +26,11 @@ static void acpm_dvfs_set_xfer(struct acpm_xfer *xfer, u32 *cmd, size_t cmdlen, { xfer->acpm_chan_id = acpm_chan_id; xfer->txd = cmd; - xfer->txlen = cmdlen; + xfer->txcnt = cmdlen;if (response) { xfer->rxd = cmd; - xfer->rxlen = cmdlen; + xfer->rxcnt = cmdlen; } }
@@ -50,7 +51,7 @@ int acpm_dvfs_set_rate(struct acpm_handle *handle,u32 cmd[4 ];acpm_dvfs_init_set_rate_cmd (cmd, clk_id, rate); -acpm_dvfs_set_xfer (&xfer, cmd,sizeof (cmd), acpm_chan_id, false); +acpm_dvfs_set_xfer (&xfer, cmd,ARRAY_SIZE (cmd), acpm_chan_id, false);return acpm_do_xfer (handle, &xfer); }
@@ -70,7 +71,7 @@ unsigned long acpm_dvfs_get_rate(struct acpm_handle *handle,int ret;acpm_dvfs_init_get_rate_cmd (cmd, clk_id); -acpm_dvfs_set_xfer (&xfer, cmd,sizeof (cmd), acpm_chan_id, true); +acpm_dvfs_set_xfer (&xfer, cmd,ARRAY_SIZE (cmd), acpm_chan_id, true); ret =acpm_do_xfer (handle, &xfer);if (ret)
drivers/firmware/samsung/exynos-acpm-pmic.c +7 -7
@@ -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)
drivers/firmware/samsung/exynos-acpm.c +7 -7
@@ -205,7 +205,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); } }
@@ -258,8 +258,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 {
@@ -269,8 +268,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);
@@ -425,7 +423,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) {
@@ -448,7 +448,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);
drivers/firmware/samsung/exynos-acpm.h +2 -2
@@ -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; };
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 2/5 firmware: exynos-acpm: Count acpm_xfer buffers with __counted_by_ptr
From: Krzysztof Kozlowski <[email protected]>
[ Upstream commit 951b8eee0581bbf39e7b0464d679eee8cb9da3e0 ]
Use __counted_by_ptr() attribute on the acpm_xfer buffers so UBSAN will validate runtime that we do not pass over the buffer size, thus making code safer.
Usage of __counted_by_ptr() (or actually __counted_by()) requires that counter is initialized before counted array.
Stable-dep-of: bf296f83a3dd ("firmware: samsung: acpm: Fix missing LKMM barriers in sequence allocator")
--- drivers/firmware/samsung/exynos-acpm-dvfs.c | 4 ++-- drivers/firmware/samsung/exynos-acpm.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/firmware/samsung/exynos-acpm-dvfs.c +2 -2
@@ -25,12 +25,12 @@ static void acpm_dvfs_set_xfer(struct acpm_xfer *xfer, u32 *cmd, size_t cmdlen, unsigned int acpm_chan_id, bool response) { xfer->acpm_chan_id = acpm_chan_id; - xfer->txd = cmd; xfer->txcnt = cmdlen; + xfer->txd = cmd;if (response) { - xfer->rxd = cmd; xfer->rxcnt = cmdlen; + xfer->rxd = cmd; } }
drivers/firmware/samsung/exynos-acpm.h +2 -2
@@ -8,8 +8,8 @@#define __EXYNOS_ACPM_H__struct acpm_xfer { -const u32 *txd; -u32 *rxd; + const u32 *txd __counted_by_ptr(txcnt); + u32 *rxd __counted_by_ptr(rxcnt);size_t txcnt;size_t rxcnt;unsigned int acpm_chan_id;
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 3/5 firmware: samsung: acpm: Fix cross-thread RX length corruption
In reply to Sasha LevinFrom: Tudor Ambarus <[email protected]>
[ 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.
Stable-dep-of: bf296f83a3dd ("firmware: samsung: acpm: Fix missing LKMM barriers in sequence allocator")
--- drivers/firmware/samsung/exynos-acpm-dvfs.c | 3 +++ drivers/firmware/samsung/exynos-acpm.c | 15 ++++++++------- 2 files changed, 11 insertions(+), 7 deletions(-)
drivers/firmware/samsung/exynos-acpm-dvfs.c +3 -0
@@ -31,6 +31,9 @@ static void acpm_dvfs_set_xfer(struct acpm_xfer *xfer, u32 *cmd, size_t cmdlen,if (response) { xfer->rxcnt = cmdlen; xfer->rxd = cmd; + }else { + xfer->rxcnt =0 ; + xfer->rxd =NULL ; } }
drivers/firmware/samsung/exynos-acpm.c +8 -7
@@ -104,12 +104,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
@@ -199,7 +199,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 ]);
@@ -256,7 +256,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;
@@ -268,7 +268,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);
@@ -380,8 +381,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);
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 4/5 firmware: samsung: acpm: Fix false timeouts and Use-After-Free in polling
In reply to Sasha LevinFrom: Tudor Ambarus <[email protected]>
[ 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.
Stable-dep-of: bf296f83a3dd ("firmware: samsung: acpm: Fix missing LKMM barriers in sequence allocator")
--- drivers/firmware/samsung/exynos-acpm.c | 68 ++++++++++++++++++-------- 1 file changed, 48 insertions(+), 20 deletions(-)
drivers/firmware/samsung/exynos-acpm.c +48 -20
@@ -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;
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 5/5 firmware: samsung: acpm: Fix missing LKMM barriers in sequence allocator
In reply to Sasha LevinFrom: Tudor Ambarus <[email protected]>
[ Upstream commit bf296f83a3ddab1ab875edc4e8862cb10553064f ]
Sashiko identified memory ordering races in [1].
The ACPM driver uses a globally shared 'bitmap_seqnum' to track available sequence numbers. Even though threads now strictly free their own sequence numbers, the allocation and freeing of these bits across concurrent threads are effectively lockless operations and require explicit LKMM memory barriers.
Previously, the driver used plain bitwise operators (test_bit, set_bit, clear_bit), which lack ordering guarantees. This creates two race conditions on weakly ordered architectures like ARM64:
1. Polling Release Violation: The polling thread copies its payload and calls clear_bit(). Without a release barrier, the CPU can reorder the memory operations, making the cleared bit globally visible before the payload reads have fully completed.
2. TX Acquire Violation: The TX thread loops on test_bit(), calls set_bit(), and then wipes the payload buffer via memset(). Without an acquire barrier, the CPU can speculatively execute the memset() before the bit is safely and formally claimed.
If these reorderings overlap, a new TX thread can claim the sequence number and overwrite the buffer while the original polling thread is still actively reading from it.
Fix this by upgrading the bitwise operators. Wrap the TX allocation in test_and_set_bit_lock() to establish formal LKMM Acquire semantics, and pair it with clear_bit_unlock() in the polling path to enforce Release semantics.
--- drivers/firmware/samsung/exynos-acpm.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
drivers/firmware/samsung/exynos-acpm.c +11 -7
@@ -7,7 +7,7 @@#include <linux/bitfield.h> #include <linux/bitmap.h> -#include <linux/bits.h> +#include <linux/bitops.h> #include <linux/cleanup.h> #include <linux/container_of.h> #include <linux/delay.h>
@@ -340,7 +340,7 @@ static int acpm_dequeue_by_polling(struct acpm_chan *achan,acpm_get_saved_rx (achan, xfer, seqnum);/* Relinquish ownership of the sequence slot */ -clear_bit (seqnum -1 , achan->bitmap_seqnum); +clear_bit_unlock (seqnum -1 , achan->bitmap_seqnum);return 0 ; }
@@ -397,11 +397,18 @@ static void acpm_prepare_xfer(struct acpm_chan *achan,struct acpm_rx_data *rx_data;u32 *txd = (u32 *)xfer->txd; -/* Prevent chan->seqnum from being re-used */ +/* +* Prevent chan->seqnum from being re-used. +* test_and_set_bit_lock() provides formal LKMM Acquire semantics. +* It pairs with the RX thread's clear_bit_unlock() to ensure the CPU +* does not speculatively execute the rx_data buffer wipe (memset) +* before the sequence number is safely claimed. +*/ do {if (++achan->seqnum ==ACPM_SEQNUM_MAX ) achan->seqnum =1 ; - }while (test_bit (achan->seqnum -1 , achan->bitmap_seqnum)); +/* Flag the index based on seqnum. (seqnum: 1~63, bitmap: 0~62) */ + }while (test_and_set_bit_lock (achan->seqnum -1 , achan->bitmap_seqnum)); txd[0 ] |=FIELD_PREP (ACPM_PROTOCOL_SEQNUM , achan->seqnum);
@@ -411,9 +418,6 @@ static void acpm_prepare_xfer(struct acpm_chan *achan,memset (rx_data->cmd,0 ,sizeof (*rx_data->cmd) * rx_data->n_cmd);/* 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); }/**
Signature · 1 line
2.53.0