Re: [PATCH] dma: sh: usb-dmac: Fix residue after the commit 24461d9792c2

[Vinod Koul <vkoul@kernel.org>]

On 21-05-20, 20:46, Yoshihiro Shimoda wrote:
> This driver assumed that freed descriptors have "done_cookie".
> But, after the commit 24461d9792c2 ("dmaengine: virt-dma: Fix
> access after free in vchan_complete()"), since the desc is freed
> after callback function was called, this driver could not
> match any done_cookie when a client driver (renesas_usbhs driver)
> calls dmaengine_tx_status() in the callback function.

Hmmm, I am not sure about this, why should we try to match! cookie is
monotonically increasing number so if you see that current cookie
completed is > requested you should return DMA_COMPLETE

The below case of checking residue should not even get executed

> So, add to check both descriptor types (freed and got) to fix
> the issue.
> 
> Reported-by: Hien Dang <hien.dang.eb@renesas.com>
> Fixes: 24461d9792c2 ("dmaengine: virt-dma: Fix access after free in vchan_complete()")
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  drivers/dma/sh/usb-dmac.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma/sh/usb-dmac.c b/drivers/dma/sh/usb-dmac.c
> index b218a01..c0adc1c8 100644
> --- a/drivers/dma/sh/usb-dmac.c
> +++ b/drivers/dma/sh/usb-dmac.c
> @@ -488,16 +488,17 @@ static u32 usb_dmac_chan_get_residue_if_complete(struct usb_dmac_chan *chan,
>  						 dma_cookie_t cookie)
>  {
>  	struct usb_dmac_desc *desc;
> -	u32 residue = 0;
>  
> +	list_for_each_entry_reverse(desc, &chan->desc_got, node) {
> +		if (desc->done_cookie == cookie)
> +			return desc->residue;
> +	}
>  	list_for_each_entry_reverse(desc, &chan->desc_freed, node) {
> -		if (desc->done_cookie == cookie) {
> -			residue = desc->residue;
> -			break;
> -		}
> +		if (desc->done_cookie == cookie)
> +			return desc->residue;
>  	}
>  
> -	return residue;
> +	return 0;
>  }
>  
>  static u32 usb_dmac_chan_get_residue(struct usb_dmac_chan *chan,
> -- 
> 2.7.4

-- 
~Vinod