[PATCH 3.16 305/328] usb: usbip: Fix BUG: KASAN: slab-out-of-bounds in vhci_hub_control()

From: Ben Hutchings
Date: Sun Dec 09 2018 - 16:55:42 EST


3.16.62-rc1 review patch. If anyone has any objections, please let me know.

------------------

From: "Shuah Khan (Samsung OSG)" <shuah@xxxxxxxxxx>

commit 81f7567c51ad97668d1c3a48e8ecc482e64d4161 upstream.

vhci_hub_control() accesses port_status array with out of bounds port
value. Fix it to reference port_status[] only with a valid rhport value
when invalid_rhport flag is true.

The invalid_rhport flag is set early on after detecting in port value
is within the bounds or not.

The following is used reproduce the problem and verify the fix:
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14ed8ab6400000

Reported-by: syzbot+bccc1fe10b70fadc78d0@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Shuah Khan (Samsung OSG) <shuah@xxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
[bwh: Backported to 3.16:
- s/VHCI_HC_PORTS/VHCI_NPORTS/
- Mask wIndex before using it, as done upstream in commit 1c9de5bf4286
"usbip: vhci-hcd: Add USB3 SuperSpeed support"
- Drop inapplicable changes
- Adjust filename]
Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
---
--- a/drivers/staging/usbip/vhci_hcd.c
+++ b/drivers/staging/usbip/vhci_hcd.c
@@ -234,7 +234,8 @@ static int vhci_hub_control(struct usb_h
{
struct vhci_hcd *dum;
int retval = 0;
- int rhport;
+ int rhport = -1;
+ bool invalid_rhport = false;

u32 prev_port_status[VHCI_NPORTS];

@@ -245,11 +246,23 @@ static int vhci_hub_control(struct usb_h
* NOTE:
* wIndex shows the port number and begins from 1.
*/
+ wIndex = ((__u8)(wIndex & 0x00ff));
usbip_dbg_vhci_rh("typeReq %x wValue %x wIndex %x\n", typeReq, wValue,
wIndex);
- if (wIndex > VHCI_NPORTS)
- pr_err("invalid port number %d\n", wIndex);
- rhport = ((__u8)(wIndex & 0x00ff)) - 1;
+
+ /*
+ * wIndex can be 0 for some request types (typeReq). rhport is
+ * in valid range when wIndex >= 1 and < VHCI_HC_PORTS.
+ *
+ * Reference port_status[] only with valid rhport when
+ * invalid_rhport is false.
+ */
+ if (wIndex < 1 || wIndex > VHCI_NPORTS) {
+ invalid_rhport = true;
+ if (wIndex > VHCI_NPORTS)
+ pr_err("invalid port number %d\n", wIndex);
+ } else
+ rhport = wIndex - 1;

dum = hcd_to_vhci(hcd);

@@ -257,8 +270,9 @@ static int vhci_hub_control(struct usb_h

/* store old status and compare now and old later */
if (usbip_dbg_flag_vhci_rh) {
- memcpy(prev_port_status, dum->port_status,
- sizeof(prev_port_status));
+ if (!invalid_rhport)
+ memcpy(prev_port_status, dum->port_status,
+ sizeof(prev_port_status));
}

switch (typeReq) {
@@ -266,8 +280,10 @@ static int vhci_hub_control(struct usb_h
usbip_dbg_vhci_rh(" ClearHubFeature\n");
break;
case ClearPortFeature:
- if (rhport < 0)
+ if (invalid_rhport) {
+ pr_err("invalid port number %d\n", wIndex);
goto error;
+ }
switch (wValue) {
case USB_PORT_FEAT_SUSPEND:
if (dum->port_status[rhport] & USB_PORT_STAT_SUSPEND) {
@@ -315,9 +331,10 @@ static int vhci_hub_control(struct usb_h
break;
case GetPortStatus:
usbip_dbg_vhci_rh(" GetPortStatus port %x\n", wIndex);
- if (wIndex > VHCI_NPORTS || wIndex < 1) {
+ if (invalid_rhport) {
pr_err("invalid port number %d\n", wIndex);
retval = -EPIPE;
+ goto error;
}

/* we do not care about resume. */
@@ -372,8 +389,10 @@ static int vhci_hub_control(struct usb_h
case USB_PORT_FEAT_RESET:
usbip_dbg_vhci_rh(
" SetPortFeature: USB_PORT_FEAT_RESET\n");
- if (rhport < 0)
+ if (invalid_rhport) {
+ pr_err("invalid port number %d\n", wIndex);
goto error;
+ }
/* if it's already running, disconnect first */
if (dum->port_status[rhport] & USB_PORT_STAT_ENABLE) {
dum->port_status[rhport] &=
@@ -389,8 +408,10 @@ static int vhci_hub_control(struct usb_h
default:
usbip_dbg_vhci_rh(" SetPortFeature: default %d\n",
wValue);
- if (rhport < 0)
+ if (invalid_rhport) {
+ pr_err("invalid port number %d\n", wIndex);
goto error;
+ }
dum->port_status[rhport] |= (1 << wValue);
break;
}
@@ -406,7 +427,7 @@ error:
if (usbip_dbg_flag_vhci_rh) {
pr_debug("port %d\n", rhport);
/* Only dump valid port status */
- if (rhport >= 0) {
+ if (!invalid_rhport) {
dump_port_status_diff(prev_port_status[rhport],
dum->port_status[rhport]);
}