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@xxxxxxxxxxx>
> Fixes: 24461d9792c2 ("dmaengine: virt-dma: Fix access after free in vchan_complete()")
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> ---
> 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
