Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 0/2] Input: rmi4 - honor touchscreen-{x,y}-mm
@ 2026-07-31 18:13 David Heidelberg via B4 Relay
  2026-07-31 18:13 ` [PATCH 1/2] Input: rmi4 - report difference between platform data and detected coord in F12 David Heidelberg via B4 Relay
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-07-31 18:13 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, phone-devel, David Heidelberg

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>



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] Input: rmi4 - report difference between platform data and detected coord in F12
  2026-07-31 18:13 [PATCH 0/2] Input: rmi4 - honor touchscreen-{x,y}-mm David Heidelberg via B4 Relay
@ 2026-07-31 18:13 ` David Heidelberg via B4 Relay
  2026-07-31 18:25   ` sashiko-bot
  2026-07-31 18:13 ` [PATCH 2/2] Input: rmi4 - fallback to platform data for x/y-mm " David Heidelberg via B4 Relay
  2026-08-05  5:21 ` [PATCH 0/2] Input: rmi4 - honor touchscreen-{x,y}-mm Dmitry Torokhov
  2 siblings, 1 reply; 7+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-07-31 18:13 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, phone-devel, David Heidelberg

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



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] Input: rmi4 - fallback to platform data for x/y-mm in F12
  2026-07-31 18:13 [PATCH 0/2] Input: rmi4 - honor touchscreen-{x,y}-mm David Heidelberg via B4 Relay
  2026-07-31 18:13 ` [PATCH 1/2] Input: rmi4 - report difference between platform data and detected coord in F12 David Heidelberg via B4 Relay
@ 2026-07-31 18:13 ` David Heidelberg via B4 Relay
  2026-07-31 18:23   ` sashiko-bot
  2026-08-05  5:21 ` [PATCH 0/2] Input: rmi4 - honor touchscreen-{x,y}-mm Dmitry Torokhov
  2 siblings, 1 reply; 7+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-07-31 18:13 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, phone-devel, David Heidelberg

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



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] Input: rmi4 - fallback to platform data for x/y-mm in F12
  2026-07-31 18:13 ` [PATCH 2/2] Input: rmi4 - fallback to platform data for x/y-mm " David Heidelberg via B4 Relay
@ 2026-07-31 18:23   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-07-31 18:23 UTC (permalink / raw)
  To: David Heidelberg; +Cc: dmitry.torokhov, linux-input

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] Input: rmi4 - report difference between platform data and detected coord in F12
  2026-07-31 18:13 ` [PATCH 1/2] Input: rmi4 - report difference between platform data and detected coord in F12 David Heidelberg via B4 Relay
@ 2026-07-31 18:25   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-07-31 18:25 UTC (permalink / raw)
  To: David Heidelberg; +Cc: dmitry.torokhov, linux-input

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] Input: rmi4 - honor touchscreen-{x,y}-mm
  2026-07-31 18:13 [PATCH 0/2] Input: rmi4 - honor touchscreen-{x,y}-mm David Heidelberg via B4 Relay
  2026-07-31 18:13 ` [PATCH 1/2] Input: rmi4 - report difference between platform data and detected coord in F12 David Heidelberg via B4 Relay
  2026-07-31 18:13 ` [PATCH 2/2] Input: rmi4 - fallback to platform data for x/y-mm " David Heidelberg via B4 Relay
@ 2026-08-05  5:21 ` Dmitry Torokhov
  2026-08-06 17:06   ` David Heidelberg
  2 siblings, 1 reply; 7+ messages in thread
From: Dmitry Torokhov @ 2026-08-05  5:21 UTC (permalink / raw)
  To: david; +Cc: linux-input, linux-kernel, phone-devel

Hi David,

On Fri, Jul 31, 2026 at 08:13:40PM +0200, David Heidelberg via B4 Relay wrote:
> 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.

This seems very complicated. If board maker specifies these parameters,
they probably want to use them instead of what the firmware reports. So
wouldn't it be simpler to not try to match and report differences but
rather skip querying hardware and use device properties if they are
specified?

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] Input: rmi4 - honor touchscreen-{x,y}-mm
  2026-08-05  5:21 ` [PATCH 0/2] Input: rmi4 - honor touchscreen-{x,y}-mm Dmitry Torokhov
@ 2026-08-06 17:06   ` David Heidelberg
  0 siblings, 0 replies; 7+ messages in thread
From: David Heidelberg @ 2026-08-06 17:06 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, phone-devel

On 05/08/2026 07:21, Dmitry Torokhov wrote:
> Hi David,
> 
> On Fri, Jul 31, 2026 at 08:13:40PM +0200, David Heidelberg via B4 Relay wrote:
>> 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.
> 
> This seems very complicated. If board maker specifies these parameters,
> they probably want to use them instead of what the firmware reports. So
> wouldn't it be simpler to not try to match and report differences but
> rather skip querying hardware and use device properties if they are
> specified?

Hi Dmitry.

Agree, I'll send next version just overriding the HW reported values.

David

> 
> Thanks.
> 

-- 
David Heidelberg


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-08-06 17:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-31 18:13 [PATCH 0/2] Input: rmi4 - honor touchscreen-{x,y}-mm David Heidelberg via B4 Relay
2026-07-31 18:13 ` [PATCH 1/2] Input: rmi4 - report difference between platform data and detected coord in F12 David Heidelberg via B4 Relay
2026-07-31 18:25   ` sashiko-bot
2026-07-31 18:13 ` [PATCH 2/2] Input: rmi4 - fallback to platform data for x/y-mm " David Heidelberg via B4 Relay
2026-07-31 18:23   ` sashiko-bot
2026-08-05  5:21 ` [PATCH 0/2] Input: rmi4 - honor touchscreen-{x,y}-mm Dmitry Torokhov
2026-08-06 17:06   ` David Heidelberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox