[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <df24d0e5-73b1-3300-79a5-3dd7329f8a57@linux.intel.com>
Date: Thu, 8 Mar 2018 15:29:48 -0800
From: sathyanarayanan kuppuswamy
<sathyanarayanan.kuppuswamy@...ux.intel.com>
To: Oliver Neukum <oneukum@...e.com>,
Greg KH <gregkh@...uxfoundation.org>
Cc: johan@...nel.org, linux-kernel@...r.kernel.org,
linux-usb@...r.kernel.org
Subject: Re: [PATCH v1 1/1] USB: serial: Add boundry check for read_urbs array
access
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()?
>
> Regards
> Oliver
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Sathyanarayanan Kuppuswamy
Linux kernel developer
Powered by blists - more mailing lists