[coreboot-gerrit] New patch to review for coreboot: libpayload: usb: don't prematurely free the usb device

[Aaron Durbin (adurbin@chromium.org)]

Aaron Durbin (adurbin at chromium.org) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/11173

-gerrit

commit 3568d444941ddf0fd066755f7bbe344790273aa8
Author: Aaron Durbin <adurbin at chromium.org>
Date:   Fri Jul 31 17:08:00 2015 -0500

    libpayload: usb: don't prematurely free the usb device
    
    Before the controller's destroy_device() could interrogate
    the usbdev_t object usb_detach_device() was freeing and
    NULLing out the pointer. That results in all callers who
    needed that object to start accessing random bits of memory.
    
    This eventually led into free()ing memory it shouldn't which
    corrupted the allocator's state. Eventually, all forward
    progress was lost by way of a single ended linked list
    turning into a circular list.
    
    The culprit seems to be a bad merge in commit e00ba21.
    
    BUG=chrome-os-partner:43419
    BRANCH=None
    TEST=Can boot into OS now w/o "hanging" on glados.
    
    Original-Change-Id: I86dcaa1dbaf112ac6782e90dad40f0932f273a1f
    Original-Signed-off-by: Aaron Durbin <adurbin at chromium.org>
    Original-Reviewed-on: https://chromium-review.googlesource.com/290048
    Original-Reviewed-by: Julius Werner <jwerner at chromium.org>
    
    Change-Id: I9135eb0f798bf7dbeccc7a033c3f8471720a0de5
    Signed-off-by: Aaron Durbin <adurbin at chromium.org>
---
 payloads/libpayload/drivers/usb/usb.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/payloads/libpayload/drivers/usb/usb.c b/payloads/libpayload/drivers/usb/usb.c
index 69d1c39..ffbe005 100644
--- a/payloads/libpayload/drivers/usb/usb.c
+++ b/payloads/libpayload/drivers/usb/usb.c
@@ -605,10 +605,12 @@ usb_detach_device(hci_t *controller, int devno)
 	   been called yet by the usb class driver */
 	if (controller->devices[devno]) {
 		controller->devices[devno]->destroy (controller->devices[devno]);
-		free(controller->devices[devno]);
-		controller->devices[devno] = NULL;
 		if (controller->destroy_device)
 			controller->destroy_device(controller, devno);
+		/* Tear down the device itself *after* destroy_device()
+		 * has had a chance to interoogate it. */
+		free(controller->devices[devno]);
+		controller->devices[devno] = NULL;
 	}
 }