All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] usb: xhci: Fix a potential NULL pointer dereference
Date: Fri, 14 Aug 2015 16:45:56 +0200	[thread overview]
Message-ID: <201508141645.56825.marex@denx.de> (raw)
In-Reply-To: <CAPEA6dYdY9bEgsXGMG9ncgBd22iUVzyi5pyJGWMnAFi7fonE6A@mail.gmail.com>

On Friday, August 14, 2015 at 04:29:43 PM, Sergei Temerkhanov wrote:
> On Fri, Aug 14, 2015 at 4:44 PM, Marek Vasut <marex@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.

Thanks :)

> > 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;
>  }

You might also want to set *controller to NULL, just to be explicitly correct.
Can you please send a V2 patch ?

  reply	other threads:[~2015-08-14 14:45 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-14 12:13 [U-Boot] [PATCH] usb: xhci: Fix a potential NULL pointer dereference Sergey Temerkhanov
2015-08-14 12:15 ` Marek Vasut
2015-08-14 13:00   ` Sergei Temerkhanov
2015-08-14 13:44     ` Marek Vasut
2015-08-14 14:29       ` Sergei Temerkhanov
2015-08-14 14:45         ` Marek Vasut [this message]
2015-08-14 14:52           ` Sergei Temerkhanov

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=201508141645.56825.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.