linux-kernel - Re: [ 45/82] USB: EHCI: dont check DMA values in QH overlays

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]

Message-ID: <1363662540.3937.366.camel@deadeye.wl.decadent.org.uk>
Date:	Tue, 19 Mar 2013 03:09:00 +0000
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	stable@...r.kernel.org, akpm@...ux-foundation.org,
	Joseph Salisbury <joseph.salisbury@...onical.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [ 45/82] USB: EHCI: dont check DMA values in QH overlays

On Mon, 2013-03-18 at 04:22 +0000, Ben Hutchings wrote:
> 3.2-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Alan Stern <stern@...land.harvard.edu>
> 
> commit feca7746d5d9e84b105a613b7f3b6ad00d327372 upstream.
> 
> This patch (as1661) fixes a rather obscure bug in ehci-hcd.  In a
> couple of places, the driver compares the DMA address stored in a QH's
> overlay region with the address of a particular qTD, in order to see
> whether that qTD is the one currently being processed by the hardware.
> (If it is then the status in the QH's overlay region is more
> up-to-date than the status in the qTD, and if it isn't then the
> overlay's value needs to be adjusted when the QH is added back to the
> active schedule.)
> 
> However, DMA address in the overlay region isn't always valid.  It
> sometimes will contain a stale value, which may happen by coincidence
> to be equal to a qTD's DMA address.  Instead of checking the DMA
> address, we should check whether the overlay region is active and
> valid.  The patch tests the ACTIVE bit in the overlay, and clears this
> bit when the overlay becomes invalid (which happens when the
> currently-executing URB is unlinked).
> 
> This is the second part of a fix for the regression reported at:
> 
> 	https://bugs.launchpad.net/bugs/1088733

Alan, the first part (commit 6402c796d3b4 aka as1660) didn't apply and I
couldn't see how to adapt it for 3.2.  Does this second part have any
value without the first?  Or, if you could provide a backport of the
first part, that would be very much appreciated.

Ben.

> Signed-off-by: Alan Stern <stern@...land.harvard.edu>
> Reported-by: Joseph Salisbury <joseph.salisbury@...onical.com>
> Reported-and-tested-by: Stephen Thirlwall <sdt@...com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
> ---
>  drivers/usb/host/ehci-q.c |   18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> --- a/drivers/usb/host/ehci-q.c
> +++ b/drivers/usb/host/ehci-q.c
> @@ -135,7 +135,7 @@ qh_refresh (struct ehci_hcd *ehci, struc
>  		 * qtd is updated in qh_completions(). Update the QH
>  		 * overlay here.
>  		 */
> -		if (cpu_to_hc32(ehci, qtd->qtd_dma) == qh->hw->hw_current) {
> +		if (qh->hw->hw_token & ACTIVE_BIT(ehci)) {
>  			qh->hw->hw_qtd_next = qtd->hw_next;
>  			qtd = NULL;
>  		}
> @@ -444,11 +444,19 @@ qh_completions (struct ehci_hcd *ehci, s
>  			else if (last_status == -EINPROGRESS && !urb->unlinked)
>  				continue;
>  
> -			/* qh unlinked; token in overlay may be most current */
> -			if (state == QH_STATE_IDLE
> -					&& cpu_to_hc32(ehci, qtd->qtd_dma)
> -						== hw->hw_current) {
> +			/*
> +			 * If this was the active qtd when the qh was unlinked
> +			 * and the overlay's token is active, then the overlay
> +			 * hasn't been written back to the qtd yet so use its
> +			 * token instead of the qtd's.  After the qtd is
> +			 * processed and removed, the overlay won't be valid
> +			 * any more.
> +			 */
> +			if (state == QH_STATE_IDLE &&
> +					qh->qtd_list.next == &qtd->qtd_list &&
> +					(hw->hw_token & ACTIVE_BIT(ehci))) {
>  				token = hc32_to_cpu(ehci, hw->hw_token);
> +				hw->hw_token &= ~ACTIVE_BIT(ehci);
>  
>  				/* An unlink may leave an incomplete
>  				 * async transaction in the TT buffer.
> 


-- 
Ben Hutchings
When you say `I wrote a program that crashed Windows', people just stare ...
and say `Hey, I got those with the system, *for free*'. - Linus Torvalds

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