[PATCH RESEND] Input: rmi4 - Move sensor initialization into own function for F12

[PATCH RESEND] Input: rmi4 - Move sensor initialization into own function for F12

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

Makes rmi_f12_probe function a bit cleaner and more structured.

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

diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
index 88c28089de993..333477bf0663e 100644
--- a/drivers/input/rmi4/rmi_f12.c
+++ b/drivers/input/rmi4/rmi_f12.c
@@ -364,29 +364,60 @@ static int rmi_f12_config(struct rmi_function *fn)
 	ret = rmi_f12_write_control_regs(fn);
 	if (ret)
 		dev_warn(&fn->dev,
 			 "Failed to write F12 control registers: %d\n", ret);
 
 	return 0;
 }
 
+static int rmi_f12_sensor_init(struct rmi_function *fn, struct f12_data *f12)
+{
+	struct rmi_2d_sensor *sensor = &f12->sensor;
+	size_t pkt_size;
+
+	sensor->fn = fn;
+	f12->data_addr = fn->fd.data_base_addr;
+	pkt_size = rmi_register_desc_calc_size(&f12->data_reg_desc);
+	if (pkt_size > SZ_1M) {
+		dev_err(&fn->dev, "Invalid data packet size: %zu\n", pkt_size);
+		return -EINVAL;
+	}
+	sensor->pkt_size = pkt_size;
+
+	sensor->axis_align = f12->sensor_pdata.axis_align;
+
+	sensor->x_mm = f12->sensor_pdata.x_mm;
+	sensor->y_mm = f12->sensor_pdata.y_mm;
+	sensor->dribble = f12->sensor_pdata.dribble;
+
+	if (sensor->sensor_type == rmi_sensor_default)
+		sensor->sensor_type = f12->sensor_pdata.sensor_type;
+
+	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: data packet size: %u\n", __func__,
+		sensor->pkt_size);
+	sensor->data_pkt = devm_kmalloc(&fn->dev, sensor->pkt_size, GFP_KERNEL);
+	if (!sensor->data_pkt)
+		return -ENOMEM;
+
+	return 0;
+}
+
 static int rmi_f12_probe(struct rmi_function *fn)
 {
 	struct f12_data *f12;
 	int ret;
 	struct rmi_device *rmi_dev = fn->rmi_dev;
 	char buf;
 	u16 query_addr = fn->fd.query_base_addr;
 	const struct rmi_register_desc_item *item;
 	struct rmi_2d_sensor *sensor;
 	struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
 	struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
 	size_t data_offset = 0;
-	size_t pkt_size;
 	int irq_mask_size;
 	int i;
 
 	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s\n", __func__);
 
 	irq_mask_size = BITS_TO_LONGS(drvdata->irq_count);
 
 	ret = rmi_read(fn->rmi_dev, query_addr, &buf);
@@ -421,39 +452,20 @@ static int rmi_f12_probe(struct rmi_function *fn)
 		f12->sensor_pdata = pdata->sensor_pdata;
 	}
 
 	ret = rmi_f12_read_register_descs(fn, f12, query_addr);
 	if (ret)
 		return ret;
 
 	sensor = &f12->sensor;
-	sensor->fn = fn;
-	f12->data_addr = fn->fd.data_base_addr;
-	pkt_size = rmi_register_desc_calc_size(&f12->data_reg_desc);
-	if (pkt_size > SZ_1M) {
-		dev_err(&fn->dev, "Invalid data packet size: %zu\n", pkt_size);
-		return -EINVAL;
-	}
-	sensor->pkt_size = pkt_size;
-
-	sensor->axis_align = f12->sensor_pdata.axis_align;
 
-	sensor->x_mm = f12->sensor_pdata.x_mm;
-	sensor->y_mm = f12->sensor_pdata.y_mm;
-	sensor->dribble = f12->sensor_pdata.dribble;
-
-	if (sensor->sensor_type == rmi_sensor_default)
-		sensor->sensor_type = f12->sensor_pdata.sensor_type;
-
-	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: data packet size: %u\n", __func__,
-		sensor->pkt_size);
-	sensor->data_pkt = devm_kmalloc(&fn->dev, sensor->pkt_size, GFP_KERNEL);
-	if (!sensor->data_pkt)
-		return -ENOMEM;
+	ret = rmi_f12_sensor_init(fn, f12);
+	if (ret)
+		return ret;
 
 	dev_set_drvdata(&fn->dev, f12);
 
 	ret = rmi_f12_read_sensor_tuning(f12);
 	if (ret)
 		return ret;
 
 	/*

---
base-commit: 3d5670d672ae08b8c534b7beed6f57c8b44e7b43
change-id: 20260627-synaptics-rmi4-fnsplit-3698a620b73c

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

Re: [PATCH RESEND] Input: rmi4 - Move sensor initialization into own function for F12

[sashiko-bot]

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

Pre-existing issues:
- [High] Heap Out-of-Bounds Read in F12 attention handler
- [High] Division by Zero kernel panic triggered by malicious device
- [High] Uninitialized Kernel Stack Memory Leak
--

--- Patch [1]: [PATCH RESEND] Input: rmi4 - Move sensor initialization into own function for F12 ---
commit ca79b6f1c1761554daeb36e4304a62bdc7fa5f10
Author: David Heidelberg <david@ixit.cz>

Input: rmi4 - Move sensor initialization into own function for F12

This commit refactors the F12 probe logic by extracting the sensor
initialization sequence into a separate helper function,
rmi_f12_sensor_init(). This reduces the size of rmi_f12_probe() and makes
the code more structured.

> diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
> index 88c28089de993..333477bf0663e 100644
> --- a/drivers/input/rmi4/rmi_f12.c
> +++ b/drivers/input/rmi4/rmi_f12.c
[ ... ]
> @@ -426,29 +457,10 @@ static int rmi_f12_probe(struct rmi_function *fn)
>  		return ret;
>  
>  	sensor = &f12->sensor;
> -	sensor->fn = fn;
> -	f12->data_addr = fn->fd.data_base_addr;
> -	pkt_size = rmi_register_desc_calc_size(&f12->data_reg_desc);
> -	if (pkt_size > SZ_1M) {
> -		dev_err(&fn->dev, "Invalid data packet size: %zu\n", pkt_size);
> -		return -EINVAL;
> -	}
> -	sensor->pkt_size = pkt_size;
> -
> -	sensor->axis_align = f12->sensor_pdata.axis_align;
>  
> -	sensor->x_mm = f12->sensor_pdata.x_mm;
> -	sensor->y_mm = f12->sensor_pdata.y_mm;
> -	sensor->dribble = f12->sensor_pdata.dribble;
> -
> -	if (sensor->sensor_type == rmi_sensor_default)
> -		sensor->sensor_type = f12->sensor_pdata.sensor_type;
> -
> -	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: data packet size: %u\n", __func__,
> -		sensor->pkt_size);
> -	sensor->data_pkt = devm_kmalloc(&fn->dev, sensor->pkt_size, GFP_KERNEL);
> -	if (!sensor->data_pkt)
> -		return -ENOMEM;
> +	ret = rmi_f12_sensor_init(fn, f12);
> +	if (ret)
> +		return ret;
>  
>  	dev_set_drvdata(&fn->dev, f12);
>  

[Severity: High]
This is a pre-existing issue, but I noticed a potential out-of-bounds heap
read when f12 processes objects.

In rmi_f12_attention(), we advance the pointer by data1_offset but pass the
full valid_bytes without adjusting it:

drivers/input/rmi4/rmi_f12.c:rmi_f12_attention() {
    ...
    if (f12->data1)
        rmi_f12_process_objects(f12,
                                &sensor->data_pkt[f12->data1_offset],
                                valid_bytes);
    ...
}

Then in rmi_f12_process_objects(), we read object coordinates:

drivers/input/rmi4/rmi_f12.c:rmi_f12_process_objects() {
    ...
    obj->x = get_unaligned_le16(&data1[1]);
    ...
}

Could a malicious device over I2C/SPI specify a non-zero Data0 register size
(offsetting Data1) and a large number of fingers, causing the loop to read
data1_offset bytes past the end of the sensor->data_pkt heap allocation?

[Severity: High]
This isn't a bug introduced by this patch, but could a malicious hardware
device trigger a kernel panic via division by zero?

In rmi_f12_read_sensor_tuning(), if the device returns 0 for the DPM
resolution:

drivers/input/rmi4/rmi_f12.c:rmi_f12_read_sensor_tuning() {
    ...
    sensor->x_mm = sensor->max_x / dpm_resolution;
    sensor->y_mm = sensor->max_y / dpm_resolution;
    ...
}

Does the driver need to check if dpm_resolution is zero before performing this
division?

[Severity: High]
This is a pre-existing issue, but could this code leak uninitialized kernel
stack memory to userspace?

In rmi_f12_read_sensor_tuning(), we read item->reg_size bytes into the buf
stack array:

drivers/input/rmi4/rmi_f12.c:rmi_f12_read_sensor_tuning() {
    ...
    ret = rmi_read_block(rmi_dev, fn->fd.control_base_addr + offset,
                         buf, item->reg_size);
    ...
    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]);
        offset += 4;
    }
    ...
}

If a malicious device specifies a small reg_size (e.g., 1 byte) but indicates
via the subpacket_map that subpacket 0 exists, could the unconditional reads
for max_x and max_y read past the initialized portion of buf?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-synaptics-rmi4-fnsplit-v1-1-8175cec2ded1@ixit.cz?part=1