linux-kernel - [77/84] xhci: Avoid NULL pointer deref when host dies.

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: <lsq.1375276989.587854424@decadent.org.uk>
Date:	Wed, 31 Jul 2013 15:23:09 +0200
From:	Ben Hutchings <ben@...adent.org.uk>
To:	linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:	akpm@...ux-foundation.org,
	"Sarah Sharp" <sarah.a.sharp@...ux.intel.com>,
	"Vincent Thiele" <vincentthiele@...il.com>
Subject: [77/84] xhci: Avoid NULL pointer deref when host dies.

3.2.50-rc1 review patch.  If anyone has any objections, please let me know.

------------------

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

commit 203a86613fb3bf2767335659513fa98563a3eb71 upstream.

When the host controller fails to respond to an Enable Slot command, and
the host fails to respond to the register write to abort the command
ring, the xHCI driver will assume the host is dead, and call
usb_hc_died().

The USB device's slot_id is still set to zero, and the pointer stored at
xhci->devs[0] will always be NULL.  The call to xhci_check_args in
xhci_free_dev should have caught the NULL virt_dev pointer.

However, xhci_free_dev is designed to free the xhci_virt_device
structures, even if the host is dead, so that we don't leak kernel
memory.  xhci_free_dev checks the return value from the generic
xhci_check_args function.  If the return value is -ENODEV, it carries on
trying to free the virtual device.

The issue is that xhci_check_args looks at the host controller state
before it looks at the xhci_virt_device pointer.  It will return -ENIVAL
because the host is dead, and xhci_free_dev will ignore the return
value, and happily dereference the NULL xhci_virt_device pointer.

The fix is to make sure that xhci_check_args checks the xhci_virt_device
pointer before it checks the host state.

See https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1203453 for
further details.  This patch doesn't solve the underlying issue, but
will ensure we don't see any more NULL pointer dereferences because of
the issue.

This patch should be backported to kernels as old as 3.1, that
contain the commit 7bd89b4017f46a9b92853940fd9771319acb578a "xhci: Don't
submit commands or URBs to halted hosts."

Signed-off-by: Sarah Sharp <sarah.a.sharp@...ux.intel.com>
Reported-by: Vincent Thiele <vincentthiele@...il.com>
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
 drivers/usb/host/xhci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1153,9 +1153,6 @@ static int xhci_check_args(struct usb_hc
 	}

 	xhci = hcd_to_xhci(hcd);
-	if (xhci->xhc_state & XHCI_STATE_HALTED)
-		return -ENODEV;
-
 	if (check_virt_dev) {
 		if (!udev->slot_id || !xhci->devs[udev->slot_id]) {
 			printk(KERN_DEBUG "xHCI %s called with unaddressed "
@@ -1171,6 +1168,9 @@ static int xhci_check_args(struct usb_hc
 		}
 	}

+	if (xhci->xhc_state & XHCI_STATE_HALTED)
+		return -ENODEV;
+
 	return 1;
 }

--
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/