HID: nintendo: fix out-of-bounds read in joycon_ctlr_read_handler()

PATCH Applied Ibrahim Hashimov 4 participants 4 messages Jul 15 – Aug 3

Subsystems (3)

Stable branch HID Nintendo HID driver

lore

Share & tools

lore

Ibrahim Hashimov

author

2026-07-15 11:53 UTC #0 lore raw

PATCH 1/1 HID: nintendo: fix out-of-bounds read in joycon_ctlr_read_handler()

joycon_ctlr_read_handler() casts an incoming HID input report to struct joycon_input_report and parses it, guarding the cast only with a 12-byte length check:

	if (size >= 12) /* make sure it contains the input report */
		joycon_parse_report(ctlr, (struct joycon_input_report *)data);

struct joycon_input_report is 49 bytes: a 13-byte header followed by a union whose IMU arm is 36 bytes. For an IMU report joycon_parse_report() -> joycon_parse_imu_report() walks that union (struct offsets 13..48), so a report of exactly 12 bytes with data[0] == JC_INPUT_IMU_DATA passes the guard yet is read up to 37 bytes past its declared length. The over-read bytes are decoded into accelerometer/gyroscope values and forwarded to userspace through the "(IMU)" input device, leaking driver-internal memory. data[0] and size are fully controlled by a malicious or spoofed Joy-Con/Pro Controller.

Receive buffers are sized to the maximum report length, so this is an over-read within the allocation rather than a slab OOB, but the decoded bytes still reach userspace.

The sibling subcmd path in joycon_ctlr_handle_event() already bounds the same cast correctly:

	if (size < sizeof(struct joycon_input_report) ||
	    data[0] != JC_INPUT_SUBCMD_REPLY)
		break;

Use the same sizeof(struct joycon_input_report) bound here.

Fixes 2af16c1f846b ("HID: nintendo: add nintendo switch controller driver")

Cc [email protected]

Signed-off-by Ibrahim Hashimov <[email protected]>

Assisted-by AuditCode-AI:2026.07

---
 drivers/hid/hid-nintendo.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

drivers/hid/hid-nintendo.c +6 -1

index e7302ec01ff1..11b5fe05acf4 100644
@@ -2607,7 +2607,12 @@ static int joycon_ctlr_read_handler(struct joycon_ctlr *ctlr, u8 *data,
 {
 	if (data[0] == JC_INPUT_SUBCMD_REPLY || data[0] == JC_INPUT_IMU_DATA ||
 	    data[0] == JC_INPUT_MCU_DATA) {
-		if (size >= 12) /* make sure it contains the input report */
+		/*
+		 * The whole struct is cast and parsed below, including the
+		 * IMU/subcmd union, not just the 12-byte partial header this
+		 * used to check for.
+		 */
+		if (size >= sizeof(struct joycon_input_report))
 			joycon_parse_report(ctlr,
 					    (struct joycon_input_report *)data);
 	}

Signature · 1 line

2.50.1 (Apple Git-155)

[email protected]

AI review

2026-07-15 12:15 UTC #1 lore raw

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

Pre-existing issues:
- [High] The input device is registered before its capabilities are fully initialized. --

--- Patch [1]: [PATCH] HID: nintendo: fix out-of-bounds read in joycon_ctlr_read_handler() --- commit 4a13815d9787a8e4509f8221af788267db0c1886
Author: Ibrahim Hashimov <[email protected]>

HID: nintendo: fix out-of-bounds read in joycon_ctlr_read_handler()

This commit changes a bounds check in joycon_ctlr_read_handler() from 12 bytes to the full size of struct joycon_input_report. This prevents an out-of-bounds read when decoding IMU reports that are smaller than the full structure size.

Quoted 8 lines

drivers/hid/hid-nintendo.c +0 -0

index e7302ec01ff1f..11b5fe05acf44 100644
@@ -2607,7 +2607,12 @@ static int joycon_ctlr_read_handler(struct joycon_ctlr *ctlr, u8 *data,
 {
 	if (data[0] == JC_INPUT_SUBCMD_REPLY || data[0] == JC_INPUT_IMU_DATA ||
 	    data[0] == JC_INPUT_MCU_DATA) {

drivers/hid/hid-nintendo.c static int joycon_ctlr_read_handler(struct joycon_ctlr *ctlr, u8 *data, · line 2618

Quoted 10 lines

drivers/hid/hid-nintendo.c +6 -1

-		if (size >= 12) /* make sure it contains the input report */
+		/*
+		 * The whole struct is cast and parsed below, including the
+		 * IMU/subcmd union, not just the 12-byte partial header this
+		 * used to check for.
+		 */
+		if (size >= sizeof(struct joycon_input_report))
 			joycon_parse_report(ctlr,
 					    (struct joycon_input_report *)data);
 	}

While reviewing this file, I noticed a pre-existing issue in joycon_input_create():

drivers/hid/hid-nintendo.c:joycon_input_create() { [ ... ]

	ctlr->input->phys = hdev->phys;
	input_set_drvdata(ctlr->input, ctlr);

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

	if (joycon_type_is_right_joycon(ctlr)) {
		joycon_config_right_stick(ctlr->input);

[ ... ]
}

[Severity: High]
This is a pre-existing issue, but is the input device registered here before its capabilities are configured?

Calling input_register_device() before setting the keys and absolute axes results in an empty capability map during registration. This causes the joydev handler to reject the device since EV_KEY and EV_ABS are not set.

Could this lead to udev exporting empty capabilities, and evdev calculating an incorrect event buffer size, causing applications to fail to reliably recognize the controller?

Signature · 1 line

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

Silvan Jegen

Reviewed-by on v1 1/1

2026-07-22 15:31 UTC #2 lore raw

In reply to Ibrahim Hashimov

heyhey!

Quoted 30 lines · Ibrahim Hashimov

joycon_ctlr_read_handler() casts an incoming HID input report to struct joycon_input_report and parses it, guarding the cast only with a 12-byte length check:
	if (size >= 12) /* make sure it contains the input report */
		joycon_parse_report(ctlr, (struct joycon_input_report *)data);
struct joycon_input_report is 49 bytes: a 13-byte header followed by a union whose IMU arm is 36 bytes. For an IMU report joycon_parse_report() -> joycon_parse_imu_report() walks that union (struct offsets 13..48), so a report of exactly 12 bytes with data[0] == JC_INPUT_IMU_DATA passes the guard yet is read up to 37 bytes past its declared length. The over-read bytes are decoded into accelerometer/gyroscope values and forwarded to userspace through the "(IMU)" input device, leaking driver-internal memory. data[0] and size are fully controlled by a malicious or spoofed Joy-Con/Pro Controller.
Receive buffers are sized to the maximum report length, so this is an over-read within the allocation rather than a slab OOB, but the decoded bytes still reach userspace.
The sibling subcmd path in joycon_ctlr_handle_event() already bounds the same cast correctly:
	if (size < sizeof(struct joycon_input_report) ||
	    data[0] != JC_INPUT_SUBCMD_REPLY)
		break;
Use the same sizeof(struct joycon_input_report) bound here.

Quoted 7 lines

Fixes 2af16c1f846b ("HID: nintendo: add nintendo switch controller driver")
Cc [email protected]
Signed-off-by Ibrahim Hashimov <[email protected]>
Assisted-by AuditCode-AI:2026.07
---
 drivers/hid/hid-nintendo.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

This LGTM!

Reviewed-by Silvan Jegen <[email protected]>

Cheers,
Silvan

Quoted 19 lines

drivers/hid/hid-nintendo.c +6 -1

index e7302ec01ff1..11b5fe05acf4 100644
@@ -2607,7 +2607,12 @@ static int joycon_ctlr_read_handler(struct joycon_ctlr *ctlr, u8 *data,
 {
 	if (data[0] == JC_INPUT_SUBCMD_REPLY || data[0] == JC_INPUT_IMU_DATA ||
 	    data[0] == JC_INPUT_MCU_DATA) {
-		if (size >= 12) /* make sure it contains the input report */
+		/*
+		 * The whole struct is cast and parsed below, including the
+		 * IMU/subcmd union, not just the 12-byte partial header this
+		 * used to check for.
+		 */
+		if (size >= sizeof(struct joycon_input_report))
 			joycon_parse_report(ctlr,
 					    (struct joycon_input_report *)data);
 	}

Jiri Kosina

maintainer Applied

2026-08-03 17:53 UTC #3 lore raw

In reply to Ibrahim Hashimov

Quoted 28 lines · Ibrahim Hashimov

joycon_ctlr_read_handler() casts an incoming HID input report to struct joycon_input_report and parses it, guarding the cast only with a 12-byte length check:
	if (size >= 12) /* make sure it contains the input report */
		joycon_parse_report(ctlr, (struct joycon_input_report *)data);
struct joycon_input_report is 49 bytes: a 13-byte header followed by a union whose IMU arm is 36 bytes. For an IMU report joycon_parse_report() -> joycon_parse_imu_report() walks that union (struct offsets 13..48), so a report of exactly 12 bytes with data[0] == JC_INPUT_IMU_DATA passes the guard yet is read up to 37 bytes past its declared length. The over-read bytes are decoded into accelerometer/gyroscope values and forwarded to userspace through the "(IMU)" input device, leaking driver-internal memory. data[0] and size are fully controlled by a malicious or spoofed Joy-Con/Pro Controller.
Receive buffers are sized to the maximum report length, so this is an over-read within the allocation rather than a slab OOB, but the decoded bytes still reach userspace.
The sibling subcmd path in joycon_ctlr_handle_event() already bounds the same cast correctly:
	if (size < sizeof(struct joycon_input_report) ||
	    data[0] != JC_INPUT_SUBCMD_REPLY)
		break;

Quoted 6 lines

Use the same sizeof(struct joycon_input_report) bound here.
Fixes 2af16c1f846b ("HID: nintendo: add nintendo switch controller driver")
Cc [email protected]
Signed-off-by Ibrahim Hashimov <[email protected]>
Assisted-by AuditCode-AI:2026.07

Applied, thanks.

Signature · 2 lines

Jiri Kosina
SUSE Labs