Re: [PATCH 3/3] usb: clamp bInterval to allowed range

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

 



Hi,

On Mon, Jul 01, 2013 at 11:23:25AM +0300, Felipe Balbi wrote:
> bInterval must be within the range 1 - 16,
> in order to catch drivers passing a too
> large bInterval (thus zeroing urb->interval),
> let's clamp() the argument to the allowed
> range.
> 
> Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> ---
>  include/linux/usb.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index a232b7e..0883e3a 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -1545,6 +1545,10 @@ static inline void usb_fill_int_urb(struct urb *urb,
>  	urb->transfer_buffer_length = buffer_length;
>  	urb->complete = complete_fn;
>  	urb->context = context;
> +
> +	/* make sure interval is within allowed range */
> +	interval = clamp(interval, 1, 16);

hmmm, there is one problem with this patch. It doesn't cope with the
fact that users could be running at full speed which would make the
allowed range 1 - 255 instead.

perhaps below is slightly better ?

commit 6d2f29734d901e483793c25dc34464eecf67b4e0
Author: Felipe Balbi <balbi@xxxxxx>
Date:   Mon Jul 1 11:09:34 2013 +0300

    usb: clamp bInterval to allowed range
    
    bInterval must be within the range 1 - 16
    when running at High/Super speed, or 1 - 255
    when running at Full/Low speed.
    
    In order to catch drivers passing a too
    large bInterval (thus zeroing urb->interval),
    let's clamp() the argument to the allowed
    ranges.
    
    Signed-off-by: Felipe Balbi <balbi@xxxxxx>

diff --git a/include/linux/usb.h b/include/linux/usb.h
index a232b7e..fcf8110 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1545,10 +1545,20 @@ static inline void usb_fill_int_urb(struct urb *urb,
 	urb->transfer_buffer_length = buffer_length;
 	urb->complete = complete_fn;
 	urb->context = context;
-	if (dev->speed == USB_SPEED_HIGH || dev->speed == USB_SPEED_SUPER)
+
+	if (dev->speed == USB_SPEED_HIGH || dev->speed == USB_SPEED_SUPER) {
+		/* make sure interval is within allowed range */
+		interval = clamp(interval, 1, 16);
+
 		urb->interval = 1 << (interval - 1);
-	else
+	} else if (dev->speed == USB_SPEED_FULL || dev->speed == USB_SPEED_LOW) {
+		/* make sure interval is within allowed range */
+		interval = clamp(interval, 1, 255);
+
 		urb->interval = interval;
+	} else { /* WIRELESS */
+		urb->interval = interval;
+	}
 	urb->start_frame = -1;
 }
 

-- 
balbi

Attachment: signature.asc
Description: 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