linux-kernel - [RFC PATCH] xhci: Fix memory use after free in xhci_free_virt

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-next>] [day] [month] [year] [list]

Date:   Tue, 15 Nov 2016 12:36:39 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Mathias Nyman <mathias.nyman@...el.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        dianders@...omium.org, briannorris@...omium.org, mka@...omium.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        Guenter Roeck <linux@...ck-us.net>
Subject: [RFC PATCH] xhci: Fix memory use after free in xhci_free_virt_device

The following use-after-free reports were seen on resume with a specific
USB hub.

BUG: KASAN: use-after-free in xhci_free_virt_device+0x8c/0x21c
	at addr ffffffc0cc1a2eb0
BUG: KASAN: use-after-free in xhci_update_tt_active_eps+0x9c/0xdc
	at addr ffffffc0cc1a2eb0

Relevant traceback for the first case is:

xhci_free_virt_device+0x8c/0x21c
xhci_mem_cleanup+0x294/0x81c
xhci_resume+0x410/0x618
xhci_plat_resume+0x54/0x74
platform_pm_resume+0x74/0x84

which points to the following code in xhci_free_virt_device().

	if (dev->tt_info)
                old_active_eps = dev->tt_info->active_eps;

Problem with this code is that xhci_mem_cleanup() cleans up devices
starting with slot 1, and dev->tt_info for a device with higher slot
number can point back to the tt_info associated with device 1.
In lsusb, this looks as follows.

/:  Bus 05.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 480M
    |__ Port 1: Dev 4, If 0, Class=Hub, Driver=hub/4p, 480M
        |__ Port 3: Dev 7, If 0, Class=Vendor Specific Class, Driver=, 12M

When the higher-numbered device is cleared, it tries to access the already
released tt_info from slot 1 to get the value of old_active_eps.

The problem is not seen with all USB hubs since not all USB hubs require
the cleanup handling in xhci_resume().

Signed-off-by: Guenter Roeck <linux@...ck-us.net>
---
Marked as RFC because I don't really like this fix at all and would prefer
a different solution.

It might be possible, for example, to change the code in xhci_mem_cleanup()
from
	for (i = 1; i < MAX_HC_SLOTS; ++i)
		xhci_free_virt_device(xhci, i);
to
	for (i = MAX_HC_SLOTS - 1; i >= 1; i--)
		xhci_free_virt_device(xhci, i);
if it is guaranteed that tt_info never points to a device with higher slot
number.

 drivers/usb/host/xhci-mem.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 6afe32381209..97a61c57a2ed 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -873,8 +873,14 @@ static void xhci_free_tt_info(struct xhci_hcd *xhci,
 	list_for_each_entry_safe(tt_info, next, tt_list_head, tt_list) {
 		/* Multi-TT hubs will have more than one entry */
 		if (tt_info->slot_id == slot_id) {
+			int i;
+
 			slot_found = true;
 			list_del(&tt_info->tt_list);
+			for (i = 0; i < MAX_HC_SLOTS; i++)
+				if (xhci->devs[i] &&
+				    xhci->devs[i]->tt_info == tt_info)
+					xhci->devs[i]->tt_info = NULL;
 			kfree(tt_info);
 		} else if (slot_found) {
 			break;
-- 
2.5.0