From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Sun, 16 Aug 2015 18:55:47 +0200
Subject: [U-Boot] [PATCH v2] usb: xhci: Fix a potential NULL pointer
	dereference
In-Reply-To: <CAPEA6dZOYiL=uHWTTURmfSEd4OAXt_+eZRAG9bVA+UWDjsN_hQ@mail.gmail.com>
References: <1439565249-12581-1-git-send-email-s.temerkhanov@gmail.com>
 <201508142246.39402.marex@denx.de>
 <CAPEA6dZOYiL=uHWTTURmfSEd4OAXt_+eZRAG9bVA+UWDjsN_hQ@mail.gmail.com>
Message-ID: <201508161855.47407.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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@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@gmail.com>
> >> Signed-off-by: Radha Mohan Chintakuntla <rchintakuntla@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!