Skip to content

Commit

Permalink
serial: 8250_dma: Fix DMA Rx rearm race
Browse files Browse the repository at this point in the history
As DMA Rx can be completed from two places, it is possible that DMA Rx
completes before DMA completion callback had a chance to complete it.
Once the previous DMA Rx has been completed, a new one can be started
on the next UART interrupt. The following race is possible
(uart_unlock_and_check_sysrq_irqrestore() replaced with
spin_unlock_irqrestore() for simplicity/clarity):

CPU0					CPU1
					dma_rx_complete()
serial8250_handle_irq()
  spin_lock_irqsave(&port->lock)
  handle_rx_dma()
    serial8250_rx_dma_flush()
      __dma_rx_complete()
        dma->rx_running = 0
        // Complete DMA Rx
  spin_unlock_irqrestore(&port->lock)

serial8250_handle_irq()
  spin_lock_irqsave(&port->lock)
  handle_rx_dma()
    serial8250_rx_dma()
      dma->rx_running = 1
      // Setup a new DMA Rx
  spin_unlock_irqrestore(&port->lock)

					  spin_lock_irqsave(&port->lock)
					  // sees dma->rx_running = 1
					  __dma_rx_complete()
					    dma->rx_running = 0
					    // Incorrectly complete
					    // running DMA Rx

This race seems somewhat theoretical to occur for real but handle it
correctly regardless. Check what is the DMA status before complething
anything in __dma_rx_complete().

Reported-by: Gilles BULOZ <gilles.buloz@kontron.com>
Tested-by: Gilles BULOZ <gilles.buloz@kontron.com>
Fixes: 9ee4b83 ("serial: 8250: Add support for dmaengine")
Cc: stable@vger.kernel.org
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://lore.kernel.org/r/20230130114841.25749-3-ilpo.jarvinen@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
ij-intel authored and gregkh committed Jan 31, 2023
1 parent 3135281 commit 57e9af7
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions drivers/tty/serial/8250/8250_dma.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,23 @@ static void __dma_rx_complete(struct uart_8250_port *p)
struct uart_8250_dma *dma = p->dma;
struct tty_port *tty_port = &p->port.state->port;
struct dma_tx_state state;
enum dma_status dma_status;
int count;

dma->rx_running = 0;
dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state);
/*
* New DMA Rx can be started during the completion handler before it
* could acquire port's lock and it might still be ongoing. Don't to
* anything in such case.
*/
dma_status = dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state);
if (dma_status == DMA_IN_PROGRESS)
return;

count = dma->rx_size - state.residue;

tty_insert_flip_string(tty_port, dma->rx_buf, count);
p->port.icount.rx += count;
dma->rx_running = 0;

tty_flip_buffer_push(tty_port);
}
Expand Down

0 comments on commit 57e9af7

Please sign in to comment.