Re: [PATCH v2] Input: xpad - fix button mapping on Razer Atrox Xbox One

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

Pre-existing issues:
- [High] Handling GIP_CMD_ANNOUNCE in the URB completion handler calls xpad_start_input(), which sleeps and double-submits the URB.
--

--- Patch [1]: [PATCH v2] Input: xpad - fix button mapping on Razer Atrox Xbox One ---
commit f5450a413687f18b1cfa1f3ba3fe1938a5e5c72f
Author: Vernon Di Carlo <[email protected]>

Input: xpad - fix button mapping on Razer Atrox Xbox One

This patch adds an Atrox-specific mapping quirk to swap LB/RB and report
LT/RT from their actual locations for the Razer Atrox Arcade Stick for Xbox
One. It also rejects short Atrox input packets.

> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index feb8f368f834e..00de9145e0916 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
[ ... ]
> @@ -1079,6 +1081,9 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
>  					 error);
>  		}

[Severity: High]
This is a pre-existing issue, but while looking at this context, I noticed
that handling GIP_CMD_ANNOUNCE here calls xpad_start_input(), which appears
to sleep in atomic context and double-submit the URB.

Since xpadone_process_packet() is called from the URB completion handler
xpad_irq_in() (atomic context), does calling xpad_start_input() trigger a
sleeping in atomic context bug?

Looking at xpad_start_input(), it makes a GFP_KERNEL allocation and submits
the URB:

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

Then when it returns, xpad_irq_in() subsequently attempts to submit the same
URB a second time:

    retval = usb_submit_urb(urb, GFP_ATOMIC);

Could this lead to deadlocks or panics, and potentially corrupt the USB
core state due to the double submission?

>  	} else if (data[0] == GIP_CMD_INPUT) { /* The main valid packet type for inputs */
> +		if ((xpad->mapping & MAP_ATROX) && len <= 22)
> +			return;
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1