| 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
| ||
|
Message-ID: <20120506190146.GA12571@polaris.bitmath.org>
Date: Sun, 6 May 2012 21:01:46 +0200
From: "Henrik Rydberg" <rydberg@...omail.se>
To: "benjamin.tissoires" <benjamin.tissoires@...il.com>
Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>,
Jiri Kosina <jkosina@...e.cz>,
Stephane Chatty <chatty@...c.fr>, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/5] HID: hid-multitouch: fix wrong protocol detection
Hi Benjamin,
> The previous implementation introduced a randomness in the splitting
> of the different touches reported by the device. This version is more
> robust as we don't rely on hi->input->absbit, but on our own structure.
>
> This also prepares hid-multitouch to better support Win8 devices.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...c.fr>
> ---
> drivers/hid/hid-multitouch.c | 58 +++++++++++++++++++++++++++++++++--------
> 1 files changed, 46 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 84bb32e..c6ffb05 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -70,9 +70,16 @@ struct mt_class {
> bool is_indirect; /* true for touchpads */
> };
>
> +struct mt_fields {
> + unsigned usages[HID_MAX_FIELDS];
> + unsigned int length;
> +};
> +
> struct mt_device {
> struct mt_slot curdata; /* placeholder of incoming data */
> struct mt_class mtclass; /* our mt device class */
> + struct mt_fields *fields; /* temporary placeholder for storing the
> + multitouch fields */
Why not skip the pointer here?
> unsigned last_field_index; /* last field index of the report */
> unsigned last_slot_field; /* the last field of a slot */
> __s8 inputmode; /* InputMode HID feature, -1 if non-existent */
> @@ -275,11 +282,15 @@ static void set_abs(struct input_dev *input, unsigned int code,
> input_set_abs_params(input, code, fmin, fmax, fuzz, 0);
> }
>
> -static void set_last_slot_field(struct hid_usage *usage, struct mt_device *td,
> +static void mt_store_field(struct hid_usage *usage, struct mt_device *td,
> struct hid_input *hi)
How about adding the last_field_index here also, using a function like
static void mt_store_field(struct mt_device *td, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage)
> {
> - if (!test_bit(usage->hid, hi->input->absbit))
> - td->last_slot_field = usage->hid;
> + struct mt_fields *f = td->fields;
And inserting td->last_field_index = field->index here.
> +
> + if (f->length >= HID_MAX_FIELDS)
> + return;
> +
> + f->usages[f->length++] = usage->hid;
> }
>
> static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> @@ -330,7 +341,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> cls->sn_move);
> /* touchscreen emulation */
> set_abs(hi->input, ABS_X, field, cls->sn_move);
> - set_last_slot_field(usage, td, hi);
> + mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> return 1;
> case HID_GD_Y:
> @@ -340,7 +351,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> cls->sn_move);
> /* touchscreen emulation */
> set_abs(hi->input, ABS_Y, field, cls->sn_move);
> - set_last_slot_field(usage, td, hi);
> + mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> return 1;
> }
> @@ -349,24 +360,24 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> case HID_UP_DIGITIZER:
> switch (usage->hid) {
> case HID_DG_INRANGE:
> - set_last_slot_field(usage, td, hi);
> + mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> return 1;
> case HID_DG_CONFIDENCE:
> - set_last_slot_field(usage, td, hi);
> + mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> return 1;
> case HID_DG_TIPSWITCH:
> hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
> input_set_capability(hi->input, EV_KEY, BTN_TOUCH);
> - set_last_slot_field(usage, td, hi);
> + mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> return 1;
> case HID_DG_CONTACTID:
> if (!td->maxcontacts)
> td->maxcontacts = MT_DEFAULT_MAXCONTACT;
> input_mt_init_slots(hi->input, td->maxcontacts);
> - td->last_slot_field = usage->hid;
> + mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> td->touches_by_report++;
> return 1;
> @@ -375,7 +386,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> EV_ABS, ABS_MT_TOUCH_MAJOR);
> set_abs(hi->input, ABS_MT_TOUCH_MAJOR, field,
> cls->sn_width);
> - set_last_slot_field(usage, td, hi);
> + mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> return 1;
> case HID_DG_HEIGHT:
> @@ -385,7 +396,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> cls->sn_height);
> input_set_abs_params(hi->input,
> ABS_MT_ORIENTATION, 0, 1, 0, 0);
> - set_last_slot_field(usage, td, hi);
> + mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> return 1;
> case HID_DG_TIPPRESSURE:
> @@ -396,7 +407,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> /* touchscreen emulation */
> set_abs(hi->input, ABS_PRESSURE, field,
> cls->sn_pressure);
> - set_last_slot_field(usage, td, hi);
> + mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> return 1;
> case HID_DG_CONTACTCOUNT:
> @@ -650,6 +661,16 @@ static void mt_post_parse_default_settings(struct mt_device *td)
> td->mtclass.quirks = quirks;
> }
>
> +static void mt_post_parse(struct mt_device *td)
> +{
> + struct mt_fields *f = td->fields;
> +
> + if (td->touches_by_report > 0) {
> + int field_count_per_touch = f->length / td->touches_by_report;
> + td->last_slot_field = f->usages[field_count_per_touch - 1]->hid;
> + }
> +}
> +
> static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> {
> int ret, i;
> @@ -680,6 +701,13 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> td->maxcontact_report_id = -1;
> hid_set_drvdata(hdev, td);
>
> + td->fields = kzalloc(sizeof(struct mt_fields), GFP_KERNEL);
> + if (!td->fields) {
> + dev_err(&hdev->dev, "cannot allocate multitouch fields data\n");
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
This can be skipped.
> ret = hid_parse(hdev);
> if (ret != 0)
> goto fail;
> @@ -688,6 +716,8 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> if (ret)
> goto fail;
>
> + mt_post_parse(td);
> +
> if (id->vendor == HID_ANY_ID && id->product == HID_ANY_ID)
> mt_post_parse_default_settings(td);
>
> @@ -705,9 +735,13 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> mt_set_maxcontacts(hdev);
> mt_set_input_mode(hdev);
>
> + kfree(td->fields);
> + td->fields = NULL;
> +
Ditto.
> return 0;
>
> fail:
> + kfree(td->fields);
Ditto.
> kfree(td);
> return ret;
> }
> --
> 1.7.7.6
>
Thanks,
Henrik
--
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