Skip to contentSkip to conversation

dmaengine: virt-dma: Free descriptor after callback

PATCH In review Joel Fernandes 3 participants 4 messages Apr 18, 2014 – Apr 22, 2014 dmaengine

lore
Share & tools
lore

Joel Fernandes

author
#0 lore raw

PATCH 1/1 dmaengine: virt-dma: Free descriptor after callback

Free the vd (virt descriptor) after the callback is called. In EDMA driver atleast which uses virt-dma, we make use of the desc during the callback and if its dangerously freed before the callback is called. I also noticed this in omap-dma dmaengine driver.

Cc Vinod Koul <[email protected]>
Cc Dan Williams <[email protected]>
Cc Russell King <[email protected]>
Signed-off-by Joel Fernandes <[email protected]>
---
 drivers/dma/virt-dma.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/dma/virt-dma.c +2 -2
index 6f80432..98aeb7f 100644
@@ -84,10 +84,10 @@ static void vchan_complete(unsigned long arg)
 
 		list_del(&vd->node);
 
-		vc->desc_free(vd);
-
 		if (cb)
 			cb(cb_data);
+
+		vc->desc_free(vd);
 	}
 }
Signature · 1 line
1.7.9.5
#1 lore raw
Quoted 4 lines · Joel Fernandes

Free the vd (virt descriptor) after the callback is called. In EDMA driver atleast which uses virt-dma, we make use of the desc during the callback and if its dangerously freed before the callback is called. I also noticed this in omap-dma dmaengine driver.

You've missed the vital bit of information: why do you make use of the descriptor afterwards? You shouldn't. omap-dma doesn't either.

Once clients submit their request to DMA engine, they must not hold any kind of reference to the descriptor other than the cookie.

Signature · 2 lines
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
author
#2 lore raw
Quoted 6 lines · Russell King - ARM Linux
Quoted 4 lines · Joel Fernandes

Free the vd (virt descriptor) after the callback is called. In EDMA driver atleast which uses virt-dma, we make use of the desc during the callback and if its dangerously freed before the callback is called. I also noticed this in omap-dma dmaengine driver.

Quoted 6 lines

You've missed the vital bit of information: why do you make use of the descriptor afterwards? You shouldn't. omap-dma doesn't either.

Once clients submit their request to DMA engine, they must not hold any kind of reference to the descriptor other than the cookie.

Sorry, I confused edma/omap-dma callbacks for virt dma callbacks.

Anyway, I think there is still a chance in edma that we refer to the echan->edesc pointer later on after virt-dma calls the free (in edma_execute), so I'll just NULL that out to be safe and submit a patch. Thanks.

regards,
-Joel

#3 lore raw
Quoted 14 lines · Joel Fernandes
Quoted 6 lines · Russell King - ARM Linux
Quoted 4 lines · Joel Fernandes

Free the vd (virt descriptor) after the callback is called. In EDMA driver atleast which uses virt-dma, we make use of the desc during the callback and if its dangerously freed before the callback is called. I also noticed this in omap-dma dmaengine driver.

Quoted 6 lines

You've missed the vital bit of information: why do you make use of the descriptor afterwards? You shouldn't. omap-dma doesn't either.

Once clients submit their request to DMA engine, they must not hold any kind of reference to the descriptor other than the cookie.

Quoted 5 lines

Sorry, I confused edma/omap-dma callbacks for virt dma callbacks.

Anyway, I think there is still a chance in edma that we refer to the echan->edesc pointer later on after virt-dma calls the free (in edma_execute), so I'll just NULL that out to be safe and submit a patch.

Yes, that would be the right way :)

While looking at this, I see it is not called out specfically in documentation, will update that as well

Signature · 1 line
~Vinod