From 161314b856b5a00a4c425958ff746885bd974721 Mon Sep 17 00:00:00 2001
From: Ben Zhang <benzh@chromium.org>
Date: Tue, 3 May 2016 21:53:16 -0700
Subject: [PATCH] CHROMIUM: dmaengine: tegra-adma: fix race between
tasklet/terminate_all (take 3)
commit 58148c0ad35f ("CHROMIUM: dmaengine: tegra-adma: Fix
another race between tasklet/terminate_all") added
tasklet_kill which is called from atomic context, causing
scheduling while atomic bug. This patch moves the the sync
point to device_free_chan_resources which is not atomic.
BUG=chrome-os-partner:46247, b:26806769
TEST=Add udelay(10000) in the tasklet before calling the
callback to simulate heavy system load. Observe the
scheduling while atomic bug. After applying this patch,
observe the bug is gone and audio playback works on Ryu.
Change-Id: I8d4408364cea492f094e15306c09d153315c9782
Signed-off-by: Ben Zhang <benzh@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/342355
Reviewed-by: Andrew Bresticker <abrestic@chromium.org>
(cherry picked from commit 20115e8a450a03cea5ff7b2e5db5fc35081b4637)
Reviewed-on: https://chromium-review.googlesource.com/342835
Commit-Queue: Andrew Bresticker <abrestic@chromium.org>
Tested-by: Andrew Bresticker <abrestic@chromium.org>
---
drivers/dma/tegra210-adma.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
index 6fb90aa7f85b1..03e88ea1841bb 100644
--- a/drivers/dma/tegra210-adma.c
+++ b/drivers/dma/tegra210-adma.c
@@ -734,18 +734,18 @@ static void tegra_adma_terminate_all(struct dma_chan *dc)
/*
* The tasklet may be active and so set the current callback
* count to zero to terminate the tasklet.
+ * 1. If tegra_adma_terminate_all is called by the tasklet callback
+ * itself due to xrun, callback_count=0 ensures the callback is not
+ * called again after the current callback returns.
+ * 2. If tegra_adma_terminate_all is called by the client thread that
+ * wants to stop audio, no new callbacks are going to be scheduled
+ * after this point. However, the callback can still be running on
+ * another CPU at this point if it was started before tdc->lock is
+ * grabbed. We sync with the callback in device_free_chan_resources
+ * since the current context is atomic.
*/
tdc->callback_count = 0;
- /* Make sure the tasklet has stopped running before we return. */
- if (!in_interrupt()) {
- tdc->busy = true;
- spin_unlock_irqrestore(&tdc->lock, flags);
- tasklet_kill(&tdc->tasklet);
- spin_lock_irqsave(&tdc->lock, flags);
- tdc->busy = false;
- }
-
spin_unlock_irqrestore(&tdc->lock, flags);
}
@@ -1141,6 +1141,18 @@ static void tegra_adma_free_chan_resources(struct dma_chan *dc)
if (tdc->busy)
tegra_adma_terminate_all(dc);
+
+ /* Ensure the callback in the tasklet finishes before freeing resources.
+ * We cannot sync in tegra_adma_terminate_all, because it is called
+ * from atomic context SNDRV_PCM_TRIGGER_STOP->DMA_TERMINATE_ALL.
+ * The current context is not atomic (from pcm_close op), so it's OK
+ * to sleep and sync here.
+ * Without this sync, the callback (e.g. dmaengine_pcm_dma_complete)
+ * may still be running after snd_pcm_release which frees
+ * snd_pcm_substream, causing use-after-free crash.
+ */
+ tasklet_kill(&tdc->tasklet);
+
pm_runtime_put(tdc->tdma->dev);
spin_lock_irqsave(&tdc->lock, flags);
list_splice_init(&tdc->pending_sg_req, &sg_req_list);