[PATCH 0/2] Input: rmi4 - honor touchscreen-{x,y}-mm

[PATCH 0/2] Input: rmi4 - honor touchscreen-{x,y}-mm

[David Heidelberg via B4 Relay]

This series ensures, that when invalid value is detected or mismatch
between value defined in the device-tree and the read from the
touchscreen is observed, the user is notified and zero is ignored.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
David Heidelberg (2):
      Input: rmi4 - report difference between platform data and detected coord in F12
      Input: rmi4 - fallback to platform data for x/y-mm in F12

 drivers/input/rmi4/rmi_f12.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)
---
base-commit: 95d6a9ccef99117115e41e9adb271243bd5e985b
change-id: 20260731-respect-x-y-mm-f74a2970e9b7

Best regards,
--  
David Heidelberg <david@ixit.cz>

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

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

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.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/input/rmi4/rmi_f12.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

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
@@ -95,16 +95,17 @@ static int rmi_f12_read_sensor_tuning(struct f12_data *f12)
 {
 	const struct rmi_register_desc_item *item;
 	struct rmi_2d_sensor *sensor = &f12->sensor;
 	struct rmi_function *fn = sensor->fn;
 	struct rmi_device *rmi_dev = fn->rmi_dev;
 	int ret;
 	int offset;
 	u8 buf[15];
+	u8 x_mm, y_mm;
 	int pitch_x = 0;
 	int pitch_y = 0;
 	int rx_receivers = 0;
 	int tx_receivers = 0;
 	u16 query_dpm_addr = 0;
 	int dpm_resolution = 0;
 
 	item = rmi_get_register_desc_item(&f12->control_reg_desc, 8);
@@ -166,33 +167,46 @@ static int rmi_f12_read_sensor_tuning(struct f12_data *f12)
 		query_dpm_addr = fn->fd.query_base_addr	+ offset;
 		ret = rmi_read(fn->rmi_dev, query_dpm_addr, buf);
 		if (ret) {
 			dev_err(&fn->dev, "Failed to read DPM value: %d\n", ret);
 			return ret;
 		}
 		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;
 	} else {
 		if (rmi_register_desc_has_subpacket(item, 3)) {
 			rx_receivers = buf[offset];
 			tx_receivers = buf[offset + 1];
 			offset += 2;
 		}
 
 		/* Skip over sensor flags */
 		if (rmi_register_desc_has_subpacket(item, 4))
 			offset += 1;
 
-		sensor->x_mm = (pitch_x * rx_receivers) >> 12;
-		sensor->y_mm = (pitch_y * tx_receivers) >> 12;
+		x_mm = (pitch_x * rx_receivers) >> 12;
+		y_mm = (pitch_y * tx_receivers) >> 12;
 	}
 
+	if (sensor->x_mm && sensor->x_mm != x_mm)
+		dev_warn(&fn->dev,
+			 "platform data x_mm (%d) != detected x_mm: (%d)\n",
+			 sensor->x_mm, x_mm);
+
+	if (sensor->y_mm && sensor->y_mm != y_mm)
+		dev_warn(&fn->dev,
+			 "platform data y_mm (%d) != detected y_mm: (%d)\n",
+			 sensor->y_mm, y_mm);
+
+	sensor->x_mm = x_mm;
+	sensor->y_mm = y_mm;
+
 	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: x_mm: %d y_mm: %d\n", __func__,
 		sensor->x_mm, sensor->y_mm);
 
 	return 0;
 }
 
 static void rmi_f12_process_objects(struct f12_data *f12, u8 *data1, u32 size)
 {

-- 
2.53.0

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

[sashiko-bot]

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 <david@ixit.cz>

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/20260731-respect-x-y-mm-v1-0-3e85a4bec745@ixit.cz?part=1

[PATCH 2/2] Input: rmi4 - fallback to platform data for x/y-mm in F12

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

When detection for touchscreen-x-mm / y-mm returns 0, fallback to the
platform data, so we get meaningful values.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/input/rmi4/rmi_f12.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
index 88797556e738c..6f2e98c3a3499 100644
--- a/drivers/input/rmi4/rmi_f12.c
+++ b/drivers/input/rmi4/rmi_f12.c
@@ -194,18 +194,29 @@ static int rmi_f12_read_sensor_tuning(struct f12_data *f12)
 			 "platform data x_mm (%d) != detected x_mm: (%d)\n",
 			 sensor->x_mm, x_mm);
 
 	if (sensor->y_mm && sensor->y_mm != y_mm)
 		dev_warn(&fn->dev,
 			 "platform data y_mm (%d) != detected y_mm: (%d)\n",
 			 sensor->y_mm, y_mm);
 
-	sensor->x_mm = x_mm;
-	sensor->y_mm = y_mm;
+	if (x_mm)
+		sensor->x_mm = x_mm;
+	else
+		dev_warn(&fn->dev,
+			 "x_mm detection returned 0, keeping %d\n",
+			 sensor->x_mm);
+
+	if (y_mm)
+		sensor->y_mm = y_mm;
+	else
+		dev_warn(&fn->dev,
+			 "y_mm detection returned 0, keeping %d\n",
+			 sensor->y_mm);
 
 	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: x_mm: %d y_mm: %d\n", __func__,
 		sensor->x_mm, sensor->y_mm);
 
 	return 0;
 }
 
 static void rmi_f12_process_objects(struct f12_data *f12, u8 *data1, u32 size)

-- 
2.53.0

Re: [PATCH 2/2] Input: rmi4 - fallback to platform data for x/y-mm in F12

[sashiko-bot]

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

Pre-existing issues:
- [High] Potential kernel panic due to division by zero if the device firmware returns 0 for DPM resolution.
- [High] Potential read of uninitialized stack memory when parsing device subpackets.
- [High] Truncation of physical dimensions `x_mm` and `y_mm` for devices larger than 255 mm.
--

--- Patch [2]: [PATCH 2/2] Input: rmi4 - fallback to platform data for x/y-mm in F12 ---
commit 825204fcd696c68bb8002a2ae57765aed06fc9e4
Author: David Heidelberg <david@ixit.cz>

Input: rmi4 - fallback to platform data for x/y-mm in F12

When detection for touchscreen-x-mm / y-mm returns 0, fallback to the
platform data, so we get meaningful values.

> diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
> index 88797556e738c..6f2e98c3a3499 100644
> --- a/drivers/input/rmi4/rmi_f12.c
> +++ b/drivers/input/rmi4/rmi_f12.c
> @@ -199,8 +199,19 @@ static int rmi_f12_read_sensor_tuning(struct f12_data *f12)

[Severity: High]
This isn't a bug introduced by this patch, but could this lead to a kernel
panic from division by zero?

Earlier in rmi_f12_read_sensor_tuning(), dpm_resolution is read directly
from the hardware into buf:

        ret = rmi_read(fn->rmi_dev, query_dpm_addr, buf);
        if (ret) {
                ...
        }
        dpm_resolution = buf[0];

        x_mm = sensor->max_x / dpm_resolution;
        y_mm = sensor->max_y / dpm_resolution;

If a buggy or malicious device returns 0 for the DPM resolution, this will
unconditionally crash the kernel during the probe path. Should there be a 
check for dpm_resolution == 0 before performing this division?


[Severity: High]
This is a pre-existing issue, but does this code read uninitialized stack 
memory when parsing device subpackets?

In rmi_f12_read_sensor_tuning(), a 15-byte stack array is declared without
initialization:

        u8 buf[15];

The driver reads item->reg_size bytes into this buffer:

        ret = rmi_read_block(rmi_dev, fn->fd.control_base_addr + offset,
                             buf, item->reg_size);

The size is controlled by the device descriptor. The driver then uses offsets
to access this buffer based purely on rmi_register_desc_has_subpacket(),
without ensuring item->reg_size actually covers the accessed offsets.

If a device supplies a small reg_size but claims all subpackets are present,
could this read past the initialized portion of buf and leak kernel stack
memory or cause erratic behavior?

>  			 "platform data y_mm (%d) != detected y_mm: (%d)\n",
>  			 sensor->y_mm, y_mm);
>  
> -	sensor->x_mm = x_mm;
> -	sensor->y_mm = y_mm;
> +	if (x_mm)
> +		sensor->x_mm = x_mm;

[Severity: High]
This is a pre-existing issue, but could the physical dimensions overflow on 
larger devices, which this patch might now incorrectly interpret as a failure?

The dimensions are calculated in millimeters and stored in u8 variables:

drivers/input/rmi4/rmi_f12.c:rmi_f12_read_sensor_tuning()
        u8 x_mm, y_mm;

And also stored as u8 in struct rmi_2d_sensor:

drivers/input/rmi4/rmi_2d_sensor.h:
        u8 x_mm;
        u8 y_mm;

For any touchscreen larger than 255 mm (approx. 10 inches), the value will
overflow and truncate. If a dimension is exactly 256 mm, it truncates to 0.

With this patch, a truncated 0 will trigger the x_mm == 0 fallback path and
incorrectly retain platform data instead of the detected value. Should these 
dimensions be stored using u16?

> +	else
> +		dev_warn(&fn->dev,
> +			 "x_mm detection returned 0, keeping %d\n",
> +			 sensor->x_mm);
> +
> +	if (y_mm)
> +		sensor->y_mm = y_mm;
> +	else
> +		dev_warn(&fn->dev,
> +			 "y_mm detection returned 0, keeping %d\n",
> +			 sensor->y_mm);
>  
>  	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: x_mm: %d y_mm: %d\n", __func__,
>  		sensor->x_mm, sensor->y_mm);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-respect-x-y-mm-v1-0-3e85a4bec745@ixit.cz?part=2