| Message ID | 1573126013-17609-1-git-send-email-bianpan2016@163.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dmaengine: sun6i: Fix use after free | expand |
On 07-11-19, 19:26, Pan Bian wrote: > The members in the LLI list is released in an incorrect way. Read and > store the next member before releasing it to avoid accessing the freed > memory. > > Fixes: a90e173f3faf ("dmaengine: sun6i: Add cyclic capability") > > Signed-off-by: Pan Bian <bianpan2016@163.com> > --- > drivers/dma/sun6i-dma.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c > index 06cd7f867f7c..096aad7e75bb 100644 > --- a/drivers/dma/sun6i-dma.c > +++ b/drivers/dma/sun6i-dma.c > @@ -687,7 +687,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg( > struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device); > struct sun6i_vchan *vchan = to_sun6i_vchan(chan); > struct dma_slave_config *sconfig = &vchan->cfg; > - struct sun6i_dma_lli *v_lli, *prev = NULL; > + struct sun6i_dma_lli *v_lli, *next, *prev = NULL; > struct sun6i_desc *txd; > struct scatterlist *sg; > dma_addr_t p_lli; > @@ -752,8 +752,12 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg( > return vchan_tx_prep(&vchan->vc, &txd->vd, flags); > > err_lli_free: > - for (prev = txd->v_lli; prev; prev = prev->v_lli_next) > - dma_pool_free(sdev->pool, prev, virt_to_phys(prev)); > + v_lli = txd->v_lli; > + while (v_lli) { > + next = v_lli->v_lli_next; > + dma_pool_free(sdev->pool, v_lli, virt_to_phys(v_lli)); > + v_lli = next; Have you seen this issue, a panic trace? Has some static checker flagged this? I see the code *seems* equivalent. The prev is assigned to txd->v_lli, checked and then code block will be executed and updated. When do you see the case when we access freed memory?
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c index 06cd7f867f7c..096aad7e75bb 100644 --- a/drivers/dma/sun6i-dma.c +++ b/drivers/dma/sun6i-dma.c @@ -687,7 +687,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg( struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device); struct sun6i_vchan *vchan = to_sun6i_vchan(chan); struct dma_slave_config *sconfig = &vchan->cfg; - struct sun6i_dma_lli *v_lli, *prev = NULL; + struct sun6i_dma_lli *v_lli, *next, *prev = NULL; struct sun6i_desc *txd; struct scatterlist *sg; dma_addr_t p_lli; @@ -752,8 +752,12 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg( return vchan_tx_prep(&vchan->vc, &txd->vd, flags); err_lli_free: - for (prev = txd->v_lli; prev; prev = prev->v_lli_next) - dma_pool_free(sdev->pool, prev, virt_to_phys(prev)); + v_lli = txd->v_lli; + while (v_lli) { + next = v_lli->v_lli_next; + dma_pool_free(sdev->pool, v_lli, virt_to_phys(v_lli)); + v_lli = next; + } kfree(txd); return NULL; } @@ -769,7 +773,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_cyclic( struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device); struct sun6i_vchan *vchan = to_sun6i_vchan(chan); struct dma_slave_config *sconfig = &vchan->cfg; - struct sun6i_dma_lli *v_lli, *prev = NULL; + struct sun6i_dma_lli *v_lli, *next, *prev = NULL; struct sun6i_desc *txd; dma_addr_t p_lli; u32 lli_cfg; @@ -820,8 +824,12 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_cyclic( return vchan_tx_prep(&vchan->vc, &txd->vd, flags); err_lli_free: - for (prev = txd->v_lli; prev; prev = prev->v_lli_next) - dma_pool_free(sdev->pool, prev, virt_to_phys(prev)); + v_lli = txd->v_lli; + while (v_lli) { + next = v_lli->v_lli_next; + dma_pool_free(sdev->pool, v_lli, virt_to_phys(v_lli)); + v_lli = next; + } kfree(txd); return NULL; }
The members in the LLI list is released in an incorrect way. Read and store the next member before releasing it to avoid accessing the freed memory. Fixes: a90e173f3faf ("dmaengine: sun6i: Add cyclic capability") Signed-off-by: Pan Bian <bianpan2016@163.com> --- drivers/dma/sun6i-dma.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)