|
Mian Yousaf Kaukab |
f3ffbc |
From: Vladimir Oltean <olteanv@gmail.com>
|
|
Mian Yousaf Kaukab |
f3ffbc |
Date: Tue, 3 Sep 2019 13:57:08 +0300
|
|
Mian Yousaf Kaukab |
f3ffbc |
Subject: spi: spi-fsl-dspi: Fix race condition in TCFQ/EOQ interrupt
|
|
Mian Yousaf Kaukab |
f3ffbc |
|
|
Mian Yousaf Kaukab |
f3ffbc |
Git-commit: e327364948492f6a4e866417d3d4d17d95fed285
|
|
Mian Yousaf Kaukab |
f3ffbc |
Patch-mainline: v5.4-rc1
|
|
Mian Yousaf Kaukab |
f3ffbc |
References: bsc#1167260
|
|
Mian Yousaf Kaukab |
f3ffbc |
|
|
Mian Yousaf Kaukab |
f3ffbc |
When the driver is working in TCFQ/EOQ mode (i.e. interacts with the SPI
|
|
Mian Yousaf Kaukab |
f3ffbc |
controller's FIFOs directly) the following sequence of operations
|
|
Mian Yousaf Kaukab |
f3ffbc |
happens:
|
|
Mian Yousaf Kaukab |
f3ffbc |
|
|
Mian Yousaf Kaukab |
f3ffbc |
- The first byte of the tx buffer gets pushed to the TX FIFO (dspi->len
|
|
Mian Yousaf Kaukab |
f3ffbc |
gets decremented). This triggers the train of interrupts that handle
|
|
Mian Yousaf Kaukab |
f3ffbc |
the rest of the bytes.
|
|
Mian Yousaf Kaukab |
f3ffbc |
|
|
Mian Yousaf Kaukab |
f3ffbc |
- The dspi_interrupt handles a TX confirmation event. It reads the newly
|
|
Mian Yousaf Kaukab |
f3ffbc |
available byte from the RX FIFO, checks the dspi->len exit condition,
|
|
Mian Yousaf Kaukab |
f3ffbc |
and if there's more to be done, it kicks off the next interrupt in the
|
|
Mian Yousaf Kaukab |
f3ffbc |
train by writing the next byte to the TX FIFO.
|
|
Mian Yousaf Kaukab |
f3ffbc |
|
|
Mian Yousaf Kaukab |
f3ffbc |
Now the problem is that the wait queue is woken up one byte too early,
|
|
Mian Yousaf Kaukab |
f3ffbc |
because dspi->len becomes 0 as soon as the byte has been pushed into the
|
|
Mian Yousaf Kaukab |
f3ffbc |
TX FIFO. Its interrupt has not yet been processed and the RX byte has
|
|
Mian Yousaf Kaukab |
f3ffbc |
not been put from the FIFO into the buffer.
|
|
Mian Yousaf Kaukab |
f3ffbc |
|
|
Mian Yousaf Kaukab |
f3ffbc |
Depending on the timing of the wait queue wakeup vs the handling of the
|
|
Mian Yousaf Kaukab |
f3ffbc |
last dspi_interrupt, it can happen that the main SPI message pump thread
|
|
Mian Yousaf Kaukab |
f3ffbc |
has already returned back into the spi_device driver. When the rx buffer
|
|
Mian Yousaf Kaukab |
f3ffbc |
is on stack (which it can be, because in this mode, the DSPI doesn't do
|
|
Mian Yousaf Kaukab |
f3ffbc |
DMA), the last interrupt will perform a memory write into an rx buffer
|
|
Mian Yousaf Kaukab |
f3ffbc |
that has been freed. This manifests as stack corruption.
|
|
Mian Yousaf Kaukab |
f3ffbc |
|
|
Mian Yousaf Kaukab |
f3ffbc |
The solution is to only wake up the wait queue when dspi_rxtx says so,
|
|
Mian Yousaf Kaukab |
f3ffbc |
i.e. after it has processed the last TX confirmation interrupt and
|
|
Mian Yousaf Kaukab |
f3ffbc |
collected the last RX byte.
|
|
Mian Yousaf Kaukab |
f3ffbc |
|
|
Mian Yousaf Kaukab |
f3ffbc |
Fixes: c55be3059159 ("spi: spi-fsl-dspi: Use poll mode in case the platform IRQ is missing")
|
|
Mian Yousaf Kaukab |
f3ffbc |
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
|
|
Mian Yousaf Kaukab |
f3ffbc |
Link: https://lore.kernel.org/r/20190903105708.32273-1-olteanv@gmail.com
|
|
Mian Yousaf Kaukab |
f3ffbc |
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Mian Yousaf Kaukab |
f3ffbc |
Signed-off-by: Mian Yousaf Kaukab <ykaukab@suse.de>
|
|
Mian Yousaf Kaukab |
f3ffbc |
---
|
|
Mian Yousaf Kaukab |
f3ffbc |
drivers/spi/spi-fsl-dspi.c | 4 +---
|
|
Mian Yousaf Kaukab |
f3ffbc |
1 file changed, 1 insertion(+), 3 deletions(-)
|
|
Mian Yousaf Kaukab |
f3ffbc |
|
|
Mian Yousaf Kaukab |
f3ffbc |
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
|
|
Mian Yousaf Kaukab |
f3ffbc |
index 77db43f1290f..bec758e978fb 100644
|
|
Mian Yousaf Kaukab |
f3ffbc |
--- a/drivers/spi/spi-fsl-dspi.c
|
|
Mian Yousaf Kaukab |
f3ffbc |
+++ b/drivers/spi/spi-fsl-dspi.c
|
|
Mian Yousaf Kaukab |
f3ffbc |
@@ -710,9 +710,7 @@ static irqreturn_t dspi_interrupt(int irq, void *dev_id)
|
|
Mian Yousaf Kaukab |
f3ffbc |
if (!(spi_sr & (SPI_SR_EOQF | SPI_SR_TCFQF)))
|
|
Mian Yousaf Kaukab |
f3ffbc |
return IRQ_NONE;
|
|
Mian Yousaf Kaukab |
f3ffbc |
|
|
Mian Yousaf Kaukab |
f3ffbc |
- dspi_rxtx(dspi);
|
|
Mian Yousaf Kaukab |
f3ffbc |
-
|
|
Mian Yousaf Kaukab |
f3ffbc |
- if (!dspi->len) {
|
|
Mian Yousaf Kaukab |
f3ffbc |
+ if (dspi_rxtx(dspi) == 0) {
|
|
Mian Yousaf Kaukab |
f3ffbc |
dspi->waitflags = 1;
|
|
Mian Yousaf Kaukab |
f3ffbc |
wake_up_interruptible(&dspi->waitq);
|
|
Mian Yousaf Kaukab |
f3ffbc |
}
|
|
Mian Yousaf Kaukab |
f3ffbc |
--
|
|
Mian Yousaf Kaukab |
f3ffbc |
2.26.2
|
|
Mian Yousaf Kaukab |
f3ffbc |
|