Re: [PATCH 1/2] Input: rmi4 - report difference between platform data and detected coord in F12

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

Pre-existing issues:
- [High] Potential division by zero if the device hardware reports a DPM resolution of 0.
- [High] Uninitialized kernel stack memory leak due to insufficient validation of register size against subpacket map.
--

--- Patch [1]: [PATCH 1/2] Input: rmi4 - report difference between platform data and detected coord in F12 ---
commit b9eac21396bb125557501cbe3a56f93e5854436c
Author: David Heidelberg <[email protected]>

Input: rmi4 - report difference between platform data and detected coord in F12

Platform data may define touchscreen-x-mm and touchscreen-y-mm, but
these are quietly overridden by data provided by sensor.

Warn when detected data doesn't match the device-tree data.

> diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
> index 88c28089de993..88797556e738c 100644
> --- a/drivers/input/rmi4/rmi_f12.c
> +++ b/drivers/input/rmi4/rmi_f12.c
> @@ -100,6 +100,7 @@ static int rmi_f12_read_sensor_tuning(struct f12_data *f12)
>  	int ret;
>  	int offset;
>  	u8 buf[15];
> +	u8 x_mm, y_mm;

[Severity: High]
This isn't a bug introduced by this patch, but could this code leak
uninitialized stack memory if a peripheral advertises a small register size?

In rmi_f12_read_sensor_tuning(), buf is populated via rmi_read_block() with
exactly item->reg_size bytes. Later in the function, it unconditionally
parses fields if the corresponding subpacket bit is set:

rmi_f12_read_sensor_tuning()
    ...
    offset = 0;
    if (rmi_register_desc_has_subpacket(item, 0)) {
        sensor->max_x = get_unaligned_le16(&buf[offset]);
        sensor->max_y = get_unaligned_le16(&buf[offset + 2]);
    ...

If a peripheral sends a crafted register descriptor with a small reg_size
(e.g., 2 bytes) but sets bits in the subpacket map, does this cause the
driver to read past the initialized portion of buf and expose stack memory
to userspace?

>  	int pitch_x = 0;
>  	int pitch_y = 0;
>  	int rx_receivers = 0;

[ ... ]

> @@ -171,8 +172,8 @@ static int rmi_f12_read_sensor_tuning(struct f12_data *f12)
>  		}
>  		dpm_resolution = buf[0];
>  
> -		sensor->x_mm = sensor->max_x / dpm_resolution;
> -		sensor->y_mm = sensor->max_y / dpm_resolution;
> +		x_mm = sensor->max_x / dpm_resolution;
> +		y_mm = sensor->max_y / dpm_resolution;

[Severity: High]
This is a pre-existing issue, but is it possible for a malicious or
malfunctioning device to cause a division by zero panic here?

The dpm_resolution value is read directly from the hardware query register
into buf[0] and then used unconditionally as the denominator:

    dpm_resolution = buf[0];
    x_mm = sensor->max_x / dpm_resolution;

If the unprivileged peripheral returns a value of 0, could this crash the
entire system?

>  	} else {
>  		if (rmi_register_desc_has_subpacket(item, 3)) {
>  			rx_receivers = buf[offset];

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