Messages in this thread | | | Subject | Re: [PATCH v1 1/1] USB: serial: Add boundry check for read_urbs array access | From | sathyanarayanan kuppuswamy <> | Date | Wed, 7 Mar 2018 13:41:51 -0800 |
| |
On 03/07/2018 12:58 PM, Greg KH wrote: > On Wed, Mar 07, 2018 at 12:23:56PM -0800, sathyanarayanan.kuppuswamy@linux.intel.com wrote: >> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> >> >> In usb_serial_generic_submit_read_urb() function we are accessing the >> port->read_urbs array without any boundry checks. This might lead to >> kernel panic when index value goes above array length. >> >> One posible call path for this issue is, >> >> usb_serial_generic_read_bulk_callback() >> { >> ... >> if (!port->throttled) { >> usb_serial_generic_submit_read_urb(port, i, GFP_ATOMIC); >> ... >> } > How does i ever get to be greater than the array size here in this > function? It directly came from looking in that array in the first > place :) > > 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.
> > 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
| |