[U-Boot] usb: hub enumeration and potential NULL ptr dereference in usb_device_info()

[Bernhard Nortmann]

Starting with commit b19236fd1c1ef289bab9e243ee5b50d658fcac3f I am observing
a breakage of the "usb info" command on my BananaPi (Allwinner A20, sun7i),
while "usb tree" and dm commands ("dm tree", "dm uclass") are fine.
See attached usb-info-breakage.log

Tracing back the error positon from pc and lr registers pointed me to the
device_get_uclass_id() call within usb_device_info(), and suggested the
problem is caused by trying to dereference a NULL struct udevice *hub.

Therefore I added a diagnostic printf and a safeguard to this routine:

diff --git a/cmd/usb.c b/cmd/usb.c
index b83d323..a5e2463 100644
--- a/cmd/usb.c
+++ b/cmd/usb.c
@@ -610,6 +610,12 @@ static int usb_device_info(void)
          struct udevice *hub;

          device_find_first_child(bus, &hub);
+        printf("bus %p, uclass %d, hub %p: ",
+            bus, device_get_uclass_id(bus), hub);
+        if (!hub) {
+            printf("<no hub>\n");
+            continue;
+        }
          if (device_get_uclass_id(hub) == UCLASS_USB_HUB &&
              device_active(hub)) {
              show_info(hub);

And it became apparent that hub actually receives NULL values during the
device enumeration. The safeguard prevented the "data abort" error and got
"usb info" working again - see attached usb-info-fixed.log

I'm not sure why this particular problem didn't manifest earlier and
only now became apparent with the change in SPL header size /
CONFIG_SPL_TEXT_BASE. But it seems that accessing a NULL hub with
device_get_uclass_id() is clearly wrong and should be avoided.
Which brings me to two questions:

1. Is getting a NULL hub value legit when iterating UCLASS_USB devices
    the way that usb_device_info() does? If yes, the code seems to be
    lacking protection against passing it to device_get_uclass_id().

2. Why does usb_device_info() choose to enumerate hubs this way at all?
    If the routine is aiming at UCLASS_USB_HUB - which seems to be the
    purpose of the subsequent device_get_uclass_id(hub) test - and the
    device tree already provides this information (as suggested by the
    output of "dm uclass"), then why not enumerate UCLASS_USB_HUB directly?

Regards, B. Nortmann

-------------- next part --------------
A non-text attachment was scrubbed...
Name: usb-info-breakage.log
Type: application/octet-stream
Size: 3004 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160622/02f353a1/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: usb-info-fixed.log
Type: application/octet-stream
Size: 3166 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160622/02f353a1/attachment-0001.obj>