linux-kernel - [PATCH 3.16 081/254] xhci: Don't add a virt_dev to the devs array before it's fully allocated

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.1519831218.520216187@decadent.org.uk>
Date:   Wed, 28 Feb 2018 15:20:18 +0000
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org,
        "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
        "Mathias Nyman" <mathias.nyman@...ux.intel.com>
Subject: [PATCH 3.16 081/254] xhci: Don't add a virt_dev to the devs array
 before it's fully allocated

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

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

From: Mathias Nyman <mathias.nyman@...ux.intel.com>

commit 5d9b70f7d52eb14bb37861c663bae44de9521c35 upstream.

Avoid null pointer dereference if some function is walking through the
devs array accessing members of a new virt_dev that is mid allocation.

Add the virt_dev to xhci->devs[i] _after_ the virt_device and all its
members are properly allocated.

issue found by KASAN: null-ptr-deref in xhci_find_slot_id_by_port

"Quick analysis suggests that xhci_alloc_virt_device() is not mutex
protected. If so, there is a time frame where xhci->devs[slot_id] is set
but not fully initialized. Specifically, xhci->devs[i]->udev can be NULL."

Signed-off-by: Mathias Nyman <mathias.nyman@...ux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
[bwh: Backported to 3.16: There is an extra failure path, so we may need to
 free dev->eps[0].ring] 
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1018,10 +1018,9 @@ int xhci_alloc_virt_device(struct xhci_h
 		return 0;
 	}
 
-	xhci->devs[slot_id] = kzalloc(sizeof(*xhci->devs[slot_id]), flags);
-	if (!xhci->devs[slot_id])
+	dev = kzalloc(sizeof(*dev), flags);
+	if (!dev)
 		return 0;
-	dev = xhci->devs[slot_id];
 
 	/* Allocate the (output) device context that will be used in the HC. */
 	dev->out_ctx = xhci_alloc_container_ctx(xhci, XHCI_CTX_TYPE_DEVICE, flags);
@@ -1069,9 +1068,19 @@ int xhci_alloc_virt_device(struct xhci_h
 		 &xhci->dcbaa->dev_context_ptrs[slot_id],
 		 le64_to_cpu(xhci->dcbaa->dev_context_ptrs[slot_id]));
 
+	xhci->devs[slot_id] = dev;
+
 	return 1;
 fail:
-	xhci_free_virt_device(xhci, slot_id);
+
+	if (dev->eps[0].ring)
+		xhci_ring_free(xhci, dev->eps[0].ring);
+	if (dev->in_ctx)
+		xhci_free_container_ctx(xhci, dev->in_ctx);
+	if (dev->out_ctx)
+		xhci_free_container_ctx(xhci, dev->out_ctx);
+	kfree(dev);
+
 	return 0;
 }