[PATCH v2 1/2] Input: xpad - query MS OS descriptor for BTP-KP20D

[PATCH v2 1/2] Input: xpad - query MS OS descriptor for BTP-KP20D

[Aaron Ma]

The Beitong BTP-KP20D dongle cycles through its operating modes and
never becomes usable unless the host requests the Microsoft OS string
descriptor at index 0xee. Windows makes this request while discovering
the XUSB10 compatible ID, but Linux USB core does not query Microsoft
OS descriptors.

Add a device-specific quirk that performs the request during probe,
allowing the dongle to remain in Xbox 360 mode.

Signed-off-by: Aaron Ma <mapengyu@gmail.com>
---
 drivers/input/joystick/xpad.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index feb8f368f834e..1616047e65f5e 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -72,6 +72,7 @@
 #define PKT_XBE2_FW_5_11    4
 
 #define FLAG_DELAY_INIT BIT(0)
+#define FLAG_MS_OS_DESC BIT(1)
 
 static bool dpad_to_buttons;
 module_param(dpad_to_buttons, bool, S_IRUGO);
@@ -334,6 +335,7 @@ static const struct xpad_device {
 	{ 0x1ee9, 0x1590, "ZOTAC Gaming Zone", 0, XTYPE_XBOX360 },
 	{ 0x20bc, 0x5134, "BETOP BTP-KP50B Xinput Dongle", 0, XTYPE_XBOX360 },
 	{ 0x20bc, 0x514a, "BETOP BTP-KP50C Xinput Dongle", 0, XTYPE_XBOX360 },
+	{ 0x20bc, 0x5159, "Beitong BTP-KP20D Controller", 0, XTYPE_XBOX360, FLAG_MS_OS_DESC },
 	{ 0x20d6, 0x2001, "BDA Xbox Series X Wired Controller", 0, XTYPE_XBOXONE },
 	{ 0x20d6, 0x2009, "PowerA Enhanced Wired Controller for Xbox Series X|S", 0, XTYPE_XBOXONE },
 	{ 0x20d6, 0x2064, "PowerA Wired Controller for Xbox", MAP_SHARE_BUTTON, XTYPE_XBOXONE },
@@ -786,6 +788,24 @@ static int xpad_start_input(struct usb_xpad *xpad);
 static void xpadone_ack_mode_report(struct usb_xpad *xpad, u8 seq_num);
 static void xpad360w_poweroff_controller(struct usb_xpad *xpad);
 
+static void xpad_query_ms_os_descriptor(struct usb_device *udev)
+{
+	u8 *descriptor;
+	int error;
+
+	descriptor = kmalloc(18, GFP_KERNEL);
+	if (!descriptor)
+		return;
+
+	error = usb_get_descriptor(udev, USB_DT_STRING, 0xee,
+				   descriptor, 18);
+	if (error != 18)
+		dev_warn(&udev->dev,
+			 "unable to query MS OS descriptor: %d\n", error);
+
+	kfree(descriptor);
+}
+
 /*
  *	xpad_process_packet
  *
@@ -2055,6 +2075,9 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 			break;
 	}
 
+	if (xpad_device[i].flags & FLAG_MS_OS_DESC)
+		xpad_query_ms_os_descriptor(udev);
+
 	xpad = kzalloc_obj(*xpad);
 	if (!xpad)
 		return -ENOMEM;
-- 
2.53.0

[PATCH v2 2/2] Input: xpad - add wired Beitong BTP-KP20D support

[Aaron Ma]

The wired BTP-KP20D resets itself when its interrupt-IN pipe is idle
for a few seconds, causing periodic re-enumeration whenever no
userspace client holds the input device open. Add a FLAG_KEEPALIVE
quirk that keeps the input URB running from probe to disconnect,
mirroring the XTYPE_XBOX360W URB lifecycle.

The device also needs the MS OS descriptor query to stay in Xbox 360
mode.

Stop any active output URB on probe failure before freeing its URB and
DMA buffer. LED setup may submit the output URB before input
initialization completes.

Signed-off-by: Aaron Ma <mapengyu@gmail.com>
---
Changes in v2:
- Stop the output URB before freeing it on probe failure.

 drivers/input/joystick/xpad.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 1616047e65f5e..9adc419066353 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -73,6 +73,7 @@
 
 #define FLAG_DELAY_INIT BIT(0)
 #define FLAG_MS_OS_DESC BIT(1)
+#define FLAG_KEEPALIVE  BIT(2)
 
 static bool dpad_to_buttons;
 module_param(dpad_to_buttons, bool, S_IRUGO);
@@ -335,6 +336,7 @@ static const struct xpad_device {
 	{ 0x1ee9, 0x1590, "ZOTAC Gaming Zone", 0, XTYPE_XBOX360 },
 	{ 0x20bc, 0x5134, "BETOP BTP-KP50B Xinput Dongle", 0, XTYPE_XBOX360 },
 	{ 0x20bc, 0x514a, "BETOP BTP-KP50C Xinput Dongle", 0, XTYPE_XBOX360 },
+	{ 0x20bc, 0x5158, "Beitong BTP-KP20D Controller", 0, XTYPE_XBOX360, FLAG_MS_OS_DESC | FLAG_KEEPALIVE },
 	{ 0x20bc, 0x5159, "Beitong BTP-KP20D Controller", 0, XTYPE_XBOX360, FLAG_MS_OS_DESC },
 	{ 0x20d6, 0x2001, "BDA Xbox Series X Wired Controller", 0, XTYPE_XBOXONE },
 	{ 0x20d6, 0x2009, "PowerA Enhanced Wired Controller for Xbox Series X|S", 0, XTYPE_XBOXONE },
@@ -780,6 +782,7 @@ struct usb_xpad {
 	time64_t mode_btn_down_ts;
 	bool delay_init;		/* init packets should be delayed */
 	bool delayed_init_done;
+	bool keepalive;			/* int URB must run from probe to disconnect */
 };
 
 static int xpad_init_input(struct usb_xpad *xpad);
@@ -1971,7 +1974,7 @@ static int xpad_init_input(struct usb_xpad *xpad)
 
 	input_set_drvdata(input_dev, xpad);
 
-	if (xpad->xtype != XTYPE_XBOX360W) {
+	if (xpad->xtype != XTYPE_XBOX360W && !xpad->keepalive) {
 		input_dev->open = xpad_open;
 		input_dev->close = xpad_close;
 	}
@@ -2105,6 +2108,8 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 	xpad->name = xpad_device[i].name;
 	if (xpad_device[i].flags & FLAG_DELAY_INIT)
 		xpad->delay_init = true;
+	if (xpad_device[i].flags & FLAG_KEEPALIVE)
+		xpad->keepalive = true;
 
 	xpad->packet_type = PKT_XB;
 	INIT_WORK(&xpad->work, xpad_presence_work);
@@ -2227,10 +2232,24 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 		error = xpad_init_input(xpad);
 		if (error)
 			goto err_deinit_output;
+
+		if (xpad->keepalive) {
+			/*
+			 * The device resets itself when the interrupt-IN pipe
+			 * is idle for a few seconds, so keep the URB running
+			 * from probe until disconnect.
+			 */
+			error = xpad_start_input(xpad);
+			if (error)
+				goto err_deinit_input;
+		}
 	}
 	return 0;
 
+err_deinit_input:
+	xpad_deinit_input(xpad);
 err_deinit_output:
+	xpad_stop_output(xpad);
 	xpad_deinit_output(xpad);
 err_free_in_urb:
 	usb_free_urb(xpad->irq_in);
@@ -2247,6 +2266,8 @@ static void xpad_disconnect(struct usb_interface *intf)
 
 	if (xpad->xtype == XTYPE_XBOX360W)
 		xpad360w_stop_input(xpad);
+	else if (xpad->keepalive)
+		xpad_stop_input(xpad);
 
 	xpad_deinit_input(xpad);
 
@@ -2288,6 +2309,8 @@ static int xpad_suspend(struct usb_interface *intf, pm_message_t message)
 		 */
 		if (auto_poweroff && xpad->pad_present)
 			xpad360w_poweroff_controller(xpad);
+	} else if (xpad->keepalive) {
+		xpad_stop_input(xpad);
 	} else {
 		guard(mutex)(&input->mutex);
 
@@ -2309,6 +2332,9 @@ static int xpad_resume(struct usb_interface *intf)
 	if (xpad->xtype == XTYPE_XBOX360W)
 		return xpad360w_start_input(xpad);
 
+	if (xpad->keepalive)
+		return xpad_start_input(xpad);
+
 	guard(mutex)(&input->mutex);
 
 	if (input_device_enabled(input))
-- 
2.53.0

Re: [PATCH v2 1/2] Input: xpad - query MS OS descriptor for BTP-KP20D

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The variable `error` holds positive values on success, violating subsystem naming conventions.
- [Low] Missed opportunity to use modern cleanup primitives for the descriptor buffer.
--

--- Patch [1]: [PATCH v2 1/2] Input: xpad - query MS OS descriptor for BTP-KP20D ---
commit eab43c4126d684c849995489e35829f7af7fb51c
Author: Aaron Ma <mapengyu@gmail.com>

Input: xpad - query MS OS descriptor for BTP-KP20D

This commit adds a quirk to query the MS OS descriptor for the Beitong
BTP-KP20D dongle during probe. This allows the device to stay in Xbox 360
mode.

> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index feb8f368f834e..1616047e65f5e 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
[ ... ]
> @@ -786,6 +788,24 @@ static int xpad_start_input(struct usb_xpad *xpad);
>  static void xpadone_ack_mode_report(struct usb_xpad *xpad, u8 seq_num);
>  static void xpad360w_poweroff_controller(struct usb_xpad *xpad);
>  
> +static void xpad_query_ms_os_descriptor(struct usb_device *udev)
> +{
> +	u8 *descriptor;
> +	int error;

[Severity: Low]
This isn't a bug, but does the error variable receive a positive value here?
The input subsystem maintainer prefers that variables named error or err
should only hold negative error codes or 0 for success. Since
usb_get_descriptor() returns the number of bytes transferred on success,
perhaps a different variable name would be better?

> +
> +	descriptor = kmalloc(18, GFP_KERNEL);

[Severity: Low]
This isn't a bug, but could this use the __free(kfree) primitive for
the allocation to automate cleanup and remove the explicit kfree(descriptor)
below, as encouraged by the maintainer preferences for new code?

> +	if (!descriptor)
> +		return;
> +
> +	error = usb_get_descriptor(udev, USB_DT_STRING, 0xee,
> +				   descriptor, 18);
> +	if (error != 18)
> +		dev_warn(&udev->dev,
> +			 "unable to query MS OS descriptor: %d\n", error);
> +
> +	kfree(descriptor);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801075623.2769601-1-mapengyu@gmail.com?part=1

Re: [PATCH v2 2/2] Input: xpad - add wired Beitong BTP-KP20D support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing workqueue cancellation in xpad_probe error path leads to Use-After-Free.
- [High] Sleeping in atomic context when processing GIP_CMD_ANNOUNCE from the URB completion handler.
--

--- Patch [2]: [PATCH v2 2/2] Input: xpad - add wired Beitong BTP-KP20D support ---
commit 600bda7a2ef7ea6aabd37796bfb7164b015f813b
Author: Aaron Ma <mapengyu@gmail.com>

Input: xpad - add wired Beitong BTP-KP20D support

This patch adds support for the wired Beitong BTP-KP20D controller and
introduces a FLAG_KEEPALIVE quirk. The quirk keeps the input URB running
from probe to disconnect to prevent the device from resetting itself when idle.

> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index 1616047e65f5e..9adc419066353 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c

[ ... ]

> @@ -2227,10 +2232,24 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  		error = xpad_init_input(xpad);
>  		if (error)
>  			goto err_deinit_output;
> +
> +		if (xpad->keepalive) {
> +			/*
> +			 * The device resets itself when the interrupt-IN pipe
> +			 * is idle for a few seconds, so keep the URB running
> +			 * from probe until disconnect.
> +			 */
> +			error = xpad_start_input(xpad);

[Severity: High]
This is a pre-existing issue, but does xpad_start_input() sleep in atomic
context when called from the URB completion handler?

For XTYPE_XBOXONE devices with FLAG_DELAY_INIT, a GIP_CMD_ANNOUNCE packet
triggers xpadone_process_packet() from the URB completion handler xpad_irq_in(),
which executes in an atomic context (softirq).

xpadone_process_packet() then invokes xpad_start_input():

	if (xpad->delay_init && !xpad->delayed_init_done) {
		xpad->delayed_init_done = true;
		error = xpad_start_input(xpad);

xpad_start_input() calls usb_submit_urb() with GFP_KERNEL, which can sleep:

	if (usb_submit_urb(xpad->irq_in, GFP_KERNEL))
		return -EIO;

It also calls xpad_start_xbox_one(), which invokes usb_set_interface() to
disable the audio interface using synchronous USB control messages that can wait
and sleep.

Can this trigger a kernel bug or panic if a malicious USB device sends a
GIP_CMD_ANNOUNCE packet?

> +			if (error)
> +				goto err_deinit_input;
> +		}
>  	}
>  	return 0;
>  
> +err_deinit_input:
> +	xpad_deinit_input(xpad);
>  err_deinit_output:
> +	xpad_stop_output(xpad);
>  	xpad_deinit_output(xpad);
>  err_free_in_urb:

[Severity: High]
This is a pre-existing issue, but is it possible for xpad_probe() to free the
xpad structure without canceling the workqueue on the error path?

For XTYPE_XBOX360W devices, xpad_probe() calls xpad360w_start_input(), which
submits the irq_in URB and then calls xpad_inquiry_pad_presence():

	error = xpad_inquiry_pad_presence(xpad);
	if (error) {
		usb_kill_urb(xpad->irq_in);
		return error;
	}

If a malicious or buggy device responds immediately via the IN endpoint,
xpad_irq_in() will run and xpad360w_process_packet() may schedule xpad->work.

If xpad_inquiry_pad_presence() fails, xpad360w_start_input() kills the URB and
returns an error without canceling the scheduled work.

xpad_probe() then takes the err_deinit_output path:

	if (xpad->xtype == XTYPE_XBOX360W) {
		...
		error = xpad360w_start_input(xpad);
		if (error)
			goto err_deinit_output;

This eventually frees the xpad structure via kfree(xpad).

Can the workqueue execute xpad_presence_work() later, causing a Use-After-Free
when accessing the freed xpad structure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801075623.2769601-1-mapengyu@gmail.com?part=2