[PATCH v2] Input: rmi4 - Use platform data instead of query, when available

[PATCH v2] Input: rmi4 - Use platform data instead of query, when available

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

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

Signed-off-by: David Heidelberg <david@ixit.cz>
---
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.
---
Changes in v2:
- Reworked to use device-tree values, when available, but still detect
  when both or one is missing. (Dmitry)
- Link to v1: https://patch.msgid.link/20260731-respect-x-y-mm-v1-0-3e85a4bec745@ixit.cz
---
 drivers/input/rmi4/rmi_f12.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
index 88c28089de993..841884d967a3d 100644
--- a/drivers/input/rmi4/rmi_f12.c
+++ b/drivers/input/rmi4/rmi_f12.c
@@ -150,47 +150,55 @@ static int rmi_f12_read_sensor_tuning(struct f12_data *f12)
 			"%s: Inactive Border xlo:%d xhi:%d ylo:%d yhi:%d\n",
 			__func__,
 			buf[offset], buf[offset + 1],
 			buf[offset + 2], buf[offset + 3]);
 
 		offset += 4;
 	}
 
+	/* When platform data are provided, we're done */
+	if (sensor->x_mm && sensor->y_mm)
+		return 0;
+
 	/*
 	 * Use the Query DPM feature when the resolution query register
 	 * exists.
 	 */
 	if (rmi_get_register_desc_item(&f12->query_reg_desc,
 				       RMI_F12_QUERY_RESOLUTION)) {
 		offset = rmi_register_desc_calc_reg_offset(&f12->query_reg_desc,
 							   RMI_F12_QUERY_RESOLUTION);
 		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;
+		if (!sensor->x_mm)
+			sensor->x_mm = sensor->max_x / dpm_resolution;
+		if (!sensor->y_mm)
+			sensor->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;
+		if (!sensor->x_mm)
+			sensor->x_mm = (pitch_x * rx_receivers) >> 12;
+		if (!sensor->y_mm)
+			sensor->y_mm = (pitch_y * tx_receivers) >> 12;
 	}
 
 	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: x_mm: %d y_mm: %d\n", __func__,
 		sensor->x_mm, sensor->y_mm);
 
 	return 0;
 }
 

---
base-commit: 95d6a9ccef99117115e41e9adb271243bd5e985b
change-id: 20260731-respect-x-y-mm-f74a2970e9b7

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

Re: [PATCH v2] Input: rmi4 - Use platform data instead of query, when available

[sashiko-bot]

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

Pre-existing issues:
- [High] Potential kernel panic due to division by zero if hardware returns 0 for DPM resolution.
- [High] Read of uninitialized kernel stack memory if a device provides a small `reg_size` but claims presence of multiple subpackets.
--

--- Patch [1]: [PATCH v2] Input: rmi4 - Use platform data instead of query, when available ---
commit abbd3f045bbd60bcd7253a3917acc57dc4a898a0
Author: David Heidelberg <david@ixit.cz>
Subject: Input: rmi4 - Use platform data instead of query, when available

This commit updates the RMI4 F12 driver to respect device-tree provided
touchscreen-x-mm and touchscreen-y-mm values. It prevents overriding
these values with sensor-provided data if they are already defined.

Link: https://patch.msgid.link/20260731-respect-x-y-mm-v1-0-3e85a4bec745@ixit.cz

> diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
> index 88c28089de993..841884d967a3d 100644
> --- a/drivers/input/rmi4/rmi_f12.c
> +++ b/drivers/input/rmi4/rmi_f12.c
> @@ -155,6 +155,10 @@ static int rmi_f12_read_sensor_tuning(struct f12_data *f12)

[Severity: High]
This is a pre-existing issue, but can this function leak uninitialized kernel
stack memory?

In rmi_f12_read_sensor_tuning(), a buffer is allocated on the stack without
initialization:

    u8 buf[15];

Later, rmi_read_block() populates item->reg_size bytes. If a malicious
peripheral provides a descriptor with a small reg_size but sets multiple bits
in subpacket_map, the function reads past the initialized portion of buf:

    sensor->max_x = get_unaligned_le16(&buf[offset]);

Is it possible to initialize buf to zero to prevent exposing stack contents
when the hardware-provided reg_size is less than the offset required by the
advertised subpackets?

>  		offset += 4;
>  	}
>  
> +	/* When platform data are provided, we're done */
> +	if (sensor->x_mm && sensor->y_mm)
> +		return 0;
> +

[ ... ]

>  		dpm_resolution = buf[0];
>  
> -		sensor->x_mm = sensor->max_x / dpm_resolution;
> -		sensor->y_mm = sensor->max_y / dpm_resolution;
> +		if (!sensor->x_mm)
> +			sensor->x_mm = sensor->max_x / dpm_resolution;

[Severity: High]
This is a pre-existing issue, but could this division cause a kernel panic?

If a misconfigured or malicious hardware peripheral returns 0 when the DPM
resolution query register is read, buf[0] and thus dpm_resolution will be 0.

Does this require a check to ensure dpm_resolution is non-zero before using
it as a divisor?

> +		if (!sensor->y_mm)
> +			sensor->y_mm = sensor->max_y / dpm_resolution;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806-respect-x-y-mm-v2-1-e0681ed3d63c@ixit.cz?part=1