public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2] usb: xhci: Fix a potential NULL pointer dereference
Date: Sun, 16 Aug 2015 18:55:47 +0200	[thread overview]
Message-ID: <201508161855.47407.marex@denx.de> (raw)
In-Reply-To: <CAPEA6dZOYiL=uHWTTURmfSEd4OAXt_+eZRAG9bVA+UWDjsN_hQ@mail.gmail.com>

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!

  reply	other threads:[~2015-08-16 16:55 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-14 15:14 [U-Boot] [PATCH v2] usb: xhci: Fix a potential NULL pointer dereference Sergey Temerkhanov
2015-08-14 20:46 ` Marek Vasut
2015-08-14 22:28   ` Sergei Temerkhanov
2015-08-16 16:55     ` Marek Vasut [this message]
2015-08-18 12:16       ` Sergei Temerkhanov
2015-08-18 13:56         ` Marek Vasut

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201508161855.47407.marex@denx.de \
    --to=marex@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox