[PATCH v5 6/7] firmware: samsung: acpm: Fix missing LKMM barriers in sequence allocator
Tudor Ambarus
tudor.ambarus at linaro.org
Tue May 5 06:13:03 PDT 2026
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.
Cc: stable at vger.kernel.org
Fixes: a88927b534ba ("firmware: add Exynos ACPM protocol driver")
Closes: https://sashiko.dev/#/patchset/20260423-acpm-fixes-sashiko-reports-v1-0-2217b790925e%40linaro.org [1]
Signed-off-by: Tudor Ambarus <tudor.ambarus at linaro.org>
---
drivers/firmware/samsung/exynos-acpm.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c
index 2dea9b7bfe91..fd2e46e9f7e9 100644
--- a/drivers/firmware/samsung/exynos-acpm.c
+++ b/drivers/firmware/samsung/exynos-acpm.c
@@ -8,7 +8,7 @@
#include <asm/barrier.h>
#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>
@@ -344,7 +344,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;
}
@@ -401,11 +401,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);
@@ -415,9 +422,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);
}
/**
--
2.54.0.545.g6539524ca2-goog
More information about the linux-arm-kernel
mailing list