Re: [PATCH 2/3] Input: xpad - add sdata_check() to xpad controllers
From: sashiko-bot@kernel.org
Date: 2026-08-03 15:46:38
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] This problem wasn't introduced by this patch, but there is a use-after-free of `input_dev` in the error path of `xpad_presence_work()` when `xpad_init_input()` fails. --
--- Patch [2]: [PATCH 2/3] Input: xpad - add sdata_check() to xpad controllers ---commit 559bf574edac1c4551addcfb6bbff7b5065fbbc5 Author: Griffin Kroah-Hartman [off-list ref] Input: xpad - add sdata_check() to xpad controllers This commit adds the sdata_check() safety wrapper to the xpad_process_packet() branch. It passes the actual length of the urb to ensure packet bounds are checked properly. This prevents out-of-bounds reads during interrupt handling for the original Xbox Controller.
quoted hunk ↗ jump to hunk
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c index c516860711a82..319a4c4a695fe 100644 --- a/drivers/input/joystick/xpad.c +++ b/drivers/input/joystick/xpad.c@@ -813,9 +813,13 @@ static void xpad360w_poweroff_controller(struct usb_xpad *xpad); * The used report descriptor was taken from ITO Takayuki's website: * http://euc.jp/periphs/xbox-controller.ja.html */ -static void xpad_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data) +static void xpad_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data, u32 len) { struct input_dev *dev = xpad->dev; + struct safe_data sdata; + + sdata.data = data; + sdata.len = len;
[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but there is a use-after-free
of input_dev in the error path of xpad_presence_work() when xpad_init_input()
fails.
In xpad_init_input(), xpad->dev is assigned to the newly allocated input_dev.
If initialization fails later, the function jumps to err_free_input and frees
input_dev, but leaves xpad->dev pointing to the freed memory:
drivers/input/joystick/xpad.c:xpad_init_input() {
...
xpad->dev = input_dev;
...
err_free_input:
input_free_device(input_dev);
return error;
}
When connecting a wireless Xbox 360 controller, xpad_presence_work() calls
xpad_init_input(). If this fails, the error path dereferences the dangling
xpad->dev pointer to log the error:
drivers/input/joystick/xpad.c:xpad_presence_work() {
...
error = xpad_init_input(xpad);
if (error) {
/* complain only, not much else we can do here */
dev_err(&xpad->dev->dev,
"unable to init device: %d\n", error);
...
}
Can we avoid dereferencing xpad->dev here, or wait to assign xpad->dev until
xpad_init_input() is fully successful?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-xpadone_packet_fix-v1-0-280da203f15c@kroah.com?part=2