[U-Boot] [PATCH v2] usb: xhci: Fix a potential NULL pointer dereference

[Sergei Temerkhanov]

On Sun, Aug 16, 2015 at 7:55 PM, Marek Vasut <marex at denx.de> wrote:
> On Saturday, August 15, 2015 at 12:28:10 AM, Sergei Temerkhanov wrote:
>> On Fri, Aug 14, 2015 at 11:46 PM, Marek Vasut <marex at denx.de> wrote:
>> > On Friday, August 14, 2015 at 05:14:09 PM, Sergey Temerkhanov wrote:
>> >> This patch fixes a potential NULL pointer dereference arising on
>> >> non-present/non-initialized xHCI controllers and adds some error
>> >> handling to xHCI code
>> >>
>> >> Signed-off-by: Sergey Temerkhanov <s.temerkhanov at gmail.com>
>> >> Signed-off-by: Radha Mohan Chintakuntla <rchintakuntla at cavium.com>
>> >>
>> >> ---
>> >>
>> >> Changes in v2:
>> >> - Add return value check with setting hccr and hcor to NULL
>> >>
>> >>  drivers/usb/host/xhci.c | 15 +++++++++++----
>> >>  1 file changed, 11 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> >> index 0b09643..f8e2d70 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);
>> >>
>> >> @@ -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;
>> >
>> > Controller should be set to NULL too, for the sake of being completely
>> > precise, don't you think so ?
>>
>> Maybe. Though the only place it's actually used at the moment (there
>> is also some USB gadget stuff
>> which seems to rely on EHCI) passes a pointer to a local variable and
>> checks the return value.
>
> I think it might be even better to shuffle the code around a little, so
> that controller is only set if ret == 0. Can you please do this last
> bit and send a V3 ? I'd like to pick the patch then. Thanks!

Please see the v3 of this patch

Regards,
Sergey