linux-kernel - Re: [PATCH v1 1/1] USB: serial: Add boundry check for read

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1520499297.2983.3.camel@suse.com>
Date:   Thu, 08 Mar 2018 09:54:57 +0100
From:   Oliver Neukum <oneukum@...e.com>
To:     sathyanarayanan.kuppuswamy@...ux.intel.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

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.

	Regards
		Oliver