Hi,
On 02-07-16 23:13, Hans de Goede wrote:
Hi,
On 29-06-16 10:43, Bernhard Nortmann wrote:
This patch modifies the usb_device_info() function to enumerate active USB hubs by iterating UCLASS_USB_HUB directly.
The previous code used UCLASS_USB nodes instead and retrieved their first child node, expecting it to be a hub. However, it did not protect against retrieving (and dereferencing) a NULL pointer this way.
Depending on the available USB hardware, this might happen easily. For example the USB controller on sun7i-a20 has top-level OHCI nodes that won't have any children as long as no USB1-only peripheral is attached. ("usb tree" will only show EHCI nodes.)
The failure can also be demonstrated with U-Boot's sandbox architecture, by simply enabling the inactive "usb_2" node in test.dts. This creates a similar situation, where the existing usb_device_info() implementation would crash (segfault) when issuing a "usb info" command.
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de Reviewed-by: Simon Glass sjg@chromium.org
cmd/usb.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/cmd/usb.c b/cmd/usb.c index 58d9db2..3146f54 100644 --- a/cmd/usb.c +++ b/cmd/usb.c @@ -602,18 +602,13 @@ static void show_info(struct udevice *dev)
static int usb_device_info(void) {
- struct udevice *bus;
- for (uclass_first_device(UCLASS_USB, &bus);
bus;uclass_next_device(&bus)) {struct udevice *hub;
- struct udevice *hub;
device_find_first_child(bus, &hub);if (device_get_uclass_id(hub) == UCLASS_USB_HUB &&device_active(hub)) {
- for (uclass_first_device(UCLASS_USB_HUB, &hub);
We really want to check for bus here not hub, there are 2 problems with checking for hubs:
- show_info recurses into child hubs, so going over all
hubs will cause any nested hubs to get printed twice (I think) 2) on some otg ports in host-mode there is no emulated root-hub in u-boot, so this will skip printing of any device attached to them
Besides this there are several other subtleties missed here wrt iterating over usb-root-devs.
I've spend quite some time getting this all right for "usb tree" but I somehow missed that "usb info" needed the same treatment.
I've prepared 2 patches which re-use the "usb tree" code rather then re-inventing the wheel again:
https://github.com/jwrdegoede/u-boot-sunxi/commit/fecf9576247c9423a20570057a... https://github.com/jwrdegoede/u-boot-sunxi/commit/d61f186f494062539418ac9da8...
As an added bonus these actually result in removing some code instead of duplicating it.
Note these are only compile tested yet. I'll properly test them tomorrow and then submit them officially.
Ok I've just completed a battery of tests and these 2 look good, I'll submit them upstream right after this mail.
Regards,
Hans