[U-Boot] [U-Boot, v1] dm: usb: Prevent NULL hub in usb_device_info()

[Hans de Goede]

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 at web.de>
> Reviewed-by: Simon Glass <sjg at 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:

1) 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/fecf9576247c9423a20570057a1de59f1dbc2da1
https://github.com/jwrdegoede/u-boot-sunxi/commit/d61f186f494062539418ac9da800a354258a4ad2

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.

Regards,

Hans