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@xxxxxxxxx> 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:
iforce-usb.patch
Description: Binary data
