* [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