[PATCH] Input: xpad - Move Xbox 360 magic packet sending

[PATCH] Input: xpad - Move Xbox 360 magic packet sending

[Vicki Pfau]

This moves the sending of the magic packet introduced in db7220c48d8d from
xpad_probe to xpad_start_input to ensure that xpad->dev->dev exists in the
event that an error occurs. This should also fix issues with suspend that may
occur with some controllers.

Fixes: db7220c48d8d ("Input: xpad - fix support for some third-party controllers")
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
 drivers/input/joystick/xpad.c | 43 +++++++++++++++++------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 138e4a9f341f..3eab76b2c8fc 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1721,6 +1721,27 @@ static int xpad_start_input(struct usb_xpad *xpad)
 			return error;
 		}
 	}
+	if (xpad->xtype == XTYPE_XBOX360) {
+		/*
+		 * Some third-party controllers Xbox 360-style controllers
+		 * require this message to finish initialization.
+		*/
+		u8 dummy[20];
+
+		error = usb_control_msg_recv(udev, 0,
+					     /* bRequest */ 0x01,
+					     /* bmRequestType */
+					     USB_TYPE_VENDOR | USB_DIR_IN |
+						USB_RECIP_INTERFACE,
+					     /* wValue */ 0x100,
+					     /* wIndex */ 0x00,
+					     dummy, sizeof(dummy),
+					     25, GFP_KERNEL);
+		if (error)
+			dev_warn(&xpad->dev->dev,
+				 "unable to receive magic message: %d\n",
+				 error);
+	}
 
 	return 0;
 }
@@ -2032,28 +2053,6 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 		goto err_free_in_urb;
 	}
 
-	if (xpad->xtype == XTYPE_XBOX360) {
-		/*
-		 * Some third-party controllers Xbox 360-style controllers
-		 * require this message to finish initialization.
-		*/
-		u8 dummy[20];
-
-		error = usb_control_msg_recv(udev, 0,
-					     /* bRequest */ 0x01,
-					     /* bmRequestType */
-					     USB_TYPE_VENDOR | USB_DIR_IN |
-						USB_RECIP_INTERFACE,
-					     /* wValue */ 0x100,
-					     /* wIndex */ 0x00,
-					     dummy, sizeof(dummy),
-					     25, GFP_KERNEL);
-		if (error)
-			dev_warn(&xpad->dev->dev,
-				 "unable to receive magic message: %d\n",
-				 error);
-	}
-
 	ep_irq_in = ep_irq_out = NULL;
 
 	for (i = 0; i < 2; i++) {
-- 
2.40.0

Re: [PATCH] Input: xpad - Move Xbox 360 magic packet sending

[Dan Carpenter]

On Tue, Apr 18, 2023 at 07:24:14PM -0700, Vicki Pfau wrote:
> This moves the sending of the magic packet introduced in db7220c48d8d from
> xpad_probe to xpad_start_input to ensure that xpad->dev->dev exists in the
> event that an error occurs. This should also fix issues with suspend that may
> occur with some controllers.
> 
> Fixes: db7220c48d8d ("Input: xpad - fix support for some third-party controllers")
> Signed-off-by: Vicki Pfau <vi@endrift.com>

Can you add the syzbot stuff as well and a Reported-by tag for
Dongliang Mu as well.

Reported-by: syzbot+a3f758b8d8cb7e49afec@syzkaller.appspotmail.com
Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
Link: https://groups.google.com/g/syzkaller-bugs/c/iMhTgpGuIbM

regards,
dan carpenter

Re: [PATCH] Input: xpad - Move Xbox 360 magic packet sending

[Vicki Pfau]

Hi Dmitry,

On 4/18/23 21:28, Dan Carpenter wrote:
> On Tue, Apr 18, 2023 at 07:24:14PM -0700, Vicki Pfau wrote:
>> This moves the sending of the magic packet introduced in db7220c48d8d from
>> xpad_probe to xpad_start_input to ensure that xpad->dev->dev exists in the
>> event that an error occurs. This should also fix issues with suspend that may
>> occur with some controllers.
>>
>> Fixes: db7220c48d8d ("Input: xpad - fix support for some third-party controllers")
>> Signed-off-by: Vicki Pfau <vi@endrift.com>
> 
> Can you add the syzbot stuff as well and a Reported-by tag for
> Dongliang Mu as well.
> 
> Reported-by: syzbot+a3f758b8d8cb7e49afec@syzkaller.appspotmail.com
> Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
> Link: https://groups.google.com/g/syzkaller-bugs/c/iMhTgpGuIbM

Do I need to add these to the commit myself and resend, or, barring code revisions, can this be added when you cherry-pick into the tree?

> 
> regards,
> dan carpenter
> 

Vicki

Re: [PATCH] Input: xpad - Move Xbox 360 magic packet sending

[Dmitry Torokhov]

On Fri, Apr 28, 2023 at 04:15:01PM -0700, Vicki Pfau wrote:
> Hi Dmitry,
> 
> On 4/18/23 21:28, Dan Carpenter wrote:
> > On Tue, Apr 18, 2023 at 07:24:14PM -0700, Vicki Pfau wrote:
> >> This moves the sending of the magic packet introduced in db7220c48d8d from
> >> xpad_probe to xpad_start_input to ensure that xpad->dev->dev exists in the
> >> event that an error occurs. This should also fix issues with suspend that may
> >> occur with some controllers.
> >>
> >> Fixes: db7220c48d8d ("Input: xpad - fix support for some third-party controllers")
> >> Signed-off-by: Vicki Pfau <vi@endrift.com>
> > 
> > Can you add the syzbot stuff as well and a Reported-by tag for
> > Dongliang Mu as well.
> > 
> > Reported-by: syzbot+a3f758b8d8cb7e49afec@syzkaller.appspotmail.com
> > Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
> > Link: https://groups.google.com/g/syzkaller-bugs/c/iMhTgpGuIbM
> 
> Do I need to add these to the commit myself and resend, or, barring code revisions, can this be added when you cherry-pick into the tree?
> 

This not only have not been tested on hardware, it was not even
compiled:

  CC [M]  drivers/input/joystick/xpad.o
  drivers/input/joystick/xpad.c: In function ‘xpad_start_input’:
  drivers/input/joystick/xpad.c:1733:46: error: ‘udev’ undeclared (first use in this function); did you mean ‘cdev’?
   1733 |                 error = usb_control_msg_recv(udev, 0,
        |                                              ^~~~
        |                                              cdev


At this point I will simply revert the original patch introducing the issue and
we can try landing this again once it is all rested.

Thanks.

-- 
Dmitry