xhci: Don't add a virt_dev to the devs array before it's fully allocated
authorMathias Nyman <mathias.nyman@linux.intel.com>
Fri, 8 Dec 2017 16:10:05 +0000 (18:10 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 20 Dec 2017 09:10:19 +0000 (10:10 +0100)
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@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/usb/host/xhci-mem.c

index 97f30eb7dac0d0a1daf636efe22d15ec6f545c0f..ccdc971283d092189eb52c3a746fcf107180454b 100644 (file)
@@ -983,10 +983,9 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id,
                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);
@@ -1027,9 +1026,17 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id,
 
        trace_xhci_alloc_virt_device(dev);
 
+       xhci->devs[slot_id] = dev;
+
        return 1;
 fail:
-       xhci_free_virt_device(xhci, slot_id);
+
+       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;
 }