| 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: <f3f844b7-777e-41ce-4661-bcf8735763c6@gmail.com>
Date: Mon, 26 Sep 2016 12:53:06 +0300
From: Nikolai Kondrashov <spbnick@...il.com>
To: Benjamin Tissoires <benjamin.tissoires@...hat.com>,
Ioan-Adrian Ratiu <adi@...rat.com>,
Vladislav Naumov <vnaum@...um.com>
Cc: jikos@...nel.org, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] hid: input: add HID_QUIRK_REUSE_AXES and fix
dragonrise
Hi Benjamin,
On 09/26/2016 12:29 PM, Benjamin Tissoires wrote:
> Ideally, we need to have Dragon Rise 0x0011 tested too. Nick, would you
> mind checking it if you still have this particular device?
I never had it, but perhaps Vladislav still has some.
Vladislav, would you be able to test a change to the kernel module for your
Dragonrise gamepads?
Please see below for context.
Thank you.
Nick
On 09/26/2016 12:29 PM, Benjamin Tissoires wrote:
> On Sep 26 2016 or thereabouts, Ioan-Adrian Ratiu wrote:
>> Commit 79346d620e9d ("HID: input: force generic axis to be mapped to their
>> user space axis") made mapping generic axes to their userspace equivalents
>> mandatory and some lower end gamepads which were depending on the previous
>> behaviour suffered severe regressions because they were reusing axes and
>> expecting hid-input to multiplex their map to the respective userspace axis
>> by always searching for and using the next available axis.
>
> Yes, I apologies for the breakage and the regression, though I must say
> that for now, only one hardware maker and one device (or range of devices
> from the look of it) has needed to be quirked.
>
>>
>> Now the result is that different device axes appear on a single axis in
>> userspace, which is clearly a regression in the hid-input driver because it
>> needs to continue to handle this hardware as expected before the forcing
>> to provide the same interface to userspace.
>>
>> Since these lower-end gamepads like 0079:0006 are definitely the exception,
>> create a quirk to fix them.
>
> Given that we only have this particular vendor that is an issue, I'd
> rather see the fix in hid-dr.c. The reason being that you actually don't
> need to have a global quirk and this simplifies the path in hid-input.
> Plus for users, they can just upgrade hid-dr without having to recompile
> their kernel when hid-core is not compiled as a module.
>
> The cleanest solution that wouldn't require any quirk in hid-core is to
> simply add an .input_mapping() callback in hid-dr.c.
>
> The code of the callback could be something like (untested):
>
> static int dr_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> struct hid_field *field, struct hid_usage *usage,
> unsigned long **bit, int *max)
> {
> switch (usage->hid) {
> /*
> * revert the old hid-input behavior where axes
> * can be randomly assigned when the hid usage is
> * reused.
> */
> case HID_GD_X: case HID_GD_Y: case HID_GD_Z:
> case HID_GD_RX: case HID_GD_RY: case HID_GD_RZ:
> if (field->flags & HID_MAIN_ITEM_RELATIVE)
> map_rel(usage->hid & 0xf);
> else
> map_abs(usage->hid & 0xf);
> return 1;
> }
>
> return 0;
> }
>
> Hopefully, something like this should revert the old behavior for all
> hid-dr touchpads.
>
> Ideally, we need to have Dragon Rise 0x0011 tested too. Nick, would you
> mind checking it if you still have this particular device?
>
> Cheers,
> Benjamin
>
>>
>> Signed-off-by: Ioan-Adrian Ratiu <adi@...rat.com>
>> ---
>> drivers/hid/hid-dr.c | 2 ++
>> drivers/hid/hid-input.c | 16 +++++++++++-----
>> include/linux/hid.h | 1 +
>> 3 files changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/hid/hid-dr.c b/drivers/hid/hid-dr.c
>> index 2523f8a..27fc826 100644
>> --- a/drivers/hid/hid-dr.c
>> +++ b/drivers/hid/hid-dr.c
>> @@ -274,6 +274,8 @@ static int dr_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> hid_hw_stop(hdev);
>> goto err;
>> }
>> + /* has only 5 axes and reuses X, Y */
>> + hdev->quirks |= HID_QUIRK_REUSE_AXES;
>> break;
>> }
>>
>> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
>> index fb9ace1..1cc6fe4 100644
>> --- a/drivers/hid/hid-input.c
>> +++ b/drivers/hid/hid-input.c
>> @@ -633,11 +633,17 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>> /* These usage IDs map directly to the usage codes. */
>> case HID_GD_X: case HID_GD_Y: case HID_GD_Z:
>> case HID_GD_RX: case HID_GD_RY: case HID_GD_RZ:
>> - if (field->flags & HID_MAIN_ITEM_RELATIVE)
>> - map_rel(usage->hid & 0xf);
>> - else
>> - map_abs_clear(usage->hid & 0xf);
>> - break;
>> +
>> + /* if quirk is active don't force the userspace mapping,
>> + * instead search and use the next available axis.
>> + */
>> + if (!(device->quirks & HID_QUIRK_REUSE_AXES)) {
>> + if (field->flags & HID_MAIN_ITEM_RELATIVE)
>> + map_rel(usage->hid & 0xf);
>> + else
>> + map_abs_clear(usage->hid & 0xf);
>> + break;
>> + }
>>
>> case HID_GD_SLIDER: case HID_GD_DIAL: case HID_GD_WHEEL:
>> if (field->flags & HID_MAIN_ITEM_RELATIVE)
>> diff --git a/include/linux/hid.h b/include/linux/hid.h
>> index 75b66ec..0979920 100644
>> --- a/include/linux/hid.h
>> +++ b/include/linux/hid.h
>> @@ -320,6 +320,7 @@ struct hid_item {
>> #define HID_QUIRK_NO_EMPTY_INPUT 0x00000100
>> #define HID_QUIRK_NO_INIT_INPUT_REPORTS 0x00000200
>> #define HID_QUIRK_ALWAYS_POLL 0x00000400
>> +#define HID_QUIRK_REUSE_AXES 0x00000800
>> #define HID_QUIRK_SKIP_OUTPUT_REPORTS 0x00010000
>> #define HID_QUIRK_SKIP_OUTPUT_REPORT_ID 0x00020000
>> #define HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP 0x00040000
>> --
>> 2.10.0
>>
Powered by blists - more mailing lists