| Message ID | 20181213184444.21904-3-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | serial: sh-sci: Fix fallback to PIO on DMA failure | expand |
On Thu, Dec 13, 2018 at 07:44:42PM +0100, Geert Uytterhoeven wrote: > When falling back to PIO, active_rx must be set to a different value > than cookie_rx[i], else sci_dma_rx_find_active() will incorrectly find a > match, leading to a NULL pointer dereference in rx_timer_fn() later. > > Use zero instead, which is the same value as after driver > initialization. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> This looks good so to me long as dmaengine_submit() doesn't return 0. Is that the case? > --- > v4: > - No changes, > > v3: > - Split in multiple patches. > --- > drivers/tty/serial/sh-sci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c > index 2a08039f792235e5..e353e03ce2602559 100644 > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -1369,7 +1369,7 @@ static void sci_submit_rx(struct sci_port *s, bool port_lock_held) > dmaengine_terminate_async(chan); > for (i = 0; i < 2; i++) > s->cookie_rx[i] = -EINVAL; > - s->active_rx = -EINVAL; > + s->active_rx = 0; > s->chan_rx = NULL; > sci_start_rx(port); > if (!port_lock_held) > -- > 2.17.1 >
Hi Simon, On Mon, Dec 17, 2018 at 2:22 PM Simon Horman <horms@verge.net.au> wrote: > On Thu, Dec 13, 2018 at 07:44:42PM +0100, Geert Uytterhoeven wrote: > > When falling back to PIO, active_rx must be set to a different value > > than cookie_rx[i], else sci_dma_rx_find_active() will incorrectly find a > > match, leading to a NULL pointer dereference in rx_timer_fn() later. > > > > Use zero instead, which is the same value as after driver > > initialization. > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > This looks good so to me long as dmaengine_submit() doesn't return 0. > Is that the case? include/linux/dmaengine.h: /** * typedef dma_cookie_t - an opaque DMA cookie * * if dma_cookie_t is >0 it's a DMA request cookie, <0 it's an error code */ typedef s32 dma_cookie_t; #define DMA_MIN_COOKIE 1 Gr{oetje,eeting}s, Geert
On Mon, Dec 17, 2018 at 02:27:47PM +0100, Geert Uytterhoeven wrote: > Hi Simon, > > On Mon, Dec 17, 2018 at 2:22 PM Simon Horman <horms@verge.net.au> wrote: > > On Thu, Dec 13, 2018 at 07:44:42PM +0100, Geert Uytterhoeven wrote: > > > When falling back to PIO, active_rx must be set to a different value > > > than cookie_rx[i], else sci_dma_rx_find_active() will incorrectly find a > > > match, leading to a NULL pointer dereference in rx_timer_fn() later. > > > > > > Use zero instead, which is the same value as after driver > > > initialization. > > > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > This looks good so to me long as dmaengine_submit() doesn't return 0. > > Is that the case? > > include/linux/dmaengine.h: > > /** > * typedef dma_cookie_t - an opaque DMA cookie > * > * if dma_cookie_t is >0 it's a DMA request cookie, <0 it's an error code > */ > typedef s32 dma_cookie_t; > #define DMA_MIN_COOKIE 1 In that case I have no objections. Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c index 2a08039f792235e5..e353e03ce2602559 100644 --- a/drivers/tty/serial/sh-sci.c +++ b/drivers/tty/serial/sh-sci.c @@ -1369,7 +1369,7 @@ static void sci_submit_rx(struct sci_port *s, bool port_lock_held) dmaengine_terminate_async(chan); for (i = 0; i < 2; i++) s->cookie_rx[i] = -EINVAL; - s->active_rx = -EINVAL; + s->active_rx = 0; s->chan_rx = NULL; sci_start_rx(port); if (!port_lock_held)
When falling back to PIO, active_rx must be set to a different value than cookie_rx[i], else sci_dma_rx_find_active() will incorrectly find a match, leading to a NULL pointer dereference in rx_timer_fn() later. Use zero instead, which is the same value as after driver initialization. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- v4: - No changes, v3: - Split in multiple patches. --- drivers/tty/serial/sh-sci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)