linux-kernel - Re: [PATCH] HID: sony: Fix for broken buttons on DS3 USB dongles

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 13 May 2020 15:06:04 -0500
From:   Scott Shumate <scott.shumate@...il.com>
To:     "Colenbrander, Roderick" <Roderick.Colenbrander@...y.com>,
        Jiri Kosina <jikos@...nel.org>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] HID: sony: Fix for broken buttons on DS3 USB dongles

Hi Roderick,

The official DS3 has a Report Count(19) instead of Report Count(13) in the exact same offset.  I 
have no idea what the silicon vendor for these dongles was thinking but it's suspicious that the 
official count of 19 (0x13) turned into 13 (0xd) in the knock-off.  It makes you wonder if the 
engineers confused the decimal/hex numbers.

As buggy as all of these third-party devices are, I'm afraid relying on the HID parser to get it 
right is only going to worse over time.  I do like your idea of having each device register 
themselves.  It would be nice to have each device provide a callback to decode its own report rather 
than handle a bunch of special conditions and quirks in a unified report decoding function.  The 
drawback of course is that its going to be a little more effort to maintain.

Cheers,
Scott


On 5/13/20 2:41 PM, Colenbrander, Roderick wrote:
> Hi Scott,
> 
> Thanks for sharing this patch. Do you know if for these controllers the data is at the same byte offsets in the reports as an official DS3?
> 
> The reason I'm asking is that I have been considering for a while to redo some of the button / stick handling code and the issue you just pointed out could be another argument towards doing that. Basically we learned that also for the official Navigation controller there are apparently a few different revisions with different report descriptors (yah!).
> 
> I have been tempted for some time to get rid of this fixup / mapping logic altogether and e.g. have a "sony_register_gamepad" and do it all ourselves and put more logic into the sixaxis_parse_report call. With many buggy devices of which we don't have most it feels fragile to have the HID parser do the work.
> 
> Thanks,
> Roderick
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> From: Scott Shumate <scott.shumate@...il.com>
> 
> Sent: Wednesday, May 13, 2020 10:05 AM
> 
> To: Jiri Kosina <jikos@...nel.org>; Benjamin Tissoires <benjamin.tissoires@...hat.com>; Colenbrander, Roderick <Roderick.Colenbrander@...y.com>; linux-input@...r.kernel.org <linux-input@...r.kernel.org>; linux-kernel@...r.kernel.org <linux-kernel@...r.kernel.org>
> 
> Subject: [PATCH] HID: sony: Fix for broken buttons on DS3 USB dongles
> 
>   
> 
> 
> Fix for non-working buttons on knock-off USB dongles for Sony
> controllers. These USB dongles are used to connect older Sony DA/DS1/DS2
> controllers via USB and are common on Amazon, AliExpress, etc.  Without
> the patch, the square, X, and circle buttons do not function.  These
> dongles used to work prior to kernel 4.10 but removing the global DS3
> report fixup in this commit exposed the problem:
> 
> commit e19a267b9987 Author: Roderick Colenbrander
>   <roderick.colenbrander@...y.com>
> Date:   Tue Mar 7 15:45:08 2017 -0800
> 
>      HID: sony: DS3 comply to Linux gamepad spec
> 
> Many people reported the problem on the Ubuntu forums and are working
> around the problem by falling back to the 4.9 hid-sony driver.
> 
> The problem stems from these dongles incorrectly reporting their button
> count as 13 instead of 16.  This patch fixes up the report descriptor by
> changing the button report count to 16 and removing 3 padding bits.
> 
> Signed-off-by: Scott Shumate <scott.shumate@...il.com>
> ---
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 4c6ed6ef31f1..2f073f536070 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -867,6 +867,23 @@ static u8 *sony_report_fixup(struct hid_device *hdev, u8 *rdesc,
>   	if (sc->quirks & PS3REMOTE)
>   		return ps3remote_fixup(hdev, rdesc, rsize);
>   
> +	/*
> +	 * Some knock-off USB dongles incorrectly report their button count
> +	 * as 13 instead of 16 causing three non-functional buttons.
> +	 */
> +	if ((sc->quirks & SIXAXIS_CONTROLLER_USB) && *rsize >= 45 &&
> +		/* Report Count (13) */
> +		rdesc[23] == 0x95 && rdesc[24] == 0x0D &&
> +		/* Usage Maximum (13) */
> +		rdesc[37] == 0x29 && rdesc[38] == 0x0D &&
> +		/* Report Count (3) */
> +		rdesc[43] == 0x95 && rdesc[44] == 0x03) {
> +		hid_info(hdev, "Fixing up USB dongle report descriptor\n");
> +		rdesc[24] = 0x10;
> +		rdesc[38] = 0x10;
> +		rdesc[44] = 0x00;
> +	}
> +
>   	return rdesc;
>   }
>   
> 
> 
>