Re: [PATCH 1/1] Fix NULL pointer dereference in imx serial driver DMA callback

[Lars-Peter Clausen]

On 08/03/2016 12:59 PM, Fabien Lahoudere wrote:
> From: Hannu Koivisto <hannu.koivisto@xxxxxxxxx>
> 
> dma_rx_callback() may see NULL dma_chan_rx if DMA interrupt [1] occurs a
> moment[2] before imx_uart_dma_exit() sets it to NULL.  imx_uart_dma_exit()
> calls dmaengine_terminate_all() and dma_release_channel() but neither of
> those prevent the callback being called after they have returned. A similar
> problem has been discussed by ALSA developers
> (http://mailman.alsa-project.org/pipermail/alsa-devel/2013-October/067239.html)
> and it was pointed out that dmaengine_terminate_all() might be called from
> the callback, so we cannot call tasklet_kill() in imx-sdma's code called by
> dmaengine_terminate_all().
> 
> Hopefully it doesn't make sense to call dma_release_channel() from the
> callback, so instead of adding synchronization to imx serial driver, we add
> tasklet_kill() call to sdma_free_chan_resources().  While most DMA drivers
> don't do that, there is one example that does: pl330.
> 
> [1] It schedules sdma_tasklet, which again calls the dma_rx_callback.
> [2] I tested this by scheduling the sdma tasklet as far as right before the
> imx_stop_tx() call in imx_shutdown() and the problem occurred.
> 
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@xxxxxxxxxxxxxxx>

I'd prefer that the driver implements the new synchronization API[1]. This
is a more generic approach and covers of all cases of this race condition.

If the synchronize() callback is implemented the core will automatically
make sure that the channel is synchronized when it is freed.

- Lars

[1]
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=b36f09c3c441a6e59eab9315032e7d546571de3f