If RX mapping fails after TX mapping succeeds, __spi_map_msg() unmaps
TX but leaves tx_sg_mapped set. If TX mapping fails on a later
transfer, mappings created for earlier transfers remain active.
In both cases, cur_{tx,rx}_dma_dev have not yet been updated because they
are assigned only after every transfer has been mapped. The subsequent
spi_unmap_msg() may therefore unmap the TX mapping again or release
earlier mappings using a NULL or stale device. Using a NULL device can
trigger an oops. An empty SG table does not prevent the NULL dereference
because dma_unmap_sg_attrs() accesses the device before checking the
entry count.
Publish both mapping devices before mapping starts and unwind all
failures through __spi_unmap_msg(). This clears the mapping flags and
releases each mapping once with the device that created it.
Publishing the devices before the loop also refreshes them when no
transfer needs mapping. No mapping flag is set in that case, so current
users do not use the pointers as mapping owners.
Fixes: e289df82344f ("spi: Rework per message DMA mapped flag to be per
transfer")
Cc: [email protected]
Signed-off-by: Honghui Jiang <[email protected]>
---
drivers/spi/spi.c | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index d9e6b4b87..12b3a3b29 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1231,6 +1231,8 @@ void spi_unmap_buf(struct spi_controller *ctlr, struct
device *dev,
spi_unmap_buf_attrs(ctlr, dev, sgt, dir, 0);
}
+static int __spi_unmap_msg(struct spi_controller *ctlr, struct spi_message
*msg);
+
static int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)
{
struct device *tx_dev, *rx_dev;
@@ -1254,7 +1256,13 @@ static int __spi_map_msg(struct spi_controller *ctlr,
struct spi_message *msg)
else
rx_dev = ctlr->dev.parent;
- ret = -ENOMSG;
+ /*
+ * Store the devices before mapping so partial failures can be unwound
+ * with the device that created each mapping.
+ */
+ ctlr->cur_tx_dma_dev = tx_dev;
+ ctlr->cur_rx_dma_dev = rx_dev;
+
list_for_each_entry(xfer, &msg->transfers, transfer_list) {
/* The sync is done before each transfer. */
unsigned long attrs = DMA_ATTR_SKIP_CPU_SYNC;
@@ -1267,8 +1275,8 @@ static int __spi_map_msg(struct spi_controller *ctlr,
struct spi_message *msg)
(void *)xfer->tx_buf,
xfer->len, DMA_TO_DEVICE,
attrs);
- if (ret != 0)
- return ret;
+ if (ret)
+ goto unwind;
xfer->tx_sg_mapped = true;
}
@@ -1277,25 +1285,19 @@ static int __spi_map_msg(struct spi_controller *ctlr,
struct spi_message *msg)
ret = spi_map_buf_attrs(ctlr, rx_dev, &xfer->rx_sg,
xfer->rx_buf, xfer->len,
DMA_FROM_DEVICE, attrs);
- if (ret != 0) {
- spi_unmap_buf_attrs(ctlr, tx_dev,
- &xfer->tx_sg, DMA_TO_DEVICE,
- attrs);
-
- return ret;
- }
+ if (ret)
+ goto unwind;
xfer->rx_sg_mapped = true;
}
}
- /* No transfer has been mapped, bail out with success */
- if (ret)
- return 0;
-
- ctlr->cur_rx_dma_dev = rx_dev;
- ctlr->cur_tx_dma_dev = tx_dev;
return 0;
+
+unwind:
+ __spi_unmap_msg(ctlr, msg);
+
+ return ret;
}
static int __spi_unmap_msg(struct spi_controller *ctlr, struct spi_message
*msg)
--
2.43.0
