[PATCH] dmaengine: dw-axi-dmac: Fix use-after-free on cyclic terminate

[PATCH] dmaengine: dw-axi-dmac: Fix use-after-free on cyclic terminate

[Wang YuWei]

dma_chan_terminate_all() frees all descriptors but leaves vc->cyclic
pointing at the active cyclic descriptor. If a period interrupt has
already run vchan_cyclic_callback() and the vchan tasklet has not run
yet, the tasklet later reads the callback from the freed descriptor:

  CPU0                                  CPU1
  axi_chan_block_xfer_complete()
    vchan_cyclic_callback(vd)
      vc->cyclic = vd
      tasklet_schedule()
                                        dmaengine_terminate_sync()
                                          dma_chan_terminate_all()
                                            vchan_get_all_descriptors()
                                            vchan_dma_desc_free_list()
                                              -> vd freed
                                          dw_axi_dma_synchronize()
                                            tasklet_kill()
  vchan_complete()
    vd = vc->cyclic          <- freed
    cb = vd->tx.callback     <- garbage

tasklet_kill() waits for the scheduled tasklet rather than cancelling
it, so dmaengine_terminate_sync() does not close the window. Any cyclic
user can hit this, e.g. ALSA via pcm_dmaengine. It was found with an
out-of-tree 8250 driver that runs UART RX as a cyclic transfer, where
closing the port crashed in the tasklet:

  Unable to handle kernel execute from non-executable memory
  lr : vchan_complete+0xbc/0x23c

The window is normally only as long as it takes the tasklet to run, but
it grows when the tasklet is delayed under heavy interrupt load, which
is how it was hit here.

The active descriptor stays on desc_issued in this driver. Take it off
and pass it to vchan_terminate_vdesc(), which clears vc->cyclic under
vc.lock, as other virt-dma drivers do in terminate_all().

Fixes: 1deb96c0fa58 ("dmaegine: dw-axi-dmac: Support device_prep_dma_cyclic()")
Cc: stable@vger.kernel.org
Signed-off-by: Wang YuWei <1973615295@qq.com>
---
Tested on the v6.1 based vendor kernel on the hardware where the crash
was seen; build-tested only on mainline (arm64, W=1).

 drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
index 7520c78f5..4d41c7538 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
@@ -1182,6 +1182,7 @@ static int dma_chan_terminate_all(struct dma_chan *dchan)
 {
 	struct axi_dma_chan *chan = dchan_to_axi_dma_chan(dchan);
 	u32 chan_active = BIT(chan->id) << DMAC_CHAN_EN_SHIFT;
+	struct virt_dma_desc *vd;
 	unsigned long flags;
 	u32 val;
 	int ret;
@@ -1202,6 +1203,17 @@ static int dma_chan_terminate_all(struct dma_chan *dchan)
 
 	spin_lock_irqsave(&chan->vc.lock, flags);
 
+	/*
+	 * The active descriptor stays on desc_issued. Hand it to
+	 * vchan_terminate_vdesc() so that a cyclic callback still pending in
+	 * the vchan tasklet is dropped before the descriptor is freed below.
+	 */
+	vd = vchan_next_desc(&chan->vc);
+	if (vd) {
+		list_del(&vd->node);
+		vchan_terminate_vdesc(vd);
+	}
+
 	vchan_get_all_descriptors(&chan->vc, &head);
 
 	chan->cyclic = false;
-- 
2.34.1