Re: USB vulnerability

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Thu Jul 28 2016 15:56:17 GMT-0400 (EDT), Alan Stern wrote:
> On Thu, 28 Jul 2016, Alan Stern wrote:
> 
>> Only bits 10..0 of the wMaxPacketSize field contain the maximum packet
>> size; bits 12..11 contain something else (valid only for high-speed
>> periodic endpoints) and bits 15..13 are reserved (see Table 9-13 in the
>> USB-2.0 spec).
>>
>> Furthermore, the value in bits 10..0 is never supposed to be larger
>> than 1024 (or less depending on the speed and the endpoint type).  We
>> should check these things in config.c/usb_parse_endpoint().
>>
>> I will whip up a patch for this shortly.
> 
> And here it is.  Rosie, can you or your intern check that this fixes 
> the problem?
> 
> Alan Stern
> 
> 
> 
> Index: usb-4.x/drivers/usb/core/config.c
> ===================================================================
> --- usb-4.x.orig/drivers/usb/core/config.c
> +++ usb-4.x/drivers/usb/core/config.c
> @@ -171,6 +171,31 @@ static void usb_parse_ss_endpoint_compan
>  							ep, buffer, size);
>  }
>  
> +static const unsigned short low_speed_maxpacket_maxes[4] = {
> +	[USB_ENDPOINT_XFER_CONTROL] = 8,
> +	[USB_ENDPOINT_XFER_ISOC] = 0,
> +	[USB_ENDPOINT_XFER_BULK] = 0,
> +	[USB_ENDPOINT_XFER_INT] = 8,
> +};
> +static const unsigned short full_speed_maxpacket_maxes[4] = {
> +	[USB_ENDPOINT_XFER_CONTROL] = 64,
> +	[USB_ENDPOINT_XFER_ISOC] = 1023,
> +	[USB_ENDPOINT_XFER_BULK] = 64,
> +	[USB_ENDPOINT_XFER_INT] = 64,
> +};
> +static const unsigned short high_speed_maxpacket_maxes[4] = {
> +	[USB_ENDPOINT_XFER_CONTROL] = 64,
> +	[USB_ENDPOINT_XFER_ISOC] = 1024,
> +	[USB_ENDPOINT_XFER_BULK] = 512,
> +	[USB_ENDPOINT_XFER_INT] = 1023,
> +};
> +static const unsigned short super_speed_maxpacket_maxes[4] = {
> +	[USB_ENDPOINT_XFER_CONTROL] = 512,
> +	[USB_ENDPOINT_XFER_ISOC] = 1024,
> +	[USB_ENDPOINT_XFER_BULK] = 1024,
> +	[USB_ENDPOINT_XFER_INT] = 1024,
> +};
> +
>  static int usb_parse_endpoint(struct device *ddev, int cfgno, int inum,
>      int asnum, struct usb_host_interface *ifp, int num_ep,
>      unsigned char *buffer, int size)
> @@ -179,6 +204,8 @@ static int usb_parse_endpoint(struct dev
>  	struct usb_endpoint_descriptor *d;
>  	struct usb_host_endpoint *endpoint;
>  	int n, i, j, retval;
> +	unsigned int maxp;
> +	const unsigned short *maxpacket_maxes;
>  
>  	d = (struct usb_endpoint_descriptor *) buffer;
>  	buffer += d->bLength;
> @@ -290,6 +317,42 @@ static int usb_parse_endpoint(struct dev
>  			endpoint->desc.wMaxPacketSize = cpu_to_le16(8);
>  	}
>  
> +	/* Validate the wMaxPacketSize field */
> +	maxp = usb_endpoint_maxp(&endpoint->desc);
> +
> +	/* Find the highest legal maxpacket size for this endpoint */
> +	i = 0;		/* additional transactions per microframe */
> +	switch (to_usb_device(ddev)->speed) {
> +	case USB_SPEED_LOW:
> +		maxpacket_maxes = low_speed_maxpacket_maxes;
> +		break;
> +	case USB_SPEED_FULL:
> +		maxpacket_maxes = full_speed_maxpacket_maxes;
> +		break;
> +	case USB_SPEED_HIGH:
> +		/* Bits 12..11 are allowed only for HS periodic endpoints */
> +		if (usb_endpoint_xfer_int(d) || usb_endpoint_xfer_isoc(d)) {
> +			i = maxp & (BIT(12) | BIT(11));
> +			maxp &= ~i;
> +		}
> +		/* fallthrough */
> +	default:
> +		maxpacket_maxes = high_speed_maxpacket_maxes;
> +		break;
> +	case USB_SPEED_SUPER:
> +	case USB_SPEED_SUPER_PLUS:
> +		maxpacket_maxes = super_speed_maxpacket_maxes;
> +		break;
> +	}
> +	j = maxpacket_maxes[usb_endpoint_type(&endpoint->desc)];
> +
> +	if (maxp > j) {
> +		dev_warn(ddev, "config %d interface %d altsetting %d endpoint 0x%X has invalid maxpacket %d, setting to %d\n",
> +		    cfgno, inum, asnum, d->bEndpointAddress, maxp, j);
> +		maxp = j;
> +		endpoint->desc.wMaxPacketSize = cpu_to_le16(i | maxp);
> +	}
> +
>  	/*
>  	 * Some buggy high speed devices have bulk endpoints using
>  	 * maxpacket sizes other than 512.  High speed HCDs may not
> @@ -297,9 +360,6 @@ static int usb_parse_endpoint(struct dev
>  	 */
>  	if (to_usb_device(ddev)->speed == USB_SPEED_HIGH
>  			&& usb_endpoint_xfer_bulk(d)) {
> -		unsigned maxp;
> -
> -		maxp = usb_endpoint_maxp(&endpoint->desc) & 0x07ff;
>  		if (maxp != 512)
>  			dev_warn(ddev, "config %d interface %d altsetting %d "
>  				"bulk endpoint 0x%X has invalid maxpacket %d\n",
> 

Alan,

Yes, Jake confirmed that both this patch and commit c66f59ee5050
referenced in your other email prevent the crash.  However, with this
patch the keyboard is still able to connect.

Thanks!
Rosie Hall

Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux