]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
firmware: samsung: acpm: Fix infinite loop on sequence number exhaustion
authorTudor Ambarus <tudor.ambarus@linaro.org>
Tue, 5 May 2026 13:13:04 +0000 (13:13 +0000)
committerKrzysztof Kozlowski <krzk@kernel.org>
Fri, 29 May 2026 12:11:23 +0000 (14:11 +0200)
Sashiko identified a possible infinite loop [1].

ACPM IPC sequence numbers are tracked via a 64-bit bitmap. Previously,
acpm_prepare_xfer() used a do...while loop to search for a free
sequence number.

If all 63 available sequence numbers are leaked due to transient
hardware timeouts or mailbox failures, the bitmap becomes full.
The next call to acpm_prepare_xfer() would enter an infinite loop.

Fix this by utilizing the kernel's optimized bitmap search functions
(find_next_zero_bit / find_first_zero_bit). If the pool is completely
exhausted, log the failure and return -EBUSY to allow the kernel to
fail gracefully instead of hanging.

Furthermore, drop the allocation loop entirely. Because
acpm_prepare_xfer() is strictly called under the 'tx_lock' mutex,
sequence number allocations are perfectly serialized. If
find_next_zero_bit() locates a free bit, a single
test_and_set_bit_lock() is mathematically guaranteed to succeed.

To enforce this locking invariant, wrap the allocation in a
WARN_ON_ONCE. If the atomic set fails, it indicates the driver's
mutex serialization is fundamentally broken. The warning generates a
stack trace for debugging, while returning -EIO immediately aborts the
transfer to prevent silent payload corruption.

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>
Link: https://patch.msgid.link/20260505-acpm-fixes-sashiko-reports-v5-7-43b5ee7f1674@linaro.org
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
drivers/firmware/samsung/exynos-acpm.c

index 3fa9fe283be4cd84b3766170bc4c12996d0778cb..19db3674a28f3edffa7d6ed31e937848490afc9e 100644 (file)
@@ -12,6 +12,7 @@
 #include <linux/container_of.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/find.h>
 #include <linux/firmware/samsung/exynos-acpm-protocol.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
@@ -390,34 +391,48 @@ static int acpm_wait_for_queue_slots(struct acpm_chan *achan, u32 next_tx_front)
  * TX queue.
  * @achan:     ACPM channel info.
  * @xfer:      reference to the transfer being prepared.
+ *
+ * Return: 0 on success, -errno otherwise.
  */
-static void acpm_prepare_xfer(struct acpm_chan *achan,
-                             const struct acpm_xfer *xfer)
+static int acpm_prepare_xfer(struct acpm_chan *achan,
+                            const struct acpm_xfer *xfer)
 {
        struct acpm_rx_data *rx_data;
        u32 *txd = (u32 *)xfer->txd;
+       unsigned long size = ACPM_SEQNUM_MAX - 1;
+       unsigned long bit = achan->seqnum;
+
+       bit = find_next_zero_bit(achan->bitmap_seqnum, size, bit);
+       if (bit >= size) {
+               bit = find_first_zero_bit(achan->bitmap_seqnum, size);
+               if (bit >= size) {
+                       dev_err_ratelimited(achan->acpm->dev,
+                                           "ACPM sequence number pool exhausted\n");
+                       return -EBUSY;
+               }
+       }
 
        /*
-        * 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.
+        * Execute the atomic set to formally claim the bit and establish
+        * LKMM Acquire semantics against the RX thread's clear_bit_unlock().
+        * A loop is unnecessary because allocations are strictly serialized
+        * by tx_lock.
         */
-       do {
-               if (++achan->seqnum == ACPM_SEQNUM_MAX)
-                       achan->seqnum = 1;
-               /* Flag the index based on seqnum. (seqnum: 1~63, bitmap: 0~62) */
-       } while (test_and_set_bit_lock(achan->seqnum - 1, achan->bitmap_seqnum));
+       if (WARN_ON_ONCE(test_and_set_bit_lock(bit, achan->bitmap_seqnum)))
+               return -EIO;
 
+       /* Flag the index based on seqnum. (seqnum: 1~63, bitmap: 0~62) */
+       achan->seqnum = bit + 1;
        txd[0] |= FIELD_PREP(ACPM_PROTOCOL_SEQNUM, achan->seqnum);
 
        /* Clear data for upcoming responses */
-       rx_data = &achan->rx_data[achan->seqnum - 1];
+       rx_data = &achan->rx_data[bit];
        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;
+
+       return 0;
 }
 
 /**
@@ -477,7 +492,9 @@ int acpm_do_xfer(struct acpm_handle *handle, const struct acpm_xfer *xfer)
                if (ret)
                        return ret;
 
-               acpm_prepare_xfer(achan, xfer);
+               ret = acpm_prepare_xfer(achan, xfer);
+               if (ret)
+                       return ret;
 
                /* Write TX command. */
                __iowrite32_copy(achan->tx.base + achan->mlen * tx_front,