Messages in this thread | | | Subject | Re: [PATCH v1 1/1] USB: serial: Add boundry check for read_urbs array access | From | sathyanarayanan kuppuswamy <> | Date | Thu, 8 Mar 2018 16:34:44 -0800 |
| |
On 03/08/2018 03:43 PM, Greg KH wrote: > 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. > > thanks, > > greg k-h > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
-- Sathyanarayanan Kuppuswamy Linux kernel developer
| |