Re: [PATCH] USB: core: of: Check device_node before parsing in usb_of_get_child_node()

[Vaibhav Hiremath]

On Monday 15 August 2016 11:41 AM, Greg KH wrote:
> On Mon, Aug 15, 2016 at 11:31:10AM -0700, Vaibhav Hiremath wrote:
> > In case of HUB devices connected to USB ports, we may not have DT
> > node representing it inside USB, and when devices connected to hub
> > gets enumerated, call to usb_of_get_child_node() leads to NULL pointer
> > dereference.
> Really?  That seems messed up.

unfortunately yes :)

> > In the usecase we have, where EHCI port is connected to USB HUB
> > device, and downward ports of HUB are connected to further USB
> > devices. When those devices gets enumerated, in order,
> >   1. USB HUB ->
> > 	-> Call to usb_of_get_child_node() is OK, as
> > 	parent->dev.of_node is pointing to host node.
> >   2. Devices connected to downward port of USB HUB
> >   	-> Call to usb_of_get_child_node() leads to NULL
> > 	pointer dereference as parent->dev.of_node = NULL,
> > 	as USB HUB DTS node may be empty.
> Why is the hub DTS empty?  Shouldn't that be the fix here?

Because HUB can be enumerated dynamically and one possible
reason could be you don't need to do anything to bring up HUB.
May be one of following could be the reason -

 1. HUB automatically comes up on power ON, and USB host enumerates it.
   There is no control path for HUB

 2. HUB has different control path, in our case it is over I2C.
   So HUB configuration and bringup happens as part of I2C client driver.



So you may not need DTS for HUB as a child node inside USB host.
What I am trying to say here is,


&usb_ehci {
    ...

    status = "ok";
};

This would enumerate HUB first, and then devices connected to HUB, right?
So this will lead to kernel crash.



Reference DTS with HUB and downward devices -

&usb_ehci {
    status = "ok";

    usb_hub: usb_hub {
        compatible = "usbxxxx";
        reg = <1>;

        usb_dev: usb_dev {
            compatible = "usbxxxx";
            reg = <1>;

            ...
        };
   };
};


Thanks,
Vaibhav