[PATCH v2] HID: nintendo: stop device IO before hid_hw_stop on probe failure

[PATCH v2] HID: nintendo: stop device IO before hid_hw_stop on probe failure

[Jiangshan Yi]

nintendo_hid_probe() calls hid_device_io_start() before joycon_init()
and joycon_leds_create().  If either fails, the error path jumps to
err_close which calls hid_hw_close()/hid_hw_stop() without first calling
hid_device_io_stop().

hid_hw_stop() does not stop device IO, so hid_input_report() may still
run and access driver data that is being torn down, resulting in a
use-after-free.

Add an err_io_stop label that calls hid_device_io_stop() before
hid_hw_close(), and point the two post-io_start error paths at it.

Fixes: 2af16c1f846b ("HID: nintendo: add nintendo switch controller driver")
Cc: stable@vger.kernel.org
Signed-off-by: Jiangshan Yi <yijiangshan@kylinos.cn>
---
  Changes in v2:
  - Resend as a standalone patch.  v1 was accidentally sent twice,
    once as [PATCH 3/6] (part of an unrelated series) and once as
    [PATCH], which confused the Sashiko bot into attempting to apply
    both copies simultaneously.  No code changes.

  v1: https://lore.kernel.org/all/20260728061132.299461-1-yijiangshan@kylinos.cn/

 drivers/hid/hid-nintendo.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index e7302ec01ff1..0db553528826 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -2736,14 +2736,14 @@ static int nintendo_hid_probe(struct hid_device *hdev,
 	ret = joycon_init(hdev);
 	if (ret) {
 		hid_err(hdev, "Failed to initialize controller; ret=%d\n", ret);
-		goto err_close;
+		goto err_io_stop;
 	}
 
 	/* Initialize the leds */
 	ret = joycon_leds_create(ctlr);
 	if (ret) {
 		hid_err(hdev, "Failed to create leds; ret=%d\n", ret);
-		goto err_close;
+		goto err_io_stop;
 	}
 
 	/* Initialize the battery power supply */
@@ -2766,7 +2766,8 @@ static int nintendo_hid_probe(struct hid_device *hdev,
 
 err_ida:
 	ida_free(&nintendo_player_id_allocator, ctlr->player_id);
-err_close:
+err_io_stop:
+	hid_device_io_stop(hdev);
 	hid_hw_close(hdev);
 err_stop:
 	hid_hw_stop(hdev);
-- 
2.25.1

Re: [PATCH v2] HID: nintendo: stop device IO before hid_hw_stop on probe failure

[sashiko-bot]

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

Pre-existing issues:
- [High] The driver registers the input device before fully initializing its capabilities and force-feedback callbacks, violating subsystem rules and introducing a race condition that can lead to a NULL pointer dereference and broken userspace device recognition.
--

--- Patch [1]: [PATCH v2] HID: nintendo: stop device IO before hid_hw_stop on probe failure ---
commit 175b2ac9211bb7f8a122c8a3e3d05eeac1ea8dac
Author: Jiangshan Yi <yijiangshan@kylinos.cn>

HID: nintendo: stop device IO before hid_hw_stop on probe failure

nintendo_hid_probe() calls hid_device_io_start() before joycon_init()
and joycon_leds_create().  If either fails, the error path jumps to
err_close which calls hid_hw_close()/hid_hw_stop() without first calling
hid_device_io_stop().

hid_hw_stop() does not stop device IO, so hid_input_report() may still
run and access driver data that is being torn down, resulting in a
use-after-free.

> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index e7302ec01ff1f..0db5535288263 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
[ ... ]
> @@ -2766,7 +2766,8 @@ static int nintendo_hid_probe(struct hid_device *hdev,
>  
>  err_ida:
>  	ida_free(&nintendo_player_id_allocator, ctlr->player_id);
> -err_close:
> +err_io_stop:
> +	hid_device_io_stop(hdev);
>  	hid_hw_close(hdev);
>  err_stop:
>  	hid_hw_stop(hdev);

[Severity: High]
This is a pre-existing issue, but I noticed a potential race condition in how
the input device is registered in joycon_input_create(), which is called 
slightly further down in nintendo_hid_probe().

In joycon_input_create(), input_register_device() is called before the device 
capabilities and force-feedback callbacks are fully configured:

    ret = input_register_device(ctlr->input);
    if (ret)
        return ret;

    if (joycon_type_is_right_joycon(ctlr)) {
        joycon_config_right_stick(ctlr->input);
...
    if (joycon_has_rumble(ctlr))
        joycon_config_rumble(ctlr);

And in joycon_config_rumble():

    input_set_capability(ctlr->input, EV_FF, FF_RUMBLE);
    input_ff_create_memless(ctlr->input, NULL, joycon_play_effect);

Because input_register_device() makes the device visible to userspace 
immediately, couldn't a userspace process open the device and trigger the 
EVIOCSFF ioctl concurrently?

If evdev calls input_ff_upload() while joycon_config_rumble() is running, it
might observe the EV_FF bit but dereference an uninitialized dev->ff,
potentially causing a NULL pointer dereference. Additionally, registering
before setting capabilities means the initial udev event might lack button
and axis information, which could cause input managers like Wayland to
ignore the device.

Should the call to input_register_device() be moved to the end of 
joycon_input_create() after all capabilities and force-feedback are 
initialized?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730031927.25444-1-yijiangshan@kylinos.cn?part=1