| 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: <CAO-hwJ+ySgHfFRmWBRALL+p64VN_+ox5VALbKOQxRUdJgommMA@mail.gmail.com>
Date: Fri, 1 Jun 2018 11:31:19 +0200
From: Benjamin Tissoires <benjamin.tissoires@...hat.com>
To: Peter Hutterer <peter.hutterer@...-t.net>
Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>,
Jiri Kosina <jikos@...nel.org>,
Henrik Rydberg <rydberg@...math.org>,
Jason Gerecke <killertofu@...il.com>,
Dennis Kempin <denniskempin@...gle.com>,
Andrew de los Reyes <adlr@...gle.com>,
"open list:HID CORE LAYER" <linux-input@...r.kernel.org>,
lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] HID: multitouch: report MT_TOOL_PALM for
non-confident touches
On Thu, May 31, 2018 at 1:12 AM, Peter Hutterer
<peter.hutterer@...-t.net> wrote:
> Hi Dmitry,
>
> On Thu, Aug 10, 2017 at 05:44:59PM -0700, Dmitry Torokhov wrote:
>> According to Microsoft specification [1] for Precision Touchpads (and
>> Touchscreens) the devices use "confidence" reports to signal accidental
>> touches, or contacts that are "too large to be a finger". Instead of
>> simply marking contact inactive in this case (which causes issues if
>> contact was originally proper and we lost confidence in it later, as
>> this results in accidental clicks, drags, etc), let's report such
>> contacts as MT_TOOL_PALM and let userspace decide what to do.
>> Additionally, let's report contact size for such touches as maximum
>> allowed for major/minor, which should help userspace that is not yet
>> aware of MT_TOOL_PALM to still perform palm rejection.
>>
>> An additional complication, is that some firmwares do not report
>> non-confident touches as active. To cope with this we delay release of
>> such contact (i.e. if contact was active we first report it as still
>> active MT+TOOL_PALM and then synthesize the release event in a separate
>> frame).
>>
>> [1] https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-windows-precision-touchpad-collection
>>
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
>
> This one popped up again in a bug report [1] and it looks like it never
> got merged. fwiw, libinput does support ABS_MT_TOOL_PALM for touchpads as of
> 1.8.0 and just releasing the touch causes fake taps. So you have the green
> light from me to merge this :)
>
> Cheers,
> Peter
>
> [1] https://bugs.freedesktop.org/show_bug.cgi?id=106716
I can probably integrate this one in the hid-multitouch revamp I am
making, to avoid other the pain of rebasing.
Cheers,
Benjamin
>
>> ---
>> drivers/hid/hid-multitouch.c | 86 +++++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 77 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index 440b999304a5..c28defe50a10 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -114,6 +114,8 @@ struct mt_device {
>> struct timer_list release_timer; /* to release sticky fingers */
>> struct mt_fields *fields; /* temporary placeholder for storing the
>> multitouch fields */
>> + unsigned long *pending_palm_slots; /* slots where we reported palm
>> + and need to release */
>> unsigned long mt_io_flags; /* mt flags (MT_IO_FLAGS_*) */
>> int cc_index; /* contact count field index in the report */
>> int cc_value_index; /* contact count value index in the field */
>> @@ -543,8 +545,12 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> case HID_DG_CONFIDENCE:
>> if ((cls->name == MT_CLS_WIN_8 ||
>> cls->name == MT_CLS_WIN_8_DUAL) &&
>> - field->application == HID_DG_TOUCHPAD)
>> + field->application == HID_DG_TOUCHPAD) {
>> cls->quirks |= MT_QUIRK_CONFIDENCE;
>> + input_set_abs_params(hi->input,
>> + ABS_MT_TOOL_TYPE,
>> + MT_TOOL_FINGER, MT_TOOL_PALM, 0, 0);
>> + }
>> mt_store_field(usage, td, hi);
>> return 1;
>> case HID_DG_TIPSWITCH:
>> @@ -657,6 +663,7 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>>
>> if (td->curvalid || (td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID)) {
>> int active;
>> + int tool;
>> int slotnum = mt_compute_slot(td, input);
>> struct mt_slot *s = &td->curdata;
>> struct input_mt *mt = input->mt;
>> @@ -671,24 +678,56 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>> return;
>> }
>>
>> + active = s->touch_state || s->inrange_state;
>> +
>> if (!(td->mtclass.quirks & MT_QUIRK_CONFIDENCE))
>> s->confidence_state = 1;
>> - active = (s->touch_state || s->inrange_state) &&
>> - s->confidence_state;
>> +
>> + if (likely(s->confidence_state)) {
>> + tool = MT_TOOL_FINGER;
>> + } else {
>> + tool = MT_TOOL_PALM;
>> + if (!active &&
>> + input_mt_is_active(&mt->slots[slotnum])) {
>> + /*
>> + * The non-confidence was reported for
>> + * previously valid contact that is also no
>> + * longer valid. We can't simply report
>> + * lift-off as userspace will not be aware
>> + * of non-confidence, so we need to split
>> + * it into 2 events: active MT_TOOL_PALM
>> + * and a separate liftoff.
>> + */
>> + active = true;
>> + set_bit(slotnum, td->pending_palm_slots);
>> + }
>> + }
>>
>> input_mt_slot(input, slotnum);
>> - input_mt_report_slot_state(input, MT_TOOL_FINGER, active);
>> + input_mt_report_slot_state(input, tool, active);
>> if (active) {
>> /* this finger is in proximity of the sensor */
>> int wide = (s->w > s->h);
>> int major = max(s->w, s->h);
>> int minor = min(s->w, s->h);
>>
>> - /*
>> - * divided by two to match visual scale of touch
>> - * for devices with this quirk
>> - */
>> - if (td->mtclass.quirks & MT_QUIRK_TOUCH_SIZE_SCALING) {
>> + if (unlikely(!s->confidence_state)) {
>> + /*
>> + * When reporting palm, set contact to maximum
>> + * size to help userspace that does not
>> + * recognize MT_TOOL_PALM to reject contacts
>> + * that are too large.
>> + */
>> + major = input_abs_get_max(input,
>> + ABS_MT_TOUCH_MAJOR);
>> + minor = input_abs_get_max(input,
>> + ABS_MT_TOUCH_MINOR);
>> + } else if (td->mtclass.quirks &
>> + MT_QUIRK_TOUCH_SIZE_SCALING) {
>> + /*
>> + * divided by two to match visual scale of touch
>> + * for devices with this quirk
>> + */
>> major = major >> 1;
>> minor = minor >> 1;
>> }
>> @@ -711,6 +750,25 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>> td->num_received++;
>> }
>>
>> +static void mt_release_pending_palms(struct mt_device *td,
>> + struct input_dev *input)
>> +{
>> + int slotnum;
>> + bool need_sync = false;
>> +
>> + for_each_set_bit(slotnum, td->pending_palm_slots, td->maxcontacts) {
>> + clear_bit(slotnum, td->pending_palm_slots);
>> +
>> + input_mt_slot(input, slotnum);
>> + input_mt_report_slot_state(input, MT_TOOL_PALM, false);
>> +
>> + need_sync = true;
>> + }
>> +
>> + if (need_sync)
>> + input_sync(input);
>> +}
>> +
>> /*
>> * this function is called when a whole packet has been received and processed,
>> * so that it can decide what to send to the input layer.
>> @@ -719,6 +777,9 @@ static void mt_sync_frame(struct mt_device *td, struct input_dev *input)
>> {
>> input_mt_sync_frame(input);
>> input_sync(input);
>> +
>> + mt_release_pending_palms(td, input);
>> +
>> td->num_received = 0;
>> if (test_bit(MT_IO_FLAGS_ACTIVE_SLOTS, &td->mt_io_flags))
>> set_bit(MT_IO_FLAGS_PENDING_SLOTS, &td->mt_io_flags);
>> @@ -903,6 +964,13 @@ static int mt_touch_input_configured(struct hid_device *hdev,
>> if (td->is_buttonpad)
>> __set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
>>
>> + td->pending_palm_slots = devm_kcalloc(&hi->input->dev,
>> + BITS_TO_LONGS(td->maxcontacts),
>> + sizeof(long),
>> + GFP_KERNEL);
>> + if (!td->pending_palm_slots)
>> + return -ENOMEM;
>> +
>> ret = input_mt_init_slots(input, td->maxcontacts, td->mt_flags);
>> if (ret)
>> return ret;
>> --
>> 2.14.0.434.g98096fd7a8-goog
>>
>> --
>> 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
>>
Powered by blists - more mailing lists