2018-04-05 22:33:53

by Shuah Khan

Subject: [PATCH] usbip: vhci_hcd: check rhport before using in vhci_hub_control()

Validate !rhport < 0 before using it to access port_status array.

Signed-off-by: Shuah Khan <[email protected]>
---
drivers/usb/usbip/vhci_hcd.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 20e3d4609583..d11f3f8dad40 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -354,6 +354,8 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
usbip_dbg_vhci_rh(" ClearHubFeature\n");
break;
case ClearPortFeature:
+ if (rhport < 0)
+ goto error;
switch (wValue) {
case USB_PORT_FEAT_SUSPEND:
if (hcd->speed == HCD_USB3) {
@@ -511,11 +513,16 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
goto error;
}

+ if (rhport < 0)
+ goto error;
+
vhci_hcd->port_status[rhport] |= USB_PORT_STAT_SUSPEND;
break;
case USB_PORT_FEAT_POWER:
usbip_dbg_vhci_rh(
" SetPortFeature: USB_PORT_FEAT_POWER\n");
+ if (rhport < 0)
+ goto error;
if (hcd->speed == HCD_USB3)
vhci_hcd->port_status[rhport] |= USB_SS_PORT_STAT_POWER;
else
@@ -524,6 +531,8 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
case USB_PORT_FEAT_BH_PORT_RESET:
usbip_dbg_vhci_rh(
" SetPortFeature: USB_PORT_FEAT_BH_PORT_RESET\n");
+ if (rhport < 0)
+ goto error;
/* Applicable only for USB3.0 hub */
if (hcd->speed != HCD_USB3) {
pr_err("USB_PORT_FEAT_BH_PORT_RESET req not "
@@ -534,6 +543,8 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
case USB_PORT_FEAT_RESET:
usbip_dbg_vhci_rh(
" SetPortFeature: USB_PORT_FEAT_RESET\n");
+ if (rhport < 0)
+ goto error;
/* if it's already enabled, disable */
if (hcd->speed == HCD_USB3) {
vhci_hcd->port_status[rhport] = 0;
@@ -554,6 +565,8 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
default:
usbip_dbg_vhci_rh(" SetPortFeature: default %d\n",
wValue);
+ if (rhport < 0)
+ goto error;
if (hcd->speed == HCD_USB3) {
if ((vhci_hcd->port_status[rhport] &
USB_SS_PORT_STAT_POWER) != 0) {
--
2.14.1

2018-04-06 08:02:53

by Sergei Shtylyov

Subject: Re: [PATCH] usbip: vhci_hcd: check rhport before using in vhci_hub_control()

Hello!

On 4/6/2018 1:31 AM, Shuah Khan wrote:

> Validate !rhport < 0 before using it to access port_status array.

Why '!'?

> Signed-off-by: Shuah Khan <[email protected]>
> ---
> drivers/usb/usbip/vhci_hcd.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> index 20e3d4609583..d11f3f8dad40 100644
> --- a/drivers/usb/usbip/vhci_hcd.c
> +++ b/drivers/usb/usbip/vhci_hcd.c
[...]

MBR, Sergei

2018-04-06 19:08:00

by Shuah Khan

Subject: Re: [PATCH] usbip: vhci_hcd: check rhport before using in vhci_hub_control()

On 04/06/2018 02:01 AM, Sergei Shtylyov wrote:
> Hello!
>
> On 4/6/2018 1:31 AM, Shuah Khan wrote:
>
>> Validate !rhport < 0 before using it to access port_status array.
>
>    Why '!'?
>

I should have explained it better in the commit log.

rhport is set based on input wIndex which could be 0. This isn't
the case for all the Request but some. wIndex is range checked in
the code paths that it shouldn't be. The same applies to rhport
in some request handling paths. Without the checks there is the
potential for out of bounds access on port_status array.

thanks,
-- Shuah