k3dma: Avoid null pointer traversal

PATCH In review John Stultz 2 participants 2 messages Dec 18, 2019 – Dec 23, 2019 dmaengine

Share & tools

John Stultz

author

2019-12-18 19:09 UTC #0 lore raw

PATCH 1/1 k3dma: Avoid null pointer traversal

In some cases we seem to submit two transactions in a row, which causes us to lose track of the first. If we then cancel the request, we may still get an interrupt, which traverses a null ds_run value.

So try to avoid starting a new transaction if the ds_run value is set.

While this patch avoids the null pointer crash, I've had some reports of the k3dma driver still getting confused, which suggests the ds_run/ds_done value handling still isn't quite right. However, I've not run into an issue recently with it so I think this patch is worth pushing upstream to avoid the crash.

Cc Vinod Koul <[email protected]>

Cc Dan Williams <[email protected]>

Cc [email protected]

Signed-off-by John Stultz <[email protected]>

---
 drivers/dma/k3dma.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

drivers/dma/k3dma.c +9 -3

index adecea51814f..c5c1aa0dcaed 100644
@@ -229,9 +229,11 @@ static irqreturn_t k3_dma_int_handler(int irq, void *dev_id)
 			c = p->vchan;
 			if (c && (tc1 & BIT(i))) {
 				spin_lock_irqsave(&c->vc.lock, flags);
-				vchan_cookie_complete(&p->ds_run->vd);
-				p->ds_done = p->ds_run;
-				p->ds_run = NULL;
+				if (p->ds_run != NULL) {
+					vchan_cookie_complete(&p->ds_run->vd);
+					p->ds_done = p->ds_run;
+					p->ds_run = NULL;
+				}
 				spin_unlock_irqrestore(&c->vc.lock, flags);
 			}
 			if (c && (tc2 & BIT(i))) {

@@ -271,6 +273,10 @@ static int k3_dma_start_txd(struct k3_dma_chan *c)
 	if (BIT(c->phy->idx) & k3_dma_get_chan_stat(d))
 		return -EAGAIN;
 
+	/* Avoid losing track of  ds_run if a transaction is in flight */
+	if (c->phy->ds_run)
+		return -EAGAIN;
+
 	if (vd) {
 		struct k3_dma_desc_sw *ds =
 			container_of(vd, struct k3_dma_desc_sw, vd);

Signature · 1 line

2.17.1

Vinod Koul

maintainer

2019-12-23 07:51 UTC #1 lore raw

Quoted 8 lines · John Stultz

In some cases we seem to submit two transactions in a row, which causes us to lose track of the first. If we then cancel the request, we may still get an interrupt, which traverses a null ds_run value.
So try to avoid starting a new transaction if the ds_run value is set.

Quoted 6 lines

While this patch avoids the null pointer crash, I've had some reports of the k3dma driver still getting confused, which suggests the ds_run/ds_done value handling still isn't quite right. However, I've not run into an issue recently with it so I think this patch is worth pushing upstream to avoid the crash.

Applied after adding dmaengine tag, thanks

Signature · 1 line

~Vinod