linux-kernel - RE: [PATCH v7 09/10] usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl fields

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:   Fri, 07 Dec 2018 08:11:47 +0200
From:   Felipe Balbi <balbi@...nel.org>
To:     Anurag Kumar Vulisha <anuragku@...inx.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Shuah Khan <shuah@...nel.org>,
        Alan Stern <stern@...land.harvard.edu>,
        Johan Hovold <johan@...nel.org>,
        Jaejoong Kim <climbbb.kim@...il.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Roger Quadros <rogerq@...com>,
        Manu Gautam <mgautam@...eaurora.org>,
        "martin.petersen\@oracle.com" <martin.petersen@...cle.com>,
        Bart Van Assche <bvanassche@....org>,
        Mike Christie <mchristi@...hat.com>,
        Matthew Wilcox <willy@...radead.org>,
        Colin Ian King <colin.king@...onical.com>
Cc:     "linux-usb\@vger.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>,
        "v.anuragkumar\@gmail.com" <v.anuragkumar@...il.com>,
        Thinh Nguyen <thinhn@...opsys.com>,
        Tejas Joglekar <tejas.joglekar@...opsys.com>,
        Ajay Yugalkishore Pandey <APANDEY@...inx.com>
Subject: RE: [PATCH v7 09/10] usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl fields


Hi,

Anurag Kumar Vulisha <anuragku@...inx.com> writes:
>>> @@ -2286,7 +2286,12 @@ static int
>>dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
>>>  	if (event->status & DEPEVT_STATUS_SHORT && !chain)
>>>  		return 1;
>>>
>>> -	if (event->status & (DEPEVT_STATUS_IOC | DEPEVT_STATUS_LST))
>>> +	if ((event->status & DEPEVT_STATUS_IOC) &&
>>> +	    (trb->ctrl & DWC3_TRB_CTRL_IOC))
>>> +		return 1;
>>
>>this shouldn't be necessary. According to databook, event->status
>>contains the bits from the completed TRB.  Which means that
>>event->status & IOC will always be equal to trb->ctrl & IOC.
>>
> Thanks for reviewing this patch. Lets consider an example where a request
> has num_sgs > 0 and each sg is mapped to a TRB and the last TRB has the
> IOC bit set. Once the controller is done with the transfer, it  generates 
> XferInProgress for the last TRB (since IOC bit is set). As a part of trb reclaim
> process  dwc3_gadget_ep_reclaim_trb_sg() calls
> dwc3_gadget_ep_reclaim_completed_trb() for req->num_sgs times. Since
> the event already has the IOC bit set, the loop is exited from the loop at the
> very first TRB and the remaining TRBs (mapped to the sglist) are left unhandled.
> To avoid this we modified the code to exit only if both TRB & event has the IOC
> bit set.

Seems like IOC case should just test for chain flag as well:

modified   drivers/usb/dwc3/gadget.c
@@ -2372,7 +2372,7 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
 	if (event->status & DEPEVT_STATUS_SHORT && !chain)
 		return 1;
 
-	if (event->status & DEPEVT_STATUS_IOC)
+	if (event->status & DEPEVT_STATUS_IOC && !chain)
 		return 1;
 
 	return 0;

-- 
balbi