linux-kernel - Re: [PATCH] usb: dwc3: gadget: check link trb after free

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:	Thu, 15 May 2014 10:37:57 -0500
From:	Felipe Balbi <balbi@...com>
To:	Zhuang Jin Can <jin.can.zhuang@...el.com>
CC:	Felipe Balbi <balbi@...com>, USB list <linux-usb@...r.kernel.org>,
	<linux-omap@...r.kernel.org>,
	Kernel development list <linux-kernel@...r.kernel.org>,
	<david.a.cohen@...ux.intel.com>, <Yuan@...el.com>,
	Hang <hang.yuan@...el.com>, <Zhuang@...el.com>, <Li@...el.com>,
	Jiebing <jiebing.li@...el.com>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Subject: Re: [PATCH] usb: dwc3: gadget: check link trb after free_slot is
 increased

Hi

On Fri, May 16, 2014 at 05:57:57AM +0800, Zhuang Jin Can wrote:
> In ISOC transfers, when free_slot points to the last TRB (i.e. Link
> TRB), and all queued requests meet Missed Interval Isoc error, busy_slot
> points to trb0.
> 	busy_slot->trb0
> 		   trb1
> 		   ...
> 	free_slot->trb31(Link TRB)
> 
> After end transfer and receiving the XferNotReady event, trb_left is
> caculated as 1 which is wrong, and no TRB will be primed to the
> endpoint.
> 
> The root cause is free_slot is not increased the same way as busy_slot.
> When busy_slot is increased by one, it checks if points to a link TRB
> after increasement, but free_slot checks it before increasement.
> free_slot should behave the same as busy_slot to make the trb_left
> caculation correct.
> 
> Signed-off-by: Zhuang Jin Can <jin.can.zhuang@...el.com>
> Signed-off-by: Jiebing Li <jiebing.li@...el.com>
> ---
>  drivers/usb/dwc3/gadget.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 54da8c8..2ebe82b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -828,10 +828,6 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
>  			length, last ? " last" : "",
>  			chain ? " chain" : "");
>  
> -	/* Skip the LINK-TRB on ISOC */
> -	if (((dep->free_slot & DWC3_TRB_MASK) == DWC3_TRB_NUM - 1) &&
> -			usb_endpoint_xfer_isoc(dep->endpoint.desc))
> -		dep->free_slot++;
>  
>  	trb = &dep->trb_pool[dep->free_slot & DWC3_TRB_MASK];

I have a feeling this has a negative side effect of letting us use the
link TRB for data transfer... I mean, if we don't increment free_slot
before accessing our trb_pool, we have no way to skip link trb on this
access here.

How did you find the bug ? do you have good instructions on how to
reproduce it ? How did you test the patch and for how long ?

cheers

-- 
balbi

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