Bug 83551
| Summary: | Kernel OOPses on access to address 0x8 when cdc-acm device is inserted with invalid descriptor. | ||
|---|---|---|---|
| Product: | Drivers | Reporter: | Simon Schubert (2+kernel) |
| Component: | USB | Assignee: | Greg Kroah-Hartman (greg) |
| Status: | NEW --- | ||
| Severity: | normal | ||
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 3.16 (tested), v3.17-rc2-227-gfd5984d (visually) | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
| Attachments: | Move interface validity check to cover both branches | ||
On Sun, Aug 31, 2014 at 10:15:37AM +0000, bugzilla-daemon@bugzilla.kernel.org wrote: > https://bugzilla.kernel.org/show_bug.cgi?id=83551 > > Bug ID: 83551 > Summary: Kernel OOPses on access to address 0x8 when cdc-acm > device is inserted with invalid descriptor. Can you please send this to the linux-usb@vger.kernel.org mailing list? |
Created attachment 148871 [details] Move interface validity check to cover both branches *** Setting Invalid configuration descriptor as follows: #+BEGIN_SRC text 0000 09 02 43 00 02 01 00 80 64 09 04 00 00 01 02 02 ..C.....d....... 0010 00 00 05 24 00 10 01 04 24 02 06 04 24 01 00 01 ...$....$...$... 0020 05 24 06 00 01 07 05 81 03 08 00 ff 09 04 01 00 .$.............. 0030 02 0a 00 00 00 07 05 82 02 40 00 ff 07 05 01 02 .........@...... 0040 20 00 ff .. #+END_SRC text In particular, the CDC Call Management Descriptor has its length declared too short (4 instead of 5), and the following CDC Union Descriptor is therefore unreachable. *** Code problems Multiple issues: 1. The ~while (buflen > 0)~ loop that parses the interface aux data does not perform correct boundary checking. In the above case, ~call_interface_num = buffer[4];~ accesses outside of the (declared) descriptor content. 2. If a union header is missing, there is no code path that checks whether the ~data_interface~ (resolved from ~call_interface_num~) actually exists. Later ~if (data_interface->cur_altsetting->desc.bInterfaceClass~ dereferences ~data_interface~. *** Proposed solution Move the ~if (!control_interface || !data_interface)~ block out the ~if (!union_header)~ conditional. That way both branches will benefit from the check. This only addresses issue (2), not issue (1). See attachment.