On Thu, Mar 08, 2018 at 03:29:48PM -0800, sathyanarayanan kuppuswamy wrote:
>
>
> On 03/08/2018 12:54 AM, Oliver Neukum wrote:
> > Am Mittwoch, den 07.03.2018, 13:41 -0800 schrieb sathyanarayanan
> > kuppuswamy :
> > > On 03/07/2018 12:58 PM, Greg KH wrote:
> > > > So I don't see why your check is needed, what other code path would ever
> > > > call this function in a way that the bounds check would be needed?
> > > void usb_serial_generic_read_bulk_callback(struct urb *urb)
> > >
> > > 385 for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i) {
> > > 386 if (urb == port->read_urbs[i])
> > > 387 break;
> > > 388 }
> > >
> > > In here, after this for loop is done (without any matching urb), i value
> > > will be equal to ARRAY_SIZE(port->read_urbs). So there is a possibility
> > > of usb_serial_generic_submit_read_urb() getting called with this invalid
> > > index.
> > If this happens the function was called for a stray URB.
> > Your check comes to late. We have called set_bit with an invalid index
> > and other shit.
> > We definitely do not just want to return an error in that case.
> In that case do you think we should use some WARN_ON() for invalid index in
> usb_serial_generic_read_bulk_callback()?
No, again, how could that ever happen?
Don't add pointless error checking for things that are impossible to
ever hit :)
thanks,
greg k-h
