linux-kernel - [114/165] xhci: Fix bug in control transfer cancellation.

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: <20110601081158.543756140@blue.kroah.org>
Date:	Wed, 01 Jun 2011 17:11:24 +0900
From:	Greg KH <gregkh@...e.de>
To:	linux-kernel@...r.kernel.org, stable@...nel.org
Cc:	stable-review@...nel.org, torvalds@...ux-foundation.org,
	akpm@...ux-foundation.org, alan@...rguk.ukuu.org.uk,
	Sarah Sharp <sarah.a.sharp@...ux.intel.com>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Greg Kroah-Hartman <gregkh@...e.de>
Subject: [114/165] xhci: Fix bug in control transfer cancellation.

2.6.39-stable review patch.  If anyone has any objections, please let us know.

------------------
Content-Length: 3172
Lines: 80

From: Sarah Sharp <sarah.a.sharp@...ux.intel.com>

commit 3abeca998a44205cfd837fa0bf1f7c24f8294acb upstream.

When the xHCI driver attempts to cancel a transfer, it issues a Stop
Endpoint command and waits for the host controller to indicate which TRB
it was in the middle of processing.  The host will put an event TRB with
completion code COMP_STOP on the event ring if it stops on a control
transfer TRB (or other types of transfer TRBs).  The ring handling code
is supposed to set ep->stopped_trb to the TRB that the host stopped on
when this happens.

Unfortunately, there is a long-standing bug in the control transfer
completion code.  It doesn't actually check to see if COMP_STOP is set
before attempting to process the transfer based on which part of the
control TD completed.  So when we get an event on the data phase of the
control TRB with COMP_STOP set, it thinks it's a normal completion of
the transfer and doesn't set ep->stopped_td or ep->stopped_trb.

When the ring handling code goes on to process the completion of the Stop
Endpoint command, it sees that ep->stopped_trb is not a part of the TD
it's trying to cancel.  It thinks the hardware has its enqueue pointer
somewhere further up in the ring, and thinks it's safe to turn the control
TRBs into no-op TRBs.  Since the hardware was in the middle of the control
TRBs to be cancelled, the proper software behavior is to issue a Set TR
dequeue pointer command.

It turns out that the NEC host controllers can handle active TRBs being
set to no-op TRBs after a stop endpoint command, but other host
controllers have issues with this out-of-spec software behavior.  Fix this
behavior.

This patch should be backported to kernels as far back as 2.6.31, but it
may be a bit challenging, since process_ctrl_td() was introduced in some
refactoring done in 2.6.36, and some endian-safe patches added in 2.6.40
that touch the same lines.

Signed-off-by: Sarah Sharp <sarah.a.sharp@...ux.intel.com>
Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...e.de>

---
 drivers/usb/host/xhci-ring.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1632,6 +1632,9 @@ static int process_ctrl_td(struct xhci_h
 		else
 			*status = 0;
 		break;
+	case COMP_STOP_INVAL:
+	case COMP_STOP:
+		return finish_td(xhci, td, event_trb, event, ep, status, false);
 	default:
 		if (!xhci_requires_manual_halt_cleanup(xhci,
 					ep_ctx, trb_comp_code))
@@ -1676,15 +1679,12 @@ static int process_ctrl_td(struct xhci_h
 			}
 		} else {
 		/* Maybe the event was for the data stage? */
-			if (trb_comp_code != COMP_STOP_INVAL) {
-				/* We didn't stop on a link TRB in the middle */
-				td->urb->actual_length =
-					td->urb->transfer_buffer_length -
-					TRB_LEN(event->transfer_len);
-				xhci_dbg(xhci, "Waiting for status "
-						"stage event\n");
-				return 0;
-			}
+			td->urb->actual_length =
+				td->urb->transfer_buffer_length -
+				TRB_LEN(le32_to_cpu(event->transfer_len));
+			xhci_dbg(xhci, "Waiting for status "
+					"stage event\n");
+			return 0;
 		}
 	}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/