From 6d0709d2000ae7dbead759715f57ba381b7057bb Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Sat, 26 Nov 2011 12:54:08 +0200 Subject: [PATCH] dma/ep93xx_dma: prevent ep93xx_dma_tasklet() to reference an empty list If dma_terminate_all() is called before the ep93xx_dma_tasklet() gets to run, it tries to access an empty ->active list which results following OOPS: Internal error: Oops - undefined instruction: 0 [#1] CPU: 0 Not tainted (3.2.0-rc1EP-1+ #1008) PC is at 0xc184c868 LR is at ep93xx_dma_tasklet+0xec/0x164 pc : [] lr : [] psr: 00000013 sp : c02b7e70 ip : ffffffff fp : c02b7ea4 r10: 00000100 r9 : 80000013 r8 : c02b7e50 r7 : c02b7e70 r6 : c02b7ea4 r5 : 000000a4 r4 : c02b7e70 r3 : c02b751d r2 : 8ae34598 r1 : c184c6e0 r0 : c02b7ea4 Flags: nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel Control: c000717f Table: c0004000 DAC: 00000017 Process swapper (pid: 0, stack limit = 0xc02b6270) Stack: (0xc02b7e70 to 0xc02b8000) 7e60: c02b7ea4 c02b7e70 c0008b64 c02bd5c4 7e80: c02d60e0 00000000 00000000 c02bd44c c02d60e0 00000100 c02b7ec4 c02b7ea8 7ea0: c001c49c c012b44c 00000018 00000001 c02d60e0 c02b6000 c02b7f04 c02b7ec8 7ec0: c001cbc0 c001c3e4 c02b7eec c02b7ed8 00000006 0000000a c02bf674 c02c458c 7ee0: 00000011 00000000 c02b7f7c c0004000 41129200 c02b0c80 c02b7f14 c02b7f08 7f00: c001cdd0 c001cb38 c02b7f34 c02b7f18 c000983c c001cd98 c0009a60 60000013 7f20: fefb0001 c02b7f7c c02b7f44 c02b7f38 c0008190 c0009810 c02b7f9c c02b7f48 7f40: c0008b64 c0008190 c02c2bf8 00000002 c02b7f90 60000013 c02b6000 c02d1504 7f60: c02baa88 c02baa80 c0004000 41129200 c02b0c80 c02b7f9c c02b7fa0 c02b7f90 7f80: c0009a54 c0009a60 60000013 ffffffff c02b7fbc c02b7fa0 c000a03c c0009a40 7fa0: c02b80b0 c02b19dc c02b19d8 c02baa80 c02b7fcc c02b7fc0 c02384e4 c0009fd4 7fc0: c02b7ff4 c02b7fd0 c029d924 c0238494 c029d49c 00000000 00000000 c02b19dc 7fe0: c0007175 c02b803c 00000000 c02b7ff8 c000803c c029d700 00000000 00000000 Backtrace: [] (ep93xx_dma_tasklet+0x0/0x164) from [] (tasklet_action+0xc8/0xdc) [] (tasklet_action+0x0/0xdc) from [] (__do_softirq+0x98/0x154) r7:c02b6000 r6:c02d60e0 r5:00000001 r4:00000018 [] (__do_softirq+0x0/0x154) from [] (irq_exit+0x48/0x50) [] (irq_exit+0x0/0x50) from [] (handle_IRQ+0x3c/0x8c) [] (handle_IRQ+0x0/0x8c) from [] (asm_do_IRQ+0x10/0x14) r7:c02b7f7c r6:fefb0001 r5:60000013 r4:c0009a60 [] (asm_do_IRQ+0x0/0x14) from [] (__irq_svc+0x24/0xc0) Exception stack(0xc02b7f48 to 0xc02b7f90) 7f40: c02c2bf8 00000002 c02b7f90 60000013 c02b6000 c02d1504 7f60: c02baa88 c02baa80 c0004000 41129200 c02b0c80 c02b7f9c c02b7fa0 c02b7f90 7f80: c0009a54 c0009a60 60000013 ffffffff [] (default_idle+0x0/0x34) from [] (cpu_idle+0x78/0xb0) [] (cpu_idle+0x0/0xb0) from [] (rest_init+0x60/0x78) r7:c02baa80 r6:c02b19d8 r5:c02b19dc r4:c02b80b0 [] (rest_init+0x0/0x78) from [] (start_kernel+0x234/0x278) [] (start_kernel+0x0/0x278) from [] (0xc000803c) r5:c02b803c r4:c0007175 Code: 42555300 54535953 643d4d45 65766972 (53007372) To make the code a bit more robust against things like these, we modify ep93xx_dma_get_active() to return NULL in case of empty ->active list and make sure that callers handle this correctly. Reported-by: Rafal Prylowski Signed-off-by: Mika Westerberg Acked-by: H Hartley Sweeten Signed-off-by: Vinod Koul --- drivers/dma/ep93xx_dma.c | 60 ++++++++++++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/drivers/dma/ep93xx_dma.c b/drivers/dma/ep93xx_dma.c index 526a7424f6a9..59e7a965772b 100644 --- a/drivers/dma/ep93xx_dma.c +++ b/drivers/dma/ep93xx_dma.c @@ -246,6 +246,9 @@ static void ep93xx_dma_set_active(struct ep93xx_dma_chan *edmac, static struct ep93xx_dma_desc * ep93xx_dma_get_active(struct ep93xx_dma_chan *edmac) { + if (list_empty(&edmac->active)) + return NULL; + return list_first_entry(&edmac->active, struct ep93xx_dma_desc, node); } @@ -263,16 +266,22 @@ ep93xx_dma_get_active(struct ep93xx_dma_chan *edmac) */ static bool ep93xx_dma_advance_active(struct ep93xx_dma_chan *edmac) { + struct ep93xx_dma_desc *desc; + list_rotate_left(&edmac->active); if (test_bit(EP93XX_DMA_IS_CYCLIC, &edmac->flags)) return true; + desc = ep93xx_dma_get_active(edmac); + if (!desc) + return false; + /* * If txd.cookie is set it means that we are back in the first * descriptor in the chain and hence done with it. */ - return !ep93xx_dma_get_active(edmac)->txd.cookie; + return !desc->txd.cookie; } /* @@ -327,9 +336,15 @@ static void m2p_hw_shutdown(struct ep93xx_dma_chan *edmac) static void m2p_fill_desc(struct ep93xx_dma_chan *edmac) { - struct ep93xx_dma_desc *desc = ep93xx_dma_get_active(edmac); + struct ep93xx_dma_desc *desc; u32 bus_addr; + desc = ep93xx_dma_get_active(edmac); + if (!desc) { + dev_warn(chan2dev(edmac), "M2P: empty descriptor list\n"); + return; + } + if (ep93xx_dma_chan_direction(&edmac->chan) == DMA_MEM_TO_DEV) bus_addr = desc->src_addr; else @@ -491,7 +506,13 @@ static void m2m_hw_shutdown(struct ep93xx_dma_chan *edmac) static void m2m_fill_desc(struct ep93xx_dma_chan *edmac) { - struct ep93xx_dma_desc *desc = ep93xx_dma_get_active(edmac); + struct ep93xx_dma_desc *desc; + + desc = ep93xx_dma_get_active(edmac); + if (!desc) { + dev_warn(chan2dev(edmac), "M2M: empty descriptor list\n"); + return; + } if (edmac->buffer == 0) { writel(desc->src_addr, edmac->regs + M2M_SAR_BASE0); @@ -669,24 +690,30 @@ static void ep93xx_dma_tasklet(unsigned long data) { struct ep93xx_dma_chan *edmac = (struct ep93xx_dma_chan *)data; struct ep93xx_dma_desc *desc, *d; - dma_async_tx_callback callback; - void *callback_param; + dma_async_tx_callback callback = NULL; + void *callback_param = NULL; LIST_HEAD(list); spin_lock_irq(&edmac->lock); + /* + * If dma_terminate_all() was called before we get to run, the active + * list has become empty. If that happens we aren't supposed to do + * anything more than call ep93xx_dma_advance_work(). + */ desc = ep93xx_dma_get_active(edmac); - if (desc->complete) { - edmac->last_completed = desc->txd.cookie; - list_splice_init(&edmac->active, &list); + if (desc) { + if (desc->complete) { + edmac->last_completed = desc->txd.cookie; + list_splice_init(&edmac->active, &list); + } + callback = desc->txd.callback; + callback_param = desc->txd.callback_param; } spin_unlock_irq(&edmac->lock); /* Pick up the next descriptor from the queue */ ep93xx_dma_advance_work(edmac); - callback = desc->txd.callback; - callback_param = desc->txd.callback_param; - /* Now we can release all the chained descriptors */ list_for_each_entry_safe(desc, d, &list, node) { /* @@ -706,13 +733,22 @@ static void ep93xx_dma_tasklet(unsigned long data) static irqreturn_t ep93xx_dma_interrupt(int irq, void *dev_id) { struct ep93xx_dma_chan *edmac = dev_id; + struct ep93xx_dma_desc *desc; irqreturn_t ret = IRQ_HANDLED; spin_lock(&edmac->lock); + desc = ep93xx_dma_get_active(edmac); + if (!desc) { + dev_warn(chan2dev(edmac), + "got interrupt while active list is empty\n"); + spin_unlock(&edmac->lock); + return IRQ_NONE; + } + switch (edmac->edma->hw_interrupt(edmac)) { case INTERRUPT_DONE: - ep93xx_dma_get_active(edmac)->complete = true; + desc->complete = true; tasklet_schedule(&edmac->tasklet); break;