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

[sashiko-bot@kernel.org]

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