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

[Marek Vasut]

On Tuesday, August 18, 2015 at 02:16:12 PM, Sergei Temerkhanov wrote:
> 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

Hi,

I saw it, it's OK and I'll pick it shortly. Thanks!

Best regards,
Marek Vasut