[PATCH] HID: picolcd: clamp eeprom debugfs read to bytes actually received

[PATCH] HID: picolcd: clamp eeprom debugfs read to bytes actually received

[Ibrahim Hashimov]

picolcd_debug_eeprom_read() trusts resp->raw_data[2] -- a length byte
supplied by the device in its REPORT_EE_DATA reply -- clamped only to
the caller's read() count:

	ret = resp->raw_data[2];
	if (ret > s)
		ret = s;
	if (copy_to_user(u, resp->raw_data+3, ret))

It never checks resp->raw_size, the number of bytes picolcd_raw_event()
actually copied into the 64-byte raw_data[] of the kmalloc'd struct
picolcd_pending. A device (or a spoofed picoLCD) returning a length byte
of 0xff, read with a count >= 255, makes copy_to_user() read past
raw_data[] into adjacent slab memory and return it to userspace through
the debugfs "eeprom" file:

	BUG: KASAN: slab-out-of-bounds in _copy_to_user
	Read of size 255 ... picolcd_debug_eeprom_read+0x214/0x2f0 [hid_picolcd]

The debug-dump path in the same file already validates the device length
byte against the received size before trusting it; this read does not.
The file is created S_IRUSR (root-only) and a crafted device is needed,
so it is neither unprivileged- nor remotely-triggerable.

Clamp the copy length to resp->raw_size - 3 (the payload actually
received, minus the 3-byte header), floored at 0 for short replies.

Fixes: 9bbf2b98ba11 ("HID: add experimental access to PicoLCD device's EEPROM and FLASH")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
 drivers/hid/hid-picolcd_debugfs.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/hid/hid-picolcd_debugfs.c b/drivers/hid/hid-picolcd_debugfs.c
index 085847a92e07..1f7dfe60e9ba 100644
--- a/drivers/hid/hid-picolcd_debugfs.c
+++ b/drivers/hid/hid-picolcd_debugfs.c
@@ -99,6 +99,15 @@ static ssize_t picolcd_debug_eeprom_read(struct file *f, char __user *u,
 		ret = resp->raw_data[2];
 		if (ret > s)
 			ret = s;
+		/*
+		 * raw_data[2] is a device-supplied length; also clamp it to
+		 * what picolcd_raw_event() actually stored (raw_size), or a
+		 * hostile device overruns the raw_data[] buffer.
+		 */
+		if (ret > resp->raw_size - 3)
+			ret = resp->raw_size - 3;
+		if (ret < 0)
+			ret = 0;
 		if (copy_to_user(u, resp->raw_data+3, ret))
 			ret = -EFAULT;
 		else
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] HID: picolcd: clamp eeprom debugfs read to bytes actually received

[sashiko-bot]

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

Pre-existing issues:
- [High] Use-After-Free on `data->rc_dev` and `data->input_keys` due to race in probe error path.
--

--- Patch [1]: [PATCH] HID: picolcd: clamp eeprom debugfs read to bytes actually received ---
commit 16232239549016339287f47b37b440446a47d534
Author: Ibrahim Hashimov <security@auditcode.ai>

HID: picolcd: clamp eeprom debugfs read to bytes actually received

This commit fixes an out-of-bounds slab read in picolcd_debug_eeprom_read()
by correctly clamping the copy length to the received payload size.

Cc: stable@vger.kernel.org

> diff --git a/drivers/hid/hid-picolcd_debugfs.c b/drivers/hid/hid-picolcd_debugfs.c
> index 085847a92e07d..1f7dfe60e9ba2 100644
> --- a/drivers/hid/hid-picolcd_debugfs.c
> +++ b/drivers/hid/hid-picolcd_debugfs.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed a potential use-after-free in the
probe error path.

If a USB device continuously sends interrupt IN packets to trigger
picolcd_raw_event() while intentionally failing device initialization, could
this cause a race condition?

In drivers/hid/hid-picolcd_core.c:picolcd_probe_lcd(), the probe error
handler destroys the cir and keys objects while the HID device is still open:

err:
	picolcd_exit_leds(data);
	picolcd_exit_framebuffer(data);
	picolcd_exit_backlight(data);
	picolcd_exit_lcd(data);
	picolcd_exit_cir(data);
	picolcd_exit_keys(data);
	return error;

Because hid_hw_close() isn't called until later, picolcd_raw_event() can
concurrently dereference the freed data->rc_dev pointer outside of the
spinlock in drivers/hid/hid-picolcd_cir.c:picolcd_raw_cir():

	spin_lock_irqsave(&data->lock, flags);
	if (!data->rc_dev || (data->status & PICOLCD_CIR_SHUN)) {
		spin_unlock_irqrestore(&data->lock, flags);
		return 1;
	}
	spin_unlock_irqrestore(&data->lock, flags);

	// ... [IR data processing] ...

	ir_raw_event_store(data->rc_dev, &rawir);

The same timing issue appears to apply to data->input_keys, which is accessed
locklessly in drivers/hid/hid-picolcd_core.c:picolcd_raw_keypad():

		input_event(data->input_keys, EV_MSC, MSC_SCAN, raw_data[i]);
		if (raw_data[i] < PICOLCD_KEYS)
			key_code = data->keycode[raw_data[i]];

Could this allow concurrent events to use these pointers after they are freed
by picolcd_exit_cir() and picolcd_exit_keys()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715115301.91063-1-security@auditcode.ai?part=1