| Message ID | 20191220131100.21804-1-peter.ujfalusi@ti.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | dmaengine: virt-dma: Fix access after free in vcna_complete() | expand |
On Fri, 2019-12-20 at 15:11 +0200, Peter Ujfalusi wrote: > [External] > > vchan_vdesc_fini() is freeing up 'vd' so the access to vd->tx_result is > via already freed up memory. > > Move the vchan_vdesc_fini() after invoking the callback to avoid this. > Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > Fixes: 09d5b702b0f97 ("dmaengine: virt-dma: store result on dma > descriptor") > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > --- > drivers/dma/virt-dma.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/dma/virt-dma.c b/drivers/dma/virt-dma.c > index ec4adf4260a0..256fc662c500 100644 > --- a/drivers/dma/virt-dma.c > +++ b/drivers/dma/virt-dma.c > @@ -104,9 +104,8 @@ static void vchan_complete(unsigned long arg) > dmaengine_desc_get_callback(&vd->tx, &cb); > > list_del(&vd->node); > - vchan_vdesc_fini(vd); > - > dmaengine_desc_callback_invoke(&cb, &vd->tx_result); > + vchan_vdesc_fini(vd); > } > } >
On Fri, 2019-12-20 at 15:11 +0200, Peter Ujfalusi wrote: > [External] > > vchan_vdesc_fini() is freeing up 'vd' so the access to vd->tx_result is > via already freed up memory. > > Move the vchan_vdesc_fini() after invoking the callback to avoid this. > Apologies for seeing this too late: typo in title vcna_complete() -> vchan_complete() > Fixes: 09d5b702b0f97 ("dmaengine: virt-dma: store result on dma > descriptor") > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > --- > drivers/dma/virt-dma.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/dma/virt-dma.c b/drivers/dma/virt-dma.c > index ec4adf4260a0..256fc662c500 100644 > --- a/drivers/dma/virt-dma.c > +++ b/drivers/dma/virt-dma.c > @@ -104,9 +104,8 @@ static void vchan_complete(unsigned long arg) > dmaengine_desc_get_callback(&vd->tx, &cb); > > list_del(&vd->node); > - vchan_vdesc_fini(vd); > - > dmaengine_desc_callback_invoke(&cb, &vd->tx_result); > + vchan_vdesc_fini(vd); > } > } >
On 20/12/2019 16.01, Ardelean, Alexandru wrote: > On Fri, 2019-12-20 at 15:11 +0200, Peter Ujfalusi wrote: >> [External] >> >> vchan_vdesc_fini() is freeing up 'vd' so the access to vd->tx_result is >> via already freed up memory. >> >> Move the vchan_vdesc_fini() after invoking the callback to avoid this. >> > > Apologies for seeing this too late: typo in title vcna_complete() -> > vchan_complete() Yep, I also noticed after sending it, I hope Vinod is kind enough and fix it up when applying ;) - Péter >> Fixes: 09d5b702b0f97 ("dmaengine: virt-dma: store result on dma >> descriptor") >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >> --- >> drivers/dma/virt-dma.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/drivers/dma/virt-dma.c b/drivers/dma/virt-dma.c >> index ec4adf4260a0..256fc662c500 100644 >> --- a/drivers/dma/virt-dma.c >> +++ b/drivers/dma/virt-dma.c >> @@ -104,9 +104,8 @@ static void vchan_complete(unsigned long arg) >> dmaengine_desc_get_callback(&vd->tx, &cb); >> >> list_del(&vd->node); >> - vchan_vdesc_fini(vd); >> - >> dmaengine_desc_callback_invoke(&cb, &vd->tx_result); >> + vchan_vdesc_fini(vd); >> } >> } >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 20-12-19, 15:11, Peter Ujfalusi wrote: > vchan_vdesc_fini() is freeing up 'vd' so the access to vd->tx_result is > via already freed up memory. > > Move the vchan_vdesc_fini() after invoking the callback to avoid this. Applied, thanks
On 20-12-19, 16:50, Peter Ujfalusi wrote: > > > On 20/12/2019 16.01, Ardelean, Alexandru wrote: > > On Fri, 2019-12-20 at 15:11 +0200, Peter Ujfalusi wrote: > >> [External] > >> > >> vchan_vdesc_fini() is freeing up 'vd' so the access to vd->tx_result is > >> via already freed up memory. > >> > >> Move the vchan_vdesc_fini() after invoking the callback to avoid this. > >> > > > > Apologies for seeing this too late: typo in title vcna_complete() -> > > vchan_complete() > > Yep, I also noticed after sending it, I hope Vinod is kind enough and > fix it up when applying ;) In case it wasnt clear, yeah trivial changes while applying are no hassle :)
diff --git a/drivers/dma/virt-dma.c b/drivers/dma/virt-dma.c index ec4adf4260a0..256fc662c500 100644 --- a/drivers/dma/virt-dma.c +++ b/drivers/dma/virt-dma.c @@ -104,9 +104,8 @@ static void vchan_complete(unsigned long arg) dmaengine_desc_get_callback(&vd->tx, &cb); list_del(&vd->node); - vchan_vdesc_fini(vd); - dmaengine_desc_callback_invoke(&cb, &vd->tx_result); + vchan_vdesc_fini(vd); } }
vchan_vdesc_fini() is freeing up 'vd' so the access to vd->tx_result is via already freed up memory. Move the vchan_vdesc_fini() after invoking the callback to avoid this. Fixes: 09d5b702b0f97 ("dmaengine: virt-dma: store result on dma descriptor") Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> --- drivers/dma/virt-dma.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)