lkml.org 
[lkml]   [2018]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v1 1/1] USB: serial: Add boundry check for read_urbs array access
From
Date


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

\
 
 \ /
  Last update: 2018-03-07 22:43    [W:0.050 / U:4.028 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site