Re: [PATCH] ASoC: dmaengine: add runtime status checking in dmaengine_pcm_dma_complete

[Lars-Peter Clausen <lars@metafoo.de>]

On 06/07/2013 04:34 PM, Mark Brown wrote:
> On Fri, Jun 07, 2013 at 07:57:29PM +0800, Qiao Zhou wrote:
>> the dmaengine_pcm_dma_complete callback is usually executed after
>> the dma interrupt, which uses tasklet_schedule, workqueue, or other
>> method for quick int handler return.
>>
>> in some corner case, where pcm stream is released unexpected, like
>> media server is killed, the runtime parameter will be freed. if it
>> happens between the t1 and t2 in below chart, then the callback
>> will try to access members of paramters which is already freed,
>> and kernel panics.
>>
>> to avoid this issue, add runtime checking before other handling in
>> dmaengine_pcm_dma_complete. if pcm stream is already released,
>> just ignore the current handling and return.
> 
> This doesn't seem like a good or robust way of fixing this, if we're
> tearing down the resources the DMA is using while the DMA is in progress
> then in the worst case that might include the memory being DMAed and of
> course there's races if you just check the pointer - the pointer can be
> checked at the same time as it's being freed (or between the free and
> the clear).
> 
> I think we should be either halting the DMA or waiting for it to finish
> here.

I haven't see the original patch, but the proper solution to this problem
should be to add a check to snd_dmaengine_pcm_close() to see if the DMA is
still running, and if it is call dmaengine_terminate_all() for the DMA channel
associated with the PCM. Everything else will probably still be racy.

- Lars