Skip to content

Commit 2ac3a10

Browse files
leitaogregkh
authored andcommitted
spi: tegra210-quad: Protect curr_xfer check in IRQ handler
[ Upstream commit edf9088 ] Now that all other accesses to curr_xfer are done under the lock, protect the curr_xfer NULL check in tegra_qspi_isr_thread() with the spinlock. Without this protection, the following race can occur: CPU0 (ISR thread) CPU1 (timeout path) ---------------- ------------------- if (!tqspi->curr_xfer) // sees non-NULL spin_lock() tqspi->curr_xfer = NULL spin_unlock() handle_*_xfer() spin_lock() t = tqspi->curr_xfer // NULL! ... t->len ... // NULL dereference! With this patch, all curr_xfer accesses are now properly synchronized. Although all accesses to curr_xfer are done under the lock, in tegra_qspi_isr_thread() it checks for NULL, releases the lock and reacquires it later in handle_cpu_based_xfer()/handle_dma_based_xfer(). There is a potential for an update in between, which could cause a NULL pointer dereference. To handle this, add a NULL check inside the handlers after acquiring the lock. This ensures that if the timeout path has already cleared curr_xfer, the handler will safely return without dereferencing the NULL pointer. Fixes: b4e002d ("spi: tegra210-quad: Fix timeout handling") Signed-off-by: Breno Leitao <[email protected]> Tested-by: Jon Hunter <[email protected]> Acked-by: Jon Hunter <[email protected]> Acked-by: Thierry Reding <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Mark Brown <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent d51554d commit 2ac3a10

1 file changed

Lines changed: 20 additions & 0 deletions

File tree

drivers/spi/spi-tegra210-quad.c

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1393,6 +1393,11 @@ static irqreturn_t handle_cpu_based_xfer(struct tegra_qspi *tqspi)
13931393
spin_lock_irqsave(&tqspi->lock, flags);
13941394
t = tqspi->curr_xfer;
13951395

1396+
if (!t) {
1397+
spin_unlock_irqrestore(&tqspi->lock, flags);
1398+
return IRQ_HANDLED;
1399+
}
1400+
13961401
if (tqspi->tx_status || tqspi->rx_status) {
13971402
tegra_qspi_handle_error(tqspi);
13981403
complete(&tqspi->xfer_completion);
@@ -1463,6 +1468,11 @@ static irqreturn_t handle_dma_based_xfer(struct tegra_qspi *tqspi)
14631468
spin_lock_irqsave(&tqspi->lock, flags);
14641469
t = tqspi->curr_xfer;
14651470

1471+
if (!t) {
1472+
spin_unlock_irqrestore(&tqspi->lock, flags);
1473+
return IRQ_HANDLED;
1474+
}
1475+
14661476
if (num_errors) {
14671477
tegra_qspi_dma_unmap_xfer(tqspi, t);
14681478
tegra_qspi_handle_error(tqspi);
@@ -1501,6 +1511,7 @@ static irqreturn_t handle_dma_based_xfer(struct tegra_qspi *tqspi)
15011511
static irqreturn_t tegra_qspi_isr_thread(int irq, void *context_data)
15021512
{
15031513
struct tegra_qspi *tqspi = context_data;
1514+
unsigned long flags;
15041515
u32 status;
15051516

15061517
/*
@@ -1518,7 +1529,9 @@ static irqreturn_t tegra_qspi_isr_thread(int irq, void *context_data)
15181529
* If no transfer is in progress, check if this was a real interrupt
15191530
* that the timeout handler already processed, or a spurious one.
15201531
*/
1532+
spin_lock_irqsave(&tqspi->lock, flags);
15211533
if (!tqspi->curr_xfer) {
1534+
spin_unlock_irqrestore(&tqspi->lock, flags);
15221535
/* Spurious interrupt - transfer not ready */
15231536
if (!(status & QSPI_RDY))
15241537
return IRQ_NONE;
@@ -1535,7 +1548,14 @@ static irqreturn_t tegra_qspi_isr_thread(int irq, void *context_data)
15351548
tqspi->rx_status = tqspi->status_reg & (QSPI_RX_FIFO_OVF | QSPI_RX_FIFO_UNF);
15361549

15371550
tegra_qspi_mask_clear_irq(tqspi);
1551+
spin_unlock_irqrestore(&tqspi->lock, flags);
15381552

1553+
/*
1554+
* Lock is released here but handlers safely re-check curr_xfer under
1555+
* lock before dereferencing.
1556+
* DMA handler also needs to sleep in wait_for_completion_*(), which
1557+
* cannot be done while holding spinlock.
1558+
*/
15391559
if (!tqspi->is_curr_dma_xfer)
15401560
return handle_cpu_based_xfer(tqspi);
15411561

0 commit comments

Comments
 (0)