Hi, On Mon, Jul 01, 2013 at 10:19:28AM -0400, Alan Stern wrote: > > On Mon, Jul 01, 2013 at 09:56:37AM -0400, Alan Stern wrote: > > > On Mon, 1 Jul 2013, Felipe Balbi wrote: > > > > > > > bInterval must be within the range 1 - 16, > > > > > > That's true only for high speed and SuperSpeed. For low speed and full > > > speed, bInterval is allowed to range from 1 to 255. See p. 271 in the > > > USB-2 spec. > > > > > > > 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); > > > > > > These lines should go within the first branch of the "if" statement > > > below. > > > > could you look at the newer version I sent as a reply to this and tell > > me what you think of it ? > > I shouldn't have replied so quickly to the original post -- it was a > violation of my normal policy to read through all the accumulated inbox > messages before replying to any of them. :-( > > Yeah, the updated version is better. I don't think we need to test the > full/low-speed case, though. There already are checks in > usb_submit_urb() to handle unreasonable values of urb->interval. The > only reason for checking in the high/SuperSpeed case is that the stored > value is computed from the input value, and you don't want the > computation to overflow. right, which is what happens on two drivers already (see patches 1 and 2). It makes sense to drop the Full/Low speed check, I'll do that and resend :-) Should I add your Reviewed-by or Acked-by ? cheers -- balbi
Attachment:
signature.asc
Description: Digital signature
