Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

serial: sh-sci: fix a race of DMA submit_tx on transfer

When DMA is enabled, sh-sci transfer begins with
uart_start()
sci_start_tx()
if (cookie_tx < 0) schedule_work()
Then, starts DMA when wq scheduled, -- (A)
process_one_work()
work_fn_rx()
cookie_tx = desc->submit_tx()
And finishes when DMA transfer ends, -- (B)
sci_dma_tx_complete()
async_tx_ack()
cookie_tx = -EINVAL
(possible another schedule_work())

This A to B sequence is not reentrant, since controlling variables
(for example, cookie_tx above) are not queues nor lists. So, they
must be invoked as A B A B..., otherwise results in kernel crash.

To ensure the sequence, sci_start_tx() seems to test if cookie_tx < 0
(represents "not used") to call schedule_work().
But cookie_tx will not be set (to a cookie, also means "used") until
in the middle of work queue scheduled function work_fn_tx().

This gap between the test and set allows the breakage of the sequence
under the very frequently call of uart_start().
Another gap between async_tx_ack() and another schedule_work() results
in the same issue, too.

This patch introduces a new condition "cookie_tx == 0" just to mark
it is "busy" and assign it within spin-locked region to fill the gaps.

Signed-off-by: Takashi Yoshii <takashi.yoshii.zj@renesas.com>
Reviewed-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: stable@vger.kernel.org
Signed-off-by: Paul Mundt <lethal@linux-sh.org>

authored by

Yoshii Takashi and committed by
Paul Mundt
49d4bcad ffe0e190

+10 -5
+10 -5
drivers/tty/serial/sh-sci.c
··· 1229 1229 port->icount.tx += sg_dma_len(&s->sg_tx); 1230 1230 1231 1231 async_tx_ack(s->desc_tx); 1232 - s->cookie_tx = -EINVAL; 1233 1232 s->desc_tx = NULL; 1234 1233 1235 1234 if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) 1236 1235 uart_write_wakeup(port); 1237 1236 1238 1237 if (!uart_circ_empty(xmit)) { 1238 + s->cookie_tx = 0; 1239 1239 schedule_work(&s->work_tx); 1240 - } else if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) { 1241 - u16 ctrl = sci_in(port, SCSCR); 1242 - sci_out(port, SCSCR, ctrl & ~SCSCR_TIE); 1240 + } else { 1241 + s->cookie_tx = -EINVAL; 1242 + if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) { 1243 + u16 ctrl = sci_in(port, SCSCR); 1244 + sci_out(port, SCSCR, ctrl & ~SCSCR_TIE); 1245 + } 1243 1246 } 1244 1247 1245 1248 spin_unlock_irqrestore(&port->lock, flags); ··· 1504 1501 } 1505 1502 1506 1503 if (s->chan_tx && !uart_circ_empty(&s->port.state->xmit) && 1507 - s->cookie_tx < 0) 1504 + s->cookie_tx < 0) { 1505 + s->cookie_tx = 0; 1508 1506 schedule_work(&s->work_tx); 1507 + } 1509 1508 #endif 1510 1509 1511 1510 if (!s->chan_tx || port->type == PORT_SCIFA || port->type == PORT_SCIFB) {