public inbox for linux-input@vger.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Tissoires <bentiss@kernel.org>
To: Zhouwang Huang <honjow311@gmail.com>
Cc: Jiri Kosina <jikos@kernel.org>,
	denis.benato@linux.dev,  linux-kernel@vger.kernel.org,
	linux-input@vger.kernel.org
Subject: Re: [PATCH] HID: core: add short report quirk and use it for GPD Win (2f24:0137)
Date: Wed, 8 Apr 2026 17:41:28 +0200	[thread overview]
Message-ID: <adZ2PTzQ05iObcGt@beelink> (raw)
In-Reply-To: <20260407173212.86942-1-honjow311@gmail.com>

Hi,

Definitely not a big fan of that new quirk. The issue is that current
hid-core fix is too restrictive because it doesn't have enough
information, like the actual allocated buffer size.

On Apr 08 2026, Zhouwang Huang wrote:
> Commit 9e2a17d2e808 ("HID: gpd: fix report descriptor on GPD Win
> handheld (2f24:0137)") used report_fixup to shrink Report Count from
> 63 to 11 so that short reports from firmware <= 1.09 would not be
> rejected by hid_report_raw_event().
> 
> However, firmware 1.10 fixed the report length and now sends the full
> 63 bytes.  Because report_fixup already shrank the descriptor,
> usbhid allocates a 12-byte URB buffer — far too small for the 64-byte
> transfer — causing continuous -EOVERFLOW on every interrupt-in URB.
> The HID report descriptor and bcdDevice are identical across firmware
> versions, so report_fixup cannot conditionally apply.

OK, so if a firmware already fixed the bug, I'll drop 9e2a17d2e808 from
for-next and not include it in the final 7.0 PR I'll need to send. We
can tell people to upgrade to the latest firmware while I work on a
proper fix.

BTW, technically we could also not change the report descriptor if we
detect the correct firmware version. That would be easier to implement
than a global quirk. But hopefully we won't have to do anything and get
the proper hid-core fix soon.

Cheers,
Benjamin

> 
> Replace the report_fixup driver with a new per-device quirk
> HID_QUIRK_ALLOW_SHORT_REPORTS.  When set, hid_report_raw_event()
> zero-pads short reports instead of rejecting them — the same behaviour
> the core had before commit 0a3fe972a7cb ("HID: core: Mitigate
> potential OOB by removing bogus memset()").  The descriptor is left
> unmodified so the URB buffer matches the declared report size and works
> with any firmware version.
> 
> Remove hid-gpd.c, its Kconfig entry and Makefile line; the device is
> now handled by hid-generic with the quirk applied from hid-quirks.c.
> 
> Fixes: 9e2a17d2e808 ("HID: gpd: fix report descriptor on GPD Win handheld (2f24:0137)")
> Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
> ---
>  drivers/hid/Kconfig      | 10 --------
>  drivers/hid/Makefile     |  1 -
>  drivers/hid/hid-core.c   | 11 +++++----
>  drivers/hid/hid-gpd.c    | 52 ----------------------------------------
>  drivers/hid/hid-quirks.c |  2 ++
>  include/linux/hid.h      |  2 ++
>  6 files changed, 11 insertions(+), 67 deletions(-)
>  delete mode 100644 drivers/hid/hid-gpd.c
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 2159d0fb7020..c1d9f7c6a5f2 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -419,16 +419,6 @@ config HID_GLORIOUS
>  	  Support for Glorious PC Gaming Race mice such as
>  	  the Glorious Model O, O- and D.
>  
> -config HID_GPD
> -	tristate "GPD Win handheld OEM HID support"
> -	depends on USB_HID
> -	help
> -	  Report descriptor fix for the OEM USB HID interface (GameSir
> -	  2f24:0137) found on GPD Win handhelds. The firmware declares 63-byte
> -	  reports but only sends 11 bytes, which the HID core rejects.
> -
> -	  Say Y or M here if you use a GPD Win handheld with this interface.
> -
>  config HID_HOLTEK
>  	tristate "Holtek HID devices"
>  	depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index f69cd6015465..e01838239ae6 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -53,7 +53,6 @@ obj-$(CONFIG_HID_ELO)		+= hid-elo.o
>  obj-$(CONFIG_HID_EVISION)	+= hid-evision.o
>  obj-$(CONFIG_HID_EZKEY)		+= hid-ezkey.o
>  obj-$(CONFIG_HID_FT260)		+= hid-ft260.o
> -obj-$(CONFIG_HID_GPD)		+= hid-gpd.o
>  obj-$(CONFIG_HID_GEMBIRD)	+= hid-gembird.o
>  obj-$(CONFIG_HID_GFRM)		+= hid-gfrm.o
>  obj-$(CONFIG_HID_GLORIOUS)  += hid-glorious.o
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index f5587b786f87..52e86f927a38 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2057,10 +2057,13 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
>  		rsize = max_buffer_size;
>  
>  	if (csize < rsize) {
> -		hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %d)\n",
> -				     report->id, rsize, csize);
> -		ret = -EINVAL;
> -		goto out;
> +		if (!(hid->quirks & HID_QUIRK_ALLOW_SHORT_REPORTS)) {
> +			hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %d)\n",
> +					     report->id, rsize, csize);
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +		memset(cdata + csize, 0, rsize - csize);
>  	}
>  
>  	if ((hid->claimed & HID_CLAIMED_HIDDEV) && hid->hiddev_report_event)
> diff --git a/drivers/hid/hid-gpd.c b/drivers/hid/hid-gpd.c
> deleted file mode 100644
> index 5b4d203e2499..000000000000
> --- a/drivers/hid/hid-gpd.c
> +++ /dev/null
> @@ -1,52 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-or-later
> -/*
> - *  HID report descriptor fixup for GPD Win handhelds.
> - *
> - *  The OEM HID interface (VID 2f24 / GameSir, PID 0137) declares Report ID 1
> - *  with Report Count 63 (8-bit fields) for both Input and Feature, but the
> - *  firmware only sends 11 bytes of payload after the report ID.
> - */
> -
> -#include <linux/hid.h>
> -#include <linux/module.h>
> -
> -#include "hid-ids.h"
> -
> -#define RDESC_LEN		38
> -#define RPT_COUNT_INPUT_OFF	21
> -#define RPT_COUNT_FEATURE_OFF	34
> -
> -static const __u8 *gpd_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> -				    unsigned int *rsize)
> -{
> -	if (*rsize != RDESC_LEN)
> -		return rdesc;
> -
> -	if (rdesc[RPT_COUNT_INPUT_OFF - 1] == 0x95 &&
> -	    rdesc[RPT_COUNT_INPUT_OFF] == 0x3f &&
> -	    rdesc[RPT_COUNT_FEATURE_OFF - 1] == 0x95 &&
> -	    rdesc[RPT_COUNT_FEATURE_OFF] == 0x3f) {
> -		hid_info(hdev, "fixing report counts (63 -> 11 bytes)\n");
> -		rdesc[RPT_COUNT_INPUT_OFF] = 11;
> -		rdesc[RPT_COUNT_FEATURE_OFF] = 11;
> -	}
> -
> -	return rdesc;
> -}
> -
> -static const struct hid_device_id gpd_devices[] = {
> -	{ HID_USB_DEVICE(USB_VENDOR_ID_GAMESIR, USB_DEVICE_ID_GAMESIR_0137) },
> -	{ }
> -};
> -MODULE_DEVICE_TABLE(hid, gpd_devices);
> -
> -static struct hid_driver gpd_driver = {
> -	.name = "gpd",
> -	.id_table = gpd_devices,
> -	.report_fixup = gpd_report_fixup,
> -};
> -
> -module_hid_driver(gpd_driver);
> -
> -MODULE_DESCRIPTION("HID report descriptor fix for GPD Win handheld (GameSir 2f24:0137)");
> -MODULE_LICENSE("GPL");
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index f6be3ffee023..b9ae1442eba9 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -97,6 +97,8 @@ static const struct hid_device_id hid_quirks[] = {
>  		HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_GAMEVICE, USB_DEVICE_ID_GAMEVICE_KISHI),
>  		HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE },
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_GAMESIR, USB_DEVICE_ID_GAMESIR_0137),
> +		HID_QUIRK_ALLOW_SHORT_REPORTS },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_HAPP, USB_DEVICE_ID_UGCI_DRIVING), HID_QUIRK_BADPAD | HID_QUIRK_MULTI_INPUT },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_HAPP, USB_DEVICE_ID_UGCI_FIGHTING), HID_QUIRK_BADPAD | HID_QUIRK_MULTI_INPUT },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_HAPP, USB_DEVICE_ID_UGCI_FLYING), HID_QUIRK_BADPAD | HID_QUIRK_MULTI_INPUT },
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 31324609af4d..212dd13bfcfa 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -381,6 +381,7 @@ struct hid_item {
>   * | @HID_QUIRK_X_INVERT:
>   * | @HID_QUIRK_Y_INVERT:
>   * | @HID_QUIRK_IGNORE_MOUSE:
> + * | @HID_QUIRK_ALLOW_SHORT_REPORTS: accept shorter-than-expected reports, zero-pad
>   * | @HID_QUIRK_SKIP_OUTPUT_REPORTS:
>   * | @HID_QUIRK_SKIP_OUTPUT_REPORT_ID:
>   * | @HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP:
> @@ -408,6 +409,7 @@ struct hid_item {
>  #define HID_QUIRK_X_INVERT			BIT(12)
>  #define HID_QUIRK_Y_INVERT			BIT(13)
>  #define HID_QUIRK_IGNORE_MOUSE			BIT(14)
> +#define HID_QUIRK_ALLOW_SHORT_REPORTS		BIT(15)
>  #define HID_QUIRK_SKIP_OUTPUT_REPORTS		BIT(16)
>  #define HID_QUIRK_SKIP_OUTPUT_REPORT_ID		BIT(17)
>  #define HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP	BIT(18)
> -- 
> 2.53.0
> 
> 

  reply	other threads:[~2026-04-08 15:41 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-07 17:32 [PATCH] HID: core: add short report quirk and use it for GPD Win (2f24:0137) Zhouwang Huang
2026-04-08 15:41 ` Benjamin Tissoires [this message]
2026-04-08 16:07   ` Zhouwang Huang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=adZ2PTzQ05iObcGt@beelink \
    --to=bentiss@kernel.org \
    --cc=denis.benato@linux.dev \
    --cc=honjow311@gmail.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox