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

[Hans de Goede]

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 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.

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