* USB vulnerabilities
@ 2016-07-28 16:23 roswest
2016-07-28 17:48 ` Dmitry Torokhov
0 siblings, 1 reply; 6+ messages in thread
From: roswest @ 2016-07-28 16:23 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, Christopher Kopek
[-- Attachment #1.1.1: Type: text/plain, Size: 359 bytes --]
Dmitry,
Hi, I am an engineer at Cisco Systems, and this summer we tasked some
interns with performing USB fuzzing. One of the interns, Anirudh Bagde,
was able to crash the USB stack due to an error in the iforce module and
discovered a buffer over-read in the usbtouchscreen module. Please see
the attachment for details.
Thank you,
Rosie Hall
[-- Attachment #1.1.2: input_issues.txt --]
[-- Type: text/plain, Size: 2279 bytes --]
================================================================================
New Issues
================================================================================
Headline: Buffer over-read in usbtouchscreen module
Platforms: Ubuntu
Versions: 16.04 LTS (Kernel 4.4.0)
CVSS Score: 0.0
CVSS Vector: AV:L/AC:M/Au:N/C:N/I:N/A:N
Filed Defects:
Related Defects:
CWE Tags:
Cycle:
Found by: Anirudh Bagde
The usbtouchscreen module can be led to exceed the bounds of an array by
providing large numbers in a data packet. This happens in the nexio_read_data
function, which is called when a data packet is sent by a Nexio iNexio
touchscreen device. The data packet contains 16-bit integers representing the
length of an array within the data packet, and the function iterates through the
buffer using this array length without checking its bounds.
This buffer overflow does not have much impact, since the contents of the
buffer are only used by a few conditionals. The buffer is only read, not written
to, so the system generally should not crash. Additionally, no data from the
overflowing buffer is sent back to the device, so there is no data leakage
either.
--------------------------------------------------------------------------------
Headline: Crash USB stack with null pointer dereference in iforce module
Platforms: Ubuntu
Versions: 16.04 LTS (Kernel 4.4.0)
CVSS Score: 1.9
CVSS Vector: AV:L/AC:M/Au:N/C:N/I:N/A:P
Filed Defects:
Related Defects:
CWE Tags:
Cycle:
Found by: Anirudh Bagde
The iforce Linux kernel module assumes there are at least two endpoints in a USB
device that matches the driver's VID/PID. If the device has fewer than two
endpoints, this will cause a null pointer dereference in the module.
This bug will usually crash the entire system, though occasionally it crashes
only the USB stack, leaving the rest of the system functional. If it does crash
only the USB stack, newly connected devices will not be detected, and any user
programs that interact with the USB stack (such as lsusb) will hang permanently.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 204 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: USB vulnerabilities
2016-07-28 16:23 USB vulnerabilities roswest
@ 2016-07-28 17:48 ` Dmitry Torokhov
2016-07-28 20:58 ` roswest
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2016-07-28 17:48 UTC (permalink / raw)
To: roswest; +Cc: linux-input, Christopher Kopek
Hi Rosie,
On Thu, Jul 28, 2016 at 12:23:09PM -0400, roswest wrote:
>
> Dmitry,
>
> Hi, I am an engineer at Cisco Systems, and this summer we tasked some
> interns with performing USB fuzzing. One of the interns, Anirudh Bagde,
> was able to crash the USB stack due to an error in the iforce module and
> discovered a buffer over-read in the usbtouchscreen module. Please see
> the attachment for details.
Thank you for the report. Any chance Anirudh could provide patches to
fix these issues as well?
Thanks!
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: USB vulnerabilities
2016-07-28 17:48 ` Dmitry Torokhov
@ 2016-07-28 20:58 ` roswest
2016-07-30 0:48 ` Rosie Hall
0 siblings, 1 reply; 6+ messages in thread
From: roswest @ 2016-07-28 20:58 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Christopher Kopek
[-- Attachment #1.1: Type: text/plain, Size: 719 bytes --]
Dmitry,
Tomorrow is his last day, but he is going to try to accomplish that.
Rosie
On Thu Jul 28 2016 13:48:48 GMT-0400 (EDT), Dmitry Torokhov wrote:
> Hi Rosie,
>
> On Thu, Jul 28, 2016 at 12:23:09PM -0400, roswest wrote:
>>
>> Dmitry,
>>
>> Hi, I am an engineer at Cisco Systems, and this summer we tasked some
>> interns with performing USB fuzzing. One of the interns, Anirudh Bagde,
>> was able to crash the USB stack due to an error in the iforce module and
>> discovered a buffer over-read in the usbtouchscreen module. Please see
>> the attachment for details.
>
> Thank you for the report. Any chance Anirudh could provide patches to
> fix these issues as well?
>
> Thanks!
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 204 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: USB vulnerabilities
2016-07-28 20:58 ` roswest
@ 2016-07-30 0:48 ` Rosie Hall
2016-07-31 1:14 ` Dmitry Torokhov
0 siblings, 1 reply; 6+ messages in thread
From: Rosie Hall @ 2016-07-30 0:48 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Christopher Kopek, abagde1
[-- Attachment #1.1.1: Type: text/plain, Size: 959 bytes --]
Dmitry,
Attached are the patches Anirudh created. Also, I have added him to the
thread if you have any questions or comments for him.
Rosie
On Thu Jul 28 2016 16:58:27 GMT-0400 (EDT), roswest wrote:
> Dmitry,
>
> Tomorrow is his last day, but he is going to try to accomplish that.
>
> Rosie
>
> On Thu Jul 28 2016 13:48:48 GMT-0400 (EDT), Dmitry Torokhov wrote:
>> Hi Rosie,
>>
>> On Thu, Jul 28, 2016 at 12:23:09PM -0400, roswest wrote:
>>>
>>> Dmitry,
>>>
>>> Hi, I am an engineer at Cisco Systems, and this summer we tasked some
>>> interns with performing USB fuzzing. One of the interns, Anirudh Bagde,
>>> was able to crash the USB stack due to an error in the iforce module and
>>> discovered a buffer over-read in the usbtouchscreen module. Please see
>>> the attachment for details.
>>
>> Thank you for the report. Any chance Anirudh could provide patches to
>> fix these issues as well?
>>
>> Thanks!
>>
>
[-- Attachment #1.1.2: iforce-usb.patch --]
[-- Type: text/plain, Size: 961 bytes --]
--- a/drivers/input/joystick/iforce/iforce-usb.c 2016-07-29 15:02:47.602630504 -0400
+++ b/drivers/input/joystick/iforce/iforce-usb.c 2016-07-29 15:02:32.946812336 -0400
@@ -135,12 +135,23 @@
{
struct usb_device *dev = interface_to_usbdev(intf);
struct usb_host_interface *interface;
- struct usb_endpoint_descriptor *epirq, *epout;
+ struct usb_endpoint_descriptor *epirq = NULL, *epout = NULL;
struct iforce *iforce;
- int err = -ENOMEM;
+ int i, err = -ENOMEM;
interface = intf->cur_altsetting;
+ for (i = 0; i < interface->desc.bNumEndpoints; i++) {
+ if (!epirq &&
+ usb_endpoint_dir_in(&interface->endpoint[i].desc))
+ epirq = &interface->endpoint[i].desc;
+ if (!epout &&
+ usb_endpoint_dir_out(&interface->endpoint[i].desc))
+ epout = &interface->endpoint[i].desc;
+ }
+ if (!epirq || !epout)
+ return -ENODEV;
+
epirq = &interface->endpoint[0].desc;
epout = &interface->endpoint[1].desc;
[-- Attachment #1.1.3: usbtouchscreen.patch --]
[-- Type: text/plain, Size: 8192 bytes --]
--- a/drivers/input/touchscreen/usbtouchscreen.c 2016-07-29 17:33:31.430429835 -0400
+++ b/drivers/input/touchscreen/usbtouchscreen.c 2016-07-29 17:31:57.795591496 -0400
@@ -85,7 +85,8 @@
*/
bool irq_always;
- void (*process_pkt) (struct usbtouch_usb *usbtouch, unsigned char *pkt, int len);
+ void (*process_pkt) (struct usbtouch_usb *usbtouch, unsigned char *pkt,
+ unsigned int len);
/*
* used to get the packet len. possible return values:
@@ -95,7 +96,8 @@
*/
int (*get_pkt_len) (unsigned char *pkt, int len);
- int (*read_data) (struct usbtouch_usb *usbtouch, unsigned char *pkt);
+ int (*read_data) (struct usbtouch_usb *usbtouch, unsigned char *pkt,
+ unsigned int len);
int (*alloc) (struct usbtouch_usb *usbtouch);
int (*init) (struct usbtouch_usb *usbtouch);
void (*exit) (struct usbtouch_usb *usbtouch);
@@ -275,7 +277,8 @@
return ret;
}
-static int e2i_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int e2i_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
int tmp = (pkt[0] << 8) | pkt[1];
dev->x = (pkt[2] << 8) | pkt[3];
@@ -343,7 +346,8 @@
return ret;
}
-static int egalax_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int egalax_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
if ((pkt[0] & EGALAX_PKT_TYPE_MASK) != EGALAX_PKT_TYPE_REPT)
return 0;
@@ -387,7 +391,8 @@
#define ETOUCH_PKT_TYPE_REPT2 0xB0
#define ETOUCH_PKT_TYPE_DIAG 0x0A
-static int etouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int etouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
if ((pkt[0] & ETOUCH_PKT_TYPE_MASK) != ETOUCH_PKT_TYPE_REPT &&
(pkt[0] & ETOUCH_PKT_TYPE_MASK) != ETOUCH_PKT_TYPE_REPT2)
@@ -422,7 +427,8 @@
* PanJit Part
*/
#ifdef CONFIG_TOUCHSCREEN_USB_PANJIT
-static int panjit_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int panjit_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
dev->x = ((pkt[2] & 0x0F) << 8) | pkt[1];
dev->y = ((pkt[4] & 0x0F) << 8) | pkt[3];
@@ -442,7 +448,8 @@
#define MTOUCHUSB_RESET 7
#define MTOUCHUSB_REQ_CTRLLR_ID 10
-static int mtouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int mtouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
if (hwcalib_xy) {
dev->x = (pkt[4] << 8) | pkt[3];
@@ -501,7 +508,8 @@
* ITM Part
*/
#ifdef CONFIG_TOUCHSCREEN_USB_ITM
-static int itm_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int itm_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
int touch;
/*
@@ -538,7 +546,8 @@
#ifndef MULTI_PACKET
#define MULTI_PACKET
#endif
-static int eturbo_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int eturbo_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
unsigned int shift;
@@ -569,7 +578,8 @@
* Gunze part
*/
#ifdef CONFIG_TOUCHSCREEN_USB_GUNZE
-static int gunze_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int gunze_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
if (!(pkt[0] & 0x80) || ((pkt[1] | pkt[2] | pkt[3]) & 0x80))
return 0;
@@ -655,7 +665,8 @@
}
-static int dmc_tsc10_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int dmc_tsc10_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
dev->x = ((pkt[2] & 0x03) << 8) | pkt[1];
dev->y = ((pkt[4] & 0x03) << 8) | pkt[3];
@@ -670,7 +681,8 @@
* IRTOUCH Part
*/
#ifdef CONFIG_TOUCHSCREEN_USB_IRTOUCH
-static int irtouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int irtouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
dev->x = (pkt[3] << 8) | pkt[2];
dev->y = (pkt[5] << 8) | pkt[4];
@@ -684,7 +696,8 @@
* ET&T TC5UH/TC4UM part
*/
#ifdef CONFIG_TOUCHSCREEN_USB_ETT_TC45USB
-static int tc45usb_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int tc45usb_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
dev->x = ((pkt[2] & 0x0F) << 8) | pkt[1];
dev->y = ((pkt[4] & 0x0F) << 8) | pkt[3];
@@ -710,7 +723,8 @@
return 0;
}
-static int idealtek_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int idealtek_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
switch (pkt[0] & 0x98) {
case 0x88:
@@ -737,7 +751,8 @@
* General Touch Part
*/
#ifdef CONFIG_TOUCHSCREEN_USB_GENERAL_TOUCH
-static int general_touch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int general_touch_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
dev->x = (pkt[2] << 8) | pkt[1];
dev->y = (pkt[4] << 8) | pkt[3];
@@ -752,7 +767,8 @@
* GoTop Part
*/
#ifdef CONFIG_TOUCHSCREEN_USB_GOTOP
-static int gotop_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int gotop_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
dev->x = ((pkt[1] & 0x38) << 4) | pkt[2];
dev->y = ((pkt[1] & 0x07) << 7) | pkt[3];
@@ -766,7 +782,8 @@
* JASTEC Part
*/
#ifdef CONFIG_TOUCHSCREEN_USB_JASTEC
-static int jastec_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int jastec_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
dev->x = ((pkt[0] & 0x3f) << 6) | (pkt[2] & 0x3f);
dev->y = ((pkt[1] & 0x3f) << 6) | (pkt[3] & 0x3f);
@@ -780,7 +797,8 @@
* Zytronic Part
*/
#ifdef CONFIG_TOUCHSCREEN_USB_ZYTRONIC
-static int zytronic_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int zytronic_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
struct usb_interface *intf = dev->interface;
@@ -964,7 +982,8 @@
kfree(priv);
}
-static int nexio_read_data(struct usbtouch_usb *usbtouch, unsigned char *pkt)
+static int nexio_read_data(struct usbtouch_usb *usbtouch, unsigned char *pkt,
+ unsigned int len)
{
struct nexio_touch_packet *packet = (void *) pkt;
struct nexio_priv *priv = usbtouch->priv;
@@ -977,6 +996,11 @@
if ((pkt[0] & 0xe0) != 0xe0)
return 0;
+ if (data_len > len)
+ data_len = len;
+ if (x_len + y_len > data_len)
+ return 0;
+
if (data_len > 0xff)
data_len -= 0x100;
if (x_len > 0xff)
@@ -1055,7 +1079,8 @@
#ifdef CONFIG_TOUCHSCREEN_USB_ELO
-static int elo_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
+static int elo_read_data(struct usbtouch_usb *dev, unsigned char *pkt,
+ unsigned int len)
{
dev->x = (pkt[3] << 8) | pkt[2];
dev->y = (pkt[5] << 8) | pkt[4];
@@ -1072,7 +1097,7 @@
*/
#ifdef MULTI_PACKET
static void usbtouch_process_multi(struct usbtouch_usb *usbtouch,
- unsigned char *pkt, int len);
+ unsigned char *pkt, unsigned int len);
#endif
static struct usbtouch_device_info usbtouch_dev_info[] = {
@@ -1303,11 +1328,11 @@
* Generic Part
*/
static void usbtouch_process_pkt(struct usbtouch_usb *usbtouch,
- unsigned char *pkt, int len)
+ unsigned char *pkt, unsigned int len)
{
struct usbtouch_device_info *type = usbtouch->type;
- if (!type->read_data(usbtouch, pkt))
+ if (!type->read_data(usbtouch, pkt, len))
return;
input_report_key(usbtouch->input, BTN_TOUCH, usbtouch->touch);
@@ -1327,7 +1352,7 @@
#ifdef MULTI_PACKET
static void usbtouch_process_multi(struct usbtouch_usb *usbtouch,
- unsigned char *pkt, int len)
+ unsigned char *pkt, unsigned int len)
{
unsigned char *buffer;
int pkt_len, pos, buf_len, tmp;
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 243 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: USB vulnerabilities
2016-07-30 0:48 ` Rosie Hall
@ 2016-07-31 1:14 ` Dmitry Torokhov
2016-08-02 4:37 ` Anirudh Bagde
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2016-07-31 1:14 UTC (permalink / raw)
To: Rosie Hall, abagde1; +Cc: linux-input, Christopher Kopek
Hi Rosie, Anirudh,
On Fri, Jul 29, 2016 at 08:48:01PM -0400, Rosie Hall wrote:
> Dmitry,
>
> Attached are the patches Anirudh created. Also, I have added him to the
> thread if you have any questions or comments for him.
>
> Rosie
...
> --- a/drivers/input/joystick/iforce/iforce-usb.c 2016-07-29 15:02:47.602630504 -0400
> +++ b/drivers/input/joystick/iforce/iforce-usb.c 2016-07-29 15:02:32.946812336 -0400
> @@ -135,12 +135,23 @@
> {
> struct usb_device *dev = interface_to_usbdev(intf);
> struct usb_host_interface *interface;
> - struct usb_endpoint_descriptor *epirq, *epout;
> + struct usb_endpoint_descriptor *epirq = NULL, *epout = NULL;
> struct iforce *iforce;
> - int err = -ENOMEM;
> + int i, err = -ENOMEM;
>
> interface = intf->cur_altsetting;
>
> + for (i = 0; i < interface->desc.bNumEndpoints; i++) {
> + if (!epirq &&
> + usb_endpoint_dir_in(&interface->endpoint[i].desc))
> + epirq = &interface->endpoint[i].desc;
> + if (!epout &&
> + usb_endpoint_dir_out(&interface->endpoint[i].desc))
> + epout = &interface->endpoint[i].desc;
> + }
> + if (!epirq || !epout)
> + return -ENODEV;
> +
> epirq = &interface->endpoint[0].desc;
> epout = &interface->endpoint[1].desc;
>
The iforce patch looks good, but I need "Signed-off-by" from Anirudh for
me to apply it. Please see Documentation/SubmittingPatches.
>
> -static int nexio_read_data(struct usbtouch_usb *usbtouch, unsigned char *pkt)
> +static int nexio_read_data(struct usbtouch_usb *usbtouch, unsigned char *pkt,
> + unsigned int len)
I do not think we need to pass the length to the readers: we know what
protocol we are dealing with and we can simply use NEXIO_BUFSIZE.
> {
> struct nexio_touch_packet *packet = (void *) pkt;
> struct nexio_priv *priv = usbtouch->priv;
> @@ -977,6 +996,11 @@
> if ((pkt[0] & 0xe0) != 0xe0)
> return 0;
>
> + if (data_len > len)
> + data_len = len;
> + if (x_len + y_len > data_len)
> + return 0;
We have more adjustments to x_len and data_len below, so maybe we
should move these new checks there as well?
> +
> if (data_len > 0xff)
> data_len -= 0x100;
> if (x_len > 0xff)
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: USB vulnerabilities
2016-07-31 1:14 ` Dmitry Torokhov
@ 2016-08-02 4:37 ` Anirudh Bagde
0 siblings, 0 replies; 6+ messages in thread
From: Anirudh Bagde @ 2016-08-02 4:37 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Rosie Hall, linux-input, Christopher Kopek
[-- Attachment #1: Type: text/plain, Size: 3550 bytes --]
Hi Dmitry,
Thanks for the input. I have added my sign off to the iforce patch.
I'm sending it as an attachment since I don't have my email client
properly setup yet for inline patches. Let me know if there is
anything else that needs to be done.
For the touchscreen patch, I'd forgotten about NEXIO_BUFSIZE, using
that is a good alternative to changing all the reader functions.
However there is still the chance that the packet byte array is
smaller than NEXIO_BUFSIZE so there can still be a buffer overflow,
though much less significant than before, and still relatively
harmless. I will update my patch to use NEXIO_BUFSIZE if you think
that is preferrable. AS for the other two adjustments, I'm not
actually sure what purpose they serve, so I thought I should leave
them alone, but I'll try to make it more concise. I will send the
updated patch soon once I get everything setup to test the patch on my
computer.
Thanks,
Anirudh Bagde
--
Anirudh Bagde
On Sat, Jul 30, 2016 at 9:14 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Rosie, Anirudh,
>
> On Fri, Jul 29, 2016 at 08:48:01PM -0400, Rosie Hall wrote:
>> Dmitry,
>>
>> Attached are the patches Anirudh created. Also, I have added him to the
>> thread if you have any questions or comments for him.
>>
>> Rosie
>
> ...
>
>
>> --- a/drivers/input/joystick/iforce/iforce-usb.c 2016-07-29 15:02:47.602630504 -0400
>> +++ b/drivers/input/joystick/iforce/iforce-usb.c 2016-07-29 15:02:32.946812336 -0400
>> @@ -135,12 +135,23 @@
>> {
>> struct usb_device *dev = interface_to_usbdev(intf);
>> struct usb_host_interface *interface;
>> - struct usb_endpoint_descriptor *epirq, *epout;
>> + struct usb_endpoint_descriptor *epirq = NULL, *epout = NULL;
>> struct iforce *iforce;
>> - int err = -ENOMEM;
>> + int i, err = -ENOMEM;
>>
>> interface = intf->cur_altsetting;
>>
>> + for (i = 0; i < interface->desc.bNumEndpoints; i++) {
>> + if (!epirq &&
>> + usb_endpoint_dir_in(&interface->endpoint[i].desc))
>> + epirq = &interface->endpoint[i].desc;
>> + if (!epout &&
>> + usb_endpoint_dir_out(&interface->endpoint[i].desc))
>> + epout = &interface->endpoint[i].desc;
>> + }
>> + if (!epirq || !epout)
>> + return -ENODEV;
>> +
>> epirq = &interface->endpoint[0].desc;
>> epout = &interface->endpoint[1].desc;
>>
>
>
> The iforce patch looks good, but I need "Signed-off-by" from Anirudh for
> me to apply it. Please see Documentation/SubmittingPatches.
>
>
>>
>> -static int nexio_read_data(struct usbtouch_usb *usbtouch, unsigned char *pkt)
>> +static int nexio_read_data(struct usbtouch_usb *usbtouch, unsigned char *pkt,
>> + unsigned int len)
>
> I do not think we need to pass the length to the readers: we know what
> protocol we are dealing with and we can simply use NEXIO_BUFSIZE.
>
>> {
>> struct nexio_touch_packet *packet = (void *) pkt;
>> struct nexio_priv *priv = usbtouch->priv;
>> @@ -977,6 +996,11 @@
>> if ((pkt[0] & 0xe0) != 0xe0)
>> return 0;
>>
>> + if (data_len > len)
>> + data_len = len;
>> + if (x_len + y_len > data_len)
>> + return 0;
>
> We have more adjustments to x_len and data_len below, so maybe we
> should move these new checks there as well?
>
>> +
>> if (data_len > 0xff)
>> data_len -= 0x100;
>> if (x_len > 0xff)
>
> Thanks.
>
> --
> Dmitry
[-- Attachment #2: iforce-usb.patch --]
[-- Type: application/octet-stream, Size: 1190 bytes --]
Patch to add error checking to the iforce joystick driver when searching for
endpoints, in order to avoid a segfault in case the interface descriptor has
less than two endpoints.
Signed-off-by: Anirudh Bagde <abagde1@gmail.com>
--- a/drivers/input/joystick/iforce/iforce-usb.c 2016-07-29 15:02:47.602630504 -0400
+++ b/drivers/input/joystick/iforce/iforce-usb.c 2016-07-29 15:02:32.946812336 -0400
@@ -135,12 +135,23 @@
{
struct usb_device *dev = interface_to_usbdev(intf);
struct usb_host_interface *interface;
- struct usb_endpoint_descriptor *epirq, *epout;
+ struct usb_endpoint_descriptor *epirq = NULL, *epout = NULL;
struct iforce *iforce;
- int err = -ENOMEM;
+ int i, err = -ENOMEM;
interface = intf->cur_altsetting;
+ for (i = 0; i < interface->desc.bNumEndpoints; i++) {
+ if (!epirq &&
+ usb_endpoint_dir_in(&interface->endpoint[i].desc))
+ epirq = &interface->endpoint[i].desc;
+ if (!epout &&
+ usb_endpoint_dir_out(&interface->endpoint[i].desc))
+ epout = &interface->endpoint[i].desc;
+ }
+ if (!epirq || !epout)
+ return -ENODEV;
+
epirq = &interface->endpoint[0].desc;
epout = &interface->endpoint[1].desc;
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-08-02 4:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-28 16:23 USB vulnerabilities roswest
2016-07-28 17:48 ` Dmitry Torokhov
2016-07-28 20:58 ` roswest
2016-07-30 0:48 ` Rosie Hall
2016-07-31 1:14 ` Dmitry Torokhov
2016-08-02 4:37 ` Anirudh Bagde
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).