* [U-Boot] [PATCH v2] usb: xhci: Fix a potential NULL pointer dereference @ 2015-08-14 15:14 Sergey Temerkhanov 2015-08-14 20:46 ` Marek Vasut 0 siblings, 1 reply; 6+ messages in thread From: Sergey Temerkhanov @ 2015-08-14 15:14 UTC (permalink / raw) To: u-boot 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; + } + return ret; } @@ -1093,9 +1098,11 @@ int usb_lowlevel_stop(int index) { struct xhci_ctrl *ctrl = (xhcic + index); - xhci_lowlevel_stop(ctrl); - xhci_hcd_stop(index); - xhci_cleanup(ctrl); + if (ctrl->hcor) { + xhci_lowlevel_stop(ctrl); + xhci_hcd_stop(index); + xhci_cleanup(ctrl); + } return 0; } -- 2.2.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] usb: xhci: Fix a potential NULL pointer dereference 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 0 siblings, 1 reply; 6+ messages in thread From: Marek Vasut @ 2015-08-14 20:46 UTC (permalink / raw) To: u-boot 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 ? > + } > + > return ret; > } > > @@ -1093,9 +1098,11 @@ int usb_lowlevel_stop(int index) > { > struct xhci_ctrl *ctrl = (xhcic + index); > > - xhci_lowlevel_stop(ctrl); > - xhci_hcd_stop(index); > - xhci_cleanup(ctrl); > + if (ctrl->hcor) { > + xhci_lowlevel_stop(ctrl); > + xhci_hcd_stop(index); > + xhci_cleanup(ctrl); > + } > > return 0; > } Good job, thanks :) Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] usb: xhci: Fix a potential NULL pointer dereference 2015-08-14 20:46 ` Marek Vasut @ 2015-08-14 22:28 ` Sergei Temerkhanov 2015-08-16 16:55 ` Marek Vasut 0 siblings, 1 reply; 6+ messages in thread From: Sergei Temerkhanov @ 2015-08-14 22:28 UTC (permalink / raw) To: u-boot 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. > >> + } >> + >> return ret; >> } >> >> @@ -1093,9 +1098,11 @@ int usb_lowlevel_stop(int index) >> { >> struct xhci_ctrl *ctrl = (xhcic + index); >> >> - xhci_lowlevel_stop(ctrl); >> - xhci_hcd_stop(index); >> - xhci_cleanup(ctrl); >> + if (ctrl->hcor) { >> + xhci_lowlevel_stop(ctrl); >> + xhci_hcd_stop(index); >> + xhci_cleanup(ctrl); >> + } >> >> return 0; >> } > > Good job, thanks :) > > Best regards, > Marek Vasut ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] usb: xhci: Fix a potential NULL pointer dereference 2015-08-14 22:28 ` Sergei Temerkhanov @ 2015-08-16 16:55 ` Marek Vasut 2015-08-18 12:16 ` Sergei Temerkhanov 0 siblings, 1 reply; 6+ messages in thread From: Marek Vasut @ 2015-08-16 16:55 UTC (permalink / raw) To: u-boot 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! ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] usb: xhci: Fix a potential NULL pointer dereference 2015-08-16 16:55 ` Marek Vasut @ 2015-08-18 12:16 ` Sergei Temerkhanov 2015-08-18 13:56 ` Marek Vasut 0 siblings, 1 reply; 6+ messages in thread From: Sergei Temerkhanov @ 2015-08-18 12:16 UTC (permalink / raw) To: u-boot On Sun, Aug 16, 2015 at 7:55 PM, Marek Vasut <marex@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@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! Please see the v3 of this patch Regards, Sergey ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] usb: xhci: Fix a potential NULL pointer dereference 2015-08-18 12:16 ` Sergei Temerkhanov @ 2015-08-18 13:56 ` Marek Vasut 0 siblings, 0 replies; 6+ messages in thread From: Marek Vasut @ 2015-08-18 13:56 UTC (permalink / raw) To: u-boot 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@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@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! > > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-08-18 13:56 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 2015-08-18 12:16 ` Sergei Temerkhanov 2015-08-18 13:56 ` Marek Vasut
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox