omap_hsmmc: race between omap_hsmmc_start_command() and DMA callback

omap_hsmmc: race between omap_hsmmc_start_command() and DMA callback

[Daniel Mack]

Hi,

I'm facing a NULL pointer dereference in omap_hsmmc_start_command() on
an AM33xx board running 3.11-rc1 (DMA enabled).

A quick debug session showed that DMA engine timing leads to a very
reproducable race condition. In omap_hsmmc_request(), we have:

        host->mrq = req;
        omap_hsmmc_prepare_data()
		omap_hsmmc_start_dma_transfer()
			tx->callback = omap_hsmmc_dma_callback;

	[*]

	omap_hsmmc_start_command()
		if (cmd == host->mrq->stop) [<-- oops]
			...

It turns out that omap_hsmmc_dma_callback() (which sets host->mrq =
NULL) is entered just after the DMA submission, and *before*
omap_hsmmc_start_command() is called, consequently leading to an Oops.

I can debug this in more depth, but maybe someone has an idea already?


Thanks,
Daniel

Re: omap_hsmmc: race between omap_hsmmc_start_command() and DMA callback

[Balaji T K]

On Thursday 18 July 2013 09:36 PM, Daniel Mack wrote:
> Hi,
>
> I'm facing a NULL pointer dereference in omap_hsmmc_start_command() on
> an AM33xx board running 3.11-rc1 (DMA enabled).
>
> A quick debug session showed that DMA engine timing leads to a very
> reproducable race condition. In omap_hsmmc_request(), we have:
>
>          host->mrq = req;
>          omap_hsmmc_prepare_data()
> 		omap_hsmmc_start_dma_transfer()
> 			tx->callback = omap_hsmmc_dma_callback;
>
> 	[*]
>
> 	omap_hsmmc_start_command()
> 		if (cmd == host->mrq->stop) [<-- oops]
> 			...
>
> It turns out that omap_hsmmc_dma_callback() (which sets host->mrq =
> NULL) is entered just after the DMA submission, and *before*
> omap_hsmmc_start_command() is called, consequently leading to an Oops.
>
> I can debug this in more depth, but maybe someone has an idea already?
>

Can you check with this hack patch in addition to other dependent patch
for adding edma nodes to dt[1] and slave sg limit [2]

diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index a432e6c..5a19164 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -1262,8 +1262,8 @@ int edma_start(unsigned channel)
  		if (test_bit(channel, edma_cc[ctlr]->edma_unused)) {
  			pr_debug("EDMA: ESR%d %08x\n", j,
  				edma_shadow0_read_array(ctlr, SH_ESR, j));
-			edma_shadow0_write_array(ctlr, SH_ESR, j, mask);
-			return 0;
+//			edma_shadow0_write_array(ctlr, SH_ESR, j, mask);
+//			return 0;
  		}

  		/* EDMA channel with event association */
-- 

[1] https://lkml.org/lkml/2013/6/18/49
[2] https://patchwork.kernel.org/patch/2228041/

>
> Thanks,
> Daniel
>