linux-kernel - Re: [PATCH v4 2/6] usb: dwc3: gadget: cancel requests instead of release after missed isoc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 22 Oct 2022 08:35:43 -0500
From:   Jeff Vanhoof <jdv1029@...il.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Dan Vacura <w36195@...orola.com>, linux-usb@...r.kernel.org,
        Daniel Scally <dan.scally@...asonboard.com>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Jeff Vanhoof <qjv001@...orola.com>, stable@...r.kernel.org,
        Jonathan Corbet <corbet@....net>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Felipe Balbi <balbi@...nel.org>,
        Paul Elder <paul.elder@...asonboard.com>,
        Michael Grzeschik <m.grzeschik@...gutronix.de>,
        linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH v4 2/6] usb: dwc3: gadget: cancel requests instead of
 release after missed isoc

Hi Greg,

On Sat, Oct 22, 2022 at 01:31:24PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Oct 18, 2022 at 04:50:38PM -0500, Dan Vacura wrote:
> > From: Jeff Vanhoof <qjv001@...orola.com>
> > 
> > arm-smmu related crashes seen after a Missed ISOC interrupt when
> > no_interrupt=1 is used. This can happen if the hardware is still using
> > the data associated with a TRB after the usb_request's ->complete call
> > has been made.  Instead of immediately releasing a request when a Missed
> > ISOC interrupt has occurred, this change will add logic to cancel the
> > request instead where it will eventually be released when the
> > END_TRANSFER command has completed. This logic is similar to some of the
> > cleanup done in dwc3_gadget_ep_dequeue.
> > 
> > Fixes: 6d8a019614f3 ("usb: dwc3: gadget: check for Missed Isoc from event status")
> > Cc: <stable@...r.kernel.org>
> > Signed-off-by: Jeff Vanhoof <qjv001@...orola.com>
> > Co-developed-by: Dan Vacura <w36195@...orola.com>
> > Signed-off-by: Dan Vacura <w36195@...orola.com>
> > ---
> > V1 -> V3:
> > - no change, new patch in series
> > V3 -> V4:
> > - no change
> 
> I need an ack from the dwc3 maintainer before I can take this one.
> 
> thanks,
> 
> greg k-h

Thinh has rejected this version of the patch. He has provided an alternative
implementation which has been testing well for us so far. Either Thinh or Dan
will formalize this patch within the next few days.
The latest proposed changes are:

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index dfaf9ac24c4f..50287437d6de 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3195,6 +3195,9 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
        if (event->status & DEPEVT_STATUS_SHORT && !chain)
                return 1;
 
+       if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC && !chain)
+               return 1;
+
        if ((trb->ctrl & DWC3_TRB_CTRL_IOC) ||
            (trb->ctrl & DWC3_TRB_CTRL_LST))
                return 1;
@@ -3211,6 +3214,7 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
        struct scatterlist *s;
        unsigned int num_queued = req->num_queued_sgs;
        unsigned int i;
+       bool missed_isoc = false;
        int ret = 0;
 
        for_each_sg(sg, s, num_queued, i) {
@@ -3219,12 +3223,18 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
                req->sg = sg_next(s);
                req->num_queued_sgs--;
 
+               if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC)
+                       missed_isoc = true;
+
                ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
                                trb, event, status, true);
                if (ret)
                        break;
        }
 
+       if (missed_isoc)
+               ret = 1;
+
        return ret;
 }


Thanks,
Jeff