| 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
| ||
|
Date: Fri, 11 Jul 2014 09:20:54 -0400
From: Benjamin Tissoires <benjamin.tissoires@...il.com>
To: Jason Gerecke <killertofu@...il.com>
Cc: Benjamin Tissoires <benjamin.tissoires@...hat.com>,
Dmitry Torokhov <dmitry.torokhov@...il.com>,
Jiri Kosina <jkosina@...e.cz>,
Ping Cheng <pinglinux@...il.com>,
Linux Input <linux-input@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
linuxwacom-devel <linuxwacom-devel@...ts.sourceforge.net>
Subject: Re: [PATCH 05/15] Input - wacom: compute the HID report size to get
the actual packet size
On Thu, Jul 10, 2014 at 9:09 PM, Jason Gerecke <killertofu@...il.com> wrote:
> On Mon, Jun 30, 2014 at 2:26 PM, Benjamin Tissoires
> <benjamin.tissoires@...hat.com> wrote:
>> This removes an USB dependency and is more accurate: the computed pktlen
>> is the actual maximum size of the reports forwarded by the device.
>>
>> Given that the pktlen is correctly computed/validated, we can store it now
>> in the features struct instead of having a special handling in the rest of
>> the code.
>>
>> Likewise, this information is not mandatory anymore in the description
>> of devices in wacom_wac.c. They will be removed in a separate patch.
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>
>
> I'm concerned this new function could be fooled if we release a tablet
> that has one report which is longer than the desired report, but none
> of the hardware I tested was like this and it would be fairly easy to
> address even if it did happen. Thought I should mention it though.
>
You are right to mention it. But in the other hand, such a device
would violate the HID specification, and I am not sure the Windows
driver (which I don't know its internal) would be happy too.
Anyway, as you said, if there is such a device, we an use the report
descriptor fixup capability of HID to prevent this from happening.
Cheers,
Benjamin
> Jason
> ---
> Now instead of four in the eights place /
> you’ve got three, ‘Cause you added one /
> (That is to say, eight) to the two, /
> But you can’t take seven from three, /
> So you look at the sixty-fours....
>
>
>> ---
>> drivers/input/tablet/wacom_sys.c | 58 ++++++++++++++++++----------------------
>> 1 file changed, 26 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c
>> index cd3d936..3f1cee6 100644
>> --- a/drivers/input/tablet/wacom_sys.c
>> +++ b/drivers/input/tablet/wacom_sys.c
>> @@ -149,7 +149,6 @@ static int wacom_parse_logical_collection(unsigned char *report,
>> if (features->type == BAMBOO_PT) {
>>
>> /* Logical collection is only used by 3rd gen Bamboo Touch */
>> - features->pktlen = WACOM_PKGLEN_BBTOUCH3;
>> features->device_type = BTN_TOOL_FINGER;
>>
>> features->x_max = features->y_max =
>> @@ -240,29 +239,6 @@ static int wacom_parse_hid(struct hid_device *hdev,
>> features->device_type = BTN_TOOL_FINGER;
>> /* touch device at least supports one touch point */
>> touch_max = 1;
>> - switch (features->type) {
>> - case TABLETPC2FG:
>> - features->pktlen = WACOM_PKGLEN_TPC2FG;
>> - break;
>> -
>> - case MTSCREEN:
>> - case WACOM_24HDT:
>> - features->pktlen = WACOM_PKGLEN_MTOUCH;
>> - break;
>> -
>> - case MTTPC:
>> - case MTTPC_B:
>> - features->pktlen = WACOM_PKGLEN_MTTPC;
>> - break;
>> -
>> - case BAMBOO_PT:
>> - features->pktlen = WACOM_PKGLEN_BBTOUCH;
>> - break;
>> -
>> - default:
>> - features->pktlen = WACOM_PKGLEN_GRAPHIRE;
>> - break;
>> - }
>>
>> switch (features->type) {
>> case BAMBOO_PT:
>> @@ -305,8 +281,6 @@ static int wacom_parse_hid(struct hid_device *hdev,
>> }
>> } else if (pen) {
>> /* penabled only accepts exact bytes of data */
>> - if (features->type >= TABLETPC)
>> - features->pktlen = WACOM_PKGLEN_GRAPHIRE;
>> features->device_type = BTN_TOOL_PEN;
>> features->x_max =
>> get_unaligned_le16(&report[i + 3]);
>> @@ -1227,12 +1201,34 @@ static void wacom_calculate_res(struct wacom_features *features)
>> features->unitExpo);
>> }
>>
>> +static int wacom_hid_report_len(struct hid_report *report)
>> +{
>> + /* equivalent to DIV_ROUND_UP(report->size, 8) + !!(report->id > 0) */
>> + return ((report->size - 1) >> 3) + 1 + (report->id > 0);
>> +}
>> +
>> +static size_t wacom_compute_pktlen(struct hid_device *hdev)
>> +{
>> + struct hid_report_enum *report_enum;
>> + struct hid_report *report;
>> + size_t size = 0;
>> +
>> + report_enum = hdev->report_enum + HID_INPUT_REPORT;
>> +
>> + list_for_each_entry(report, &report_enum->report_list, list) {
>> + size_t report_size = wacom_hid_report_len(report);
>> + if (report_size > size)
>> + size = report_size;
>> + }
>> +
>> + return size;
>> +}
>> +
>> static int wacom_probe(struct hid_device *hdev,
>> const struct hid_device_id *id)
>> {
>> struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
>> struct usb_device *dev = interface_to_usbdev(intf);
>> - struct usb_endpoint_descriptor *endpoint;
>> struct wacom *wacom;
>> struct wacom_wac *wacom_wac;
>> struct wacom_features *features;
>> @@ -1258,6 +1254,7 @@ static int wacom_probe(struct hid_device *hdev,
>> wacom_wac = &wacom->wacom_wac;
>> wacom_wac->features = *((struct wacom_features *)id->driver_data);
>> features = &wacom_wac->features;
>> + features->pktlen = wacom_compute_pktlen(hdev);
>> if (features->pktlen > WACOM_PKGLEN_MAX) {
>> error = -EINVAL;
>> goto fail1;
>> @@ -1275,8 +1272,6 @@ static int wacom_probe(struct hid_device *hdev,
>> usb_make_path(dev, wacom->phys, sizeof(wacom->phys));
>> strlcat(wacom->phys, "/input0", sizeof(wacom->phys));
>>
>> - endpoint = &intf->cur_altsetting->endpoint[0].desc;
>> -
>> /* set the default size in case we do not get them from hid */
>> wacom_set_default_phy(features);
>>
>> @@ -1287,13 +1282,12 @@ static int wacom_probe(struct hid_device *hdev,
>>
>> /*
>> * Intuos5 has no useful data about its touch interface in its
>> - * HID descriptor. If this is the touch interface (wMaxPacketSize
>> + * HID descriptor. If this is the touch interface (PacketSize
>> * of WACOM_PKGLEN_BBTOUCH3), override the table values.
>> */
>> if (features->type >= INTUOS5S && features->type <= INTUOSHT) {
>> - if (endpoint->wMaxPacketSize == WACOM_PKGLEN_BBTOUCH3) {
>> + if (features->pktlen == WACOM_PKGLEN_BBTOUCH3) {
>> features->device_type = BTN_TOOL_FINGER;
>> - features->pktlen = WACOM_PKGLEN_BBTOUCH3;
>>
>> features->x_max = 4096;
>> features->y_max = 4096;
>> --
>> 2.0.0
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists