CVE-2017-16530: Out-of-bounds Read (CWE-125)
value: The uas driver in the Linux kernel before 4.13.6 allows local users to cause a denial of service (out-of-bounds read and system crash) or possibly have unspecified other impact via a crafted USB device, related to drivers/usb/storage/uas-detect.h and drivers/usb/storage/uas.c.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 6.6
Published: 2017-11-04T01:29Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: drivers/usb/storage/uas-detect.h, drivers/usb/storage/uas.c
Commit date: 2017-09-22 18:29:00 +0200
Commit message: USB: uas: fix bug in handling of alternate settings

The uas driver has a subtle bug in the way it handles alternate
settings.  The uas_find_uas_alt_setting() routine returns an
altsetting value (the bAlternateSetting number in the descriptor), but
uas_use_uas_driver() then treats that value as an index to the
intf->altsetting array, which it isn't.

Normally this doesn't cause any problems because the various
alternate settings have bAlternateSetting values 0, 1, 2, ..., so the
value is equal to the index in the array.  But this is not guaranteed,
and Andrey Konovalov used the syzkaller fuzzer with KASAN to get a
slab-out-of-bounds error by violating this assumption.

This patch fixes the bug by making uas_find_uas_alt_setting() return a
pointer to the altsetting entry rather than either the value or the
index.  Pointers are less subject to misinterpretation.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Andrey Konovalov <andreyknvl@google.com>
Tested-by: Andrey Konovalov <andreyknvl@google.com>
CC: Oliver Neukum <oneukum@suse.com>
CC: <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Diff: diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h
index f58caa9e6a27e6..a155cd02bce240 100644
--- a/drivers/usb/storage/uas-detect.h
+++ b/drivers/usb/storage/uas-detect.h
@@ -7,22 +7,23 @@ static int uas_is_interface(struct usb_host_interface *intf)
 	return (intf->desc.bInterfaceClass == USB_CLASS_MASS_STORAGE &&
 		intf->desc.bInterfaceSubClass == USB_SC_SCSI &&
 		intf->desc.bInterfaceProtocol == USB_PR_UAS);
 }
 
-static int uas_find_uas_alt_setting(struct usb_interface *intf)
+static struct usb_host_interface *uas_find_uas_alt_setting(
+		struct usb_interface *intf)
 {
 	int i;
 
 	for (i = 0; i < intf->num_altsetting; i++) {
 		struct usb_host_interface *alt = &intf->altsetting[i];
 
 		if (uas_is_interface(alt))
-			return alt->desc.bAlternateSetting;
+			return alt;
 	}
 
-	return -ENODEV;
+	return NULL;
 }
 
 static int uas_find_endpoints(struct usb_host_interface *alt,
 			      struct usb_host_endpoint *eps[])
 {
@@ -56,18 +57,18 @@ static int uas_use_uas_driver(struct usb_interface *intf,
 {
 	struct usb_host_endpoint *eps[4] = { };
 	struct usb_device *udev = interface_to_usbdev(intf);
 	struct usb_hcd *hcd = bus_to_hcd(udev->bus);
 	unsigned long flags = id->driver_info;
-	int r, alt;
-
+	struct usb_host_interface *alt;
+	int r;
 
 	alt = uas_find_uas_alt_setting(intf);
-	if (alt < 0)
+	if (!alt)
 		return 0;
 
-	r = uas_find_endpoints(&intf->altsetting[alt], eps);
+	r = uas_find_endpoints(alt, eps);
 	if (r < 0)
 		return 0;
 
 	/*
 	 * ASMedia has a number of usb3 to sata bridge chips, at the time of
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index cfb1e3bbd43471..63cf981ed81cf8 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -871,18 +871,18 @@ MODULE_DEVICE_TABLE(usb, uas_usb_ids);
 #undef UNUSUAL_DEV
 
 static int uas_switch_interface(struct usb_device *udev,
 				struct usb_interface *intf)
 {
-	int alt;
+	struct usb_host_interface *alt;
 
 	alt = uas_find_uas_alt_setting(intf);
-	if (alt < 0)
-		return alt;
+	if (!alt)
+		return -ENODEV;
 
-	return usb_set_interface(udev,
-			intf->altsetting[0].desc.bInterfaceNumber, alt);
+	return usb_set_interface(udev, alt->desc.bInterfaceNumber,
+			alt->desc.bAlternateSetting);
 }
 
 static int uas_configure_endpoints(struct uas_dev_info *devinfo)
 {
 	struct usb_host_endpoint *eps[4] = { };

Vulnerable code: static int uas_find_uas_alt_setting(struct usb_interface *intf)
			return alt->desc.bAlternateSetting;
	return -ENODEV;
	int r, alt;

	if (alt < 0)
	r = uas_find_endpoints(&intf->altsetting[alt], eps);
	int alt;
	if (alt < 0)
		return alt;
	return usb_set_interface(udev,
			intf->altsetting[0].desc.bInterfaceNumber, alt);
Fixed code: static struct usb_host_interface *uas_find_uas_alt_setting(
		struct usb_interface *intf)
			return alt;
	return NULL;
	struct usb_host_interface *alt;
	int r;
	if (!alt)
	r = uas_find_endpoints(alt, eps);
	struct usb_host_interface *alt;
	if (!alt)
		return -ENODEV;
	return usb_set_interface(udev, alt->desc.bInterfaceNumber,
			alt->desc.bAlternateSetting);