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:   Wed, 05 Dec 2018 11:07:44 +0200
From:   Felipe Balbi <balbi@...nel.org>
To:     Anurag Kumar Vulisha <anurag.kumar.vulisha@...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@...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@...r.kernel.org, linux-kernel@...r.kernel.org,
        v.anuragkumar@...il.com, Thinh Nguyen <thinhn@...opsys.com>,
        Tejas Joglekar <tejas.joglekar@...opsys.com>,
        Ajay Yugalkishore Pandey <APANDEY@...inx.com>,
        Anurag Kumar Vulisha <anurag.kumar.vulisha@...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 <anurag.kumar.vulisha@...inx.com> writes:
> The present code in dwc3_gadget_ep_reclaim_completed_trb() will check
> for IOC/LST bit in the event->status and returns if IOC/LST bit is
> set. This logic doesn't work if multiple TRBs are queued per
> request and the IOC/LST bit is set on the last TRB of that request.
> Consider an example where a queued request has multiple queued TRBs
> and IOC/LST bit is set only for the last TRB. In this case, the Core
> generates XferComplete/XferInProgress events only for the last TRB
> (since IOC/LST are set only for the last TRB). As per the logic in
> dwc3_gadget_ep_reclaim_completed_trb() event->status is checked for
> IOC/LST bit and returns on the first TRB. This makes the remaining
> TRBs left unhandled.
> To aviod this, changed the code to check for IOC/LST bits in both
> event->status & TRB->ctrl. This patch does the same.
>
> Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@...inx.com>
> Reviewed-by: Thinh Nguyen <thinhn@...opsys.com>
> Tested-by: Tejas Joglekar <tejas.joglekar@...opsys.com>
> ---
>  Changes in v7:
> 	1. None
>
>  Changes in v6:
> 	1. None
>
>  Changes in v5:
> 	1. None
>
>  Changes in v4:
> 	1. None
>
>  Changes in v3:
> 	1. None
>
>  Changes in v2:
> 	1. None
> ---
>  drivers/usb/dwc3/gadget.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 9ddc9fd..216179e 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -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.

Can you further describe the situation here? Perhaps share tracepoints
exposing the problem?

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)