[U-Boot] [PATCH v2] usb: xhci: Fix a potential NULL pointer dereference
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; }
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
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
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!
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
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
participants (3)
-
Marek Vasut -
Sergei Temerkhanov -
Sergey Temerkhanov