[PATCH v2] spi: pic32-sqi: guard against empty BD list in pic32_sqi_one_message()

[PATCH v2] spi: pic32-sqi: guard against empty BD list in pic32_sqi_one_message()

[Li Ximo]

From: Wang Haoran <haoranwangsec@gmail.com>

list_first_entry()/list_last_entry() do not return NULL on an empty
list; they return the container_of() of the list head, i.e. a wild
pointer.

The SPI core rejects a message with no transfers, but it accepts a
transfer that has neither tx_buf nor rx_buf. For such transfers
pic32_sqi_one_transfer() iterates an empty scatterlist, queues no
buffer descriptor and still returns success, so a message made up only
of buffer-less transfers leaves sqi->bd_list_used empty. The subsequent
list_last_entry() then returns a wild pointer through which the driver
sets BD_LAST/BD_CS_DEASSERT, and list_first_entry() returns another one
whose bd_dma is programmed as the DMA base address, corrupting memory.

Bail out with -EINVAL when the BD list is empty, setting msg->status
the same way the timeout path below does. The xfer_out cleanup path
tolerates an empty list, and normal messages are unaffected.

This issue was found by static analysis.

Reported-by: Wang Haoran <haoranwangsec@gmail.com>
Reported-by: Li Ximo <lixicaicai@gmail.com>
Signed-off-by: Wang Haoran <haoranwangsec@gmail.com>
---
v1: https://lore.kernel.org/r/20261026115219.3528114-1-lixicaicai@gmail.com
v2: set msg->status on the error path as the timeout path does, and
correct the description of how the empty list is reached (the core
rejects transfer-less messages but accepts buffer-less transfers), per
Mark Brown's review.

 drivers/spi/spi-pic32-sqi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/spi/spi-pic32-sqi.c b/drivers/spi/spi-pic32-sqi.c
index 5d3921e29..ce8d60497 100644
--- a/drivers/spi/spi-pic32-sqi.c
+++ b/drivers/spi/spi-pic32-sqi.c
@@ -391,6 +391,17 @@ static int pic32_sqi_one_message(struct spi_controller *host,
 		}
 	}
 
+	/*
+	 * A message whose transfers all have neither tx_buf nor rx_buf
+	 * leaves the BD list empty. Fail the message like the timeout
+	 * path does, before chaining the BDs.
+	 */
+	if (list_empty(&sqi->bd_list_used)) {
+		ret = -EINVAL;
+		msg->status = ret;
+		goto xfer_out;
+	}
+
 	/* BDs are prepared and chained. Now mark LAST_BD, CS_DEASSERT at last
 	 * element of the list.
 	 */
-- 
2.30.2