On Fri, Aug 14, 2015 at 4:44 PM, Marek Vasut <ma...@denx.de> wrote:
> On Friday, August 14, 2015 at 03:00:31 PM, Sergei Temerkhanov wrote:
>> This may happen when, for example, one tries to get a single binary for
>> similar systems where those controllers may or may not present.
>
> Please DO STOP TOP-POSTING, I mentioned it already, it is really not helpful.
>
Sorry about that, had to struggle with Gmail on my tablet.
> I think your patch is missing one small bit in usb_lowlevel_init() in xhci.c
> In case xhci_lowlevel_init() fails and thus the controller is NOT inited, the
> usb_lowlevel_stop() will still try to unregister such controller, which will
> likely fail.
>
> You might want to add a check which sets the HCOR and HCCR to NULL if the
> xhci_lowlevel_init() fails. What do you think ?
Might be useful - it's more about error handling rather then core
configurations.
For the latter xhci_hcd_init() seems to be an appropriate place to set
hccr and hcor
to NULL.
For error handling, it might be useful to modify usb_low_level_init like this:
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index a6c6659..f8e2d70 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1079,6 +1079,11 @@ int usb_lowlevel_init(int index, enum
usb_init_type init, void **controller)
*controller = &xhcic[index];
+ if (ret) {
+ ctrl->hccr = NULL;
+ ctrl->hcor = NULL;
+ }
+
return ret;
}
>
>> Regards,
>> Sergey
>>
>> On Fri, Aug 14, 2015 at 3:15 PM, Marek Vasut <ma...@denx.de> wrote:
>> > On Friday, August 14, 2015 at 02:13:06 PM, Sergey Temerkhanov wrote:
>> > > This patch fixes a potential NULL pointer dereference arising on
>> > > non-present/non-initialized xHCI controllers
>> >
>> > Hi,
>> >
>> > can you please explain how can such a condition even happen ?
>> > I believe the hcor must always be inited at that point. What
>> > is the condition which triggers this ?
>> >
>> > > Signed-off-by: Sergey Temerkhanov <s.temerkha...@gmail.com>
>> > > Signed-off-by: Radha Mohan Chintakuntla <rchintakun...@cavium.com>
>> > > ---
>> > >
>> > > drivers/usb/host/xhci.c | 10 ++++++----
>> > > 1 file changed, 6 insertions(+), 4 deletions(-)
>> > >
>> > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> > > index 0b09643..a6c6659 100644
>> > > --- a/drivers/usb/host/xhci.c
>> > > +++ b/drivers/usb/host/xhci.c
>> > > @@ -199,7 +199,7 @@ int xhci_reset(struct xhci_hcor *hcor)
>> > >
>> > > int ret;
>> > >
>> > > /* Halting the Host first */
>> > >
>> > > - debug("// Halt the HC\n");
>> > > + debug("// Halt the HC: %p\n", hcor);
>> > >
>> > > state = xhci_readl(&hcor->or_usbsts) & STS_HALT;
>> > > if (!state) {
>> > >
>> > > cmd = xhci_readl(&hcor->or_usbcmd);
>> > >
>> > > @@ -1093,9 +1093,11 @@ int usb_lowlevel_stop(int index)
>> > >
>> > > {
>> > >
>> > > struct xhci_ctrl *ctrl = (xhcic + index);
>> > >
>> > > - xhci_lowlevel_stop(ctrl);
>> > > - xhci_hcd_stop(index);
>> > > - xhci_cleanup(ctrl);
>> > > + if (ctrl->hcor) {
>> > > + xhci_lowlevel_stop(ctrl);
>> > > + xhci_hcd_stop(index);
>> > > + xhci_cleanup(ctrl);
>> > > + }
>> > >
>> > > return 0;
>> > >
>> > > }
>> >
>> > Best regards,
>> > Marek Vasut
>
> Best regards,
> Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
