Re: [PATCH 3/4] HID: picolcd: Use min() macro to simplify code

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

Pre-existing issues:
- [High] Out-of-bounds read in `picolcd_debug_eeprom_read` leaking kernel heap memory to userspace.
--

--- Patch [3]: [PATCH 3/4] HID: picolcd: Use min() macro to simplify code ---
commit 1474980d788a9366218f25c39b115f1aca750b68
Author: Chen Changcheng <[email protected]>

HID: picolcd: Use min() macro to simplify code

This commit replaces an open-coded minimum check with the standard min()
macro in hid-picolcd_debugfs.c. It simplifies the logic for limiting the
read size for EEPROM data.

> diff --git a/drivers/hid/hid-picolcd_debugfs.c b/drivers/hid/hid-picolcd_debugfs.c
> --- a/drivers/hid/hid-picolcd_debugfs.c
> +++ b/drivers/hid/hid-picolcd_debugfs.c
> @@ -86,7 +86,7 @@ static ssize_t picolcd_debug_eeprom_read(struct file *f, char __user *u,
>  	/* prepare buffer with info about what we want to read (addr & len) */
>  	raw_data[0] = *off & 0xff;
>  	raw_data[1] = (*off >> 8) & 0xff;
> -	raw_data[2] = s < 20 ? s : 20;
> +	raw_data[2] = min(s, 20);

[ ... ]

> 	if (resp->in_report && resp->in_report->id == REPORT_EE_DATA) {
> 		/* successful read :) */
> 		ret = resp->raw_data[2];
> 		if (ret > s)
> 			ret = s;
> 		if (copy_to_user(u, resp->raw_data+3, ret))

[Severity: High]
This is a pre-existing issue, but while reviewing this function, can a
malicious or buggy USB device cause an out-of-bounds read here?

If a user reads from the eeprom file with a large size, for example 
s = 256, and a malicious device responds with a REPORT_EE_DATA containing
a large embedded length field in resp->raw_data[2] up to 255, ret will take
that device-controlled value.

Since ret (255) would not be greater than s (256), this copy_to_user()
call could attempt to copy up to 255 bytes. 

Given that resp->raw_data is a fixed 64-byte array, does this allow reading
past the end of the struct picolcd_pending heap allocation and leaking
adjacent kernel heap memory to userspace?

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