Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1455781

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

From Lars-Peter Clausen <lars@metafoo.de>
Newsgroups linux.kernel
Subject Re: [PATCH 1/1] Fix NULL pointer dereference in imx serial driver DMA callback
Date 2016-08-03 14:30 +0200
Message-ID <s239L-2mN-23@gated-at.bofh.it> (permalink)
References <s2242-1tn-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 08/03/2016 12:59 PM, Fabien Lahoudere wrote:
> From: Hannu Koivisto <hannu.koivisto@vincit.fi>
> 
> 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@collabora.co.uk>

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

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

[PATCH 1/1] Fix NULL pointer dereference in imx serial driver DMA callback Fabien Lahoudere <fabien.lahoudere@collabora.co.uk> - 2016-08-03 13:00 +0200
  Re: [PATCH 1/1] Fix NULL pointer dereference in imx serial driver DMA  callback Lars-Peter Clausen <lars@metafoo.de> - 2016-08-03 14:10 +0200
  Re: [PATCH 1/1] Fix NULL pointer dereference in imx serial driver DMA  callback Lars-Peter Clausen <lars@metafoo.de> - 2016-08-03 14:30 +0200

csiph-web