Re: [PATCH] USB: validate wMaxPacketValue entries in endpoint descriptors

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

 



Alan,

I think it would be more accurate to put Jake Lamberson as the reporter
and tester.

Thanks,
Rosie

On Mon Aug 01 2016 12:18:54 GMT-0400 (EDT), Alan Stern wrote:
> Erroneous or malicious endpoint descriptors may have non-zero bits in
> reserved positions, or out-of-bounds values.  This patch helps prevent
> these from causing problems by bounds-checking the wMaxPacketValue
> entries in endpoint descriptors and capping the values at the maximum
> allowed.
> 
> Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> Reported-by: roswest <roswest@xxxxxxxxx>
> Tested-by: roswest <roswest@xxxxxxxxx>
> 
> ---
> 
> I don't think this needs to go into the -stable kernels, but if anyone
> disagrees I won't object.
> 
> [as1806]
> 
> 
>  drivers/usb/core/config.c |   66 +++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 63 insertions(+), 3 deletions(-)
> 
> 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",
> 

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