Re: [PATCH v2 2/5] Input: zinitix - do not ignore non-moving fingers

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Device properties are read and validated after the input device has been registered, creating a race condition with userspace.
- [High] The driver allocates I2C transfer buffers on the stack, which violates DMA mapping rules and can cause memory corruption or panics.
--

--- Patch [2]: [PATCH v2 2/5] Input: zinitix - do not ignore non-moving fingers ---
commit 79b05b71b81ce17697cba617a5a5c777d1fe07a7
Author: Kaustabh Chakraborty <kauschluss@disroot.org>
Subject: Input: zinitix - do not ignore non-moving fingers

This commit fixes an issue where multitouch does not work reliably on the
ZT7548 touchscreen by no longer ignoring fingers that lack movement flags.
It ensures contacts with the SUB_BIT_EXIST flag are correctly processed in
zinitix_report_finger().

> diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
> index 3421b8ffb19b2..a0a89d25e25bb 100644
> --- a/drivers/input/touchscreen/zinitix.c
> +++ b/drivers/input/touchscreen/zinitix.c
[ ... ]
> @@ -473,7 +473,7 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
>  		const struct point_coord *p = &touch_event.point_coord[i];
>  
>  		/* Only process contacts that are actually reported */
> -		if (p->sub_status & SUB_BIT_EXIST)
> +		if (p->sub_status)
>  			zinitix_report_finger(bt541, i, p);
>  	}
>  

[Severity: High]
This isn't a bug introduced by this patch, but does the interrupt handler
allocate I2C transfer buffers on the stack, potentially violating DMA
mapping constraints?

In zinitix_ts_irq_handler(), touch_event is declared on the stack and passed
directly down to the I2C subsystem:

drivers/input/touchscreen/zinitix.c:zinitix_ts_irq_handler() {
	struct touch_event touch_event;
	...
	error = zinitix_read_data(bt541->client, ZINITIX_POINT_STATUS_REG,
				  &touch_event, sizeof(struct touch_event));

The helpers zinitix_read_data() and zinitix_write_u16() also allocate buffers
on the stack before passing them to i2c_master_send():

drivers/input/touchscreen/zinitix.c:zinitix_read_data() {
	__le16 reg_le = cpu_to_le16(reg);
	...
	ret = i2c_master_send(client, (u8 *)&reg_le, sizeof(reg_le));

Could this cause memory corruption or silent DMA failures if the underlying
I2C adapter attempts to DMA-map these stack pointers?


[Severity: High]
This isn't a bug introduced by this patch, but is there a race condition
with userspace during the driver's probe?

The driver initializes and registers the input device, exposing it to
userspace, before finishing its property validation:

drivers/input/touchscreen/zinitix.c:zinitix_ts_probe() {
	...
	error = zinitix_init_input_dev(bt541);
	// device is now registered and visible to userspace
	...
	error = device_property_read_u32(&client->dev, "zinitix,mode",
					 &bt541->zinitix_mode);
	...
	if (bt541->zinitix_mode != 2) {
		dev_err(&client->dev,
			"Malformed zinitix,mode property, must be 2 ...\n",
			bt541->zinitix_mode);
		return -EINVAL;
	}

If userspace opens the device right after registration, zinitix_start()
could run with an uninitialized bt541->zinitix_mode, writing garbage to the
hardware.

Furthermore, if the probe fails due to a malformed mode property, the
already-exposed device will be abruptly destroyed while potentially in use.

Could the device properties be read and validated before calling
input_register_device() to ensure safe initialization?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-zinitix-modes-v2-0-ffe2ec9a9031@disroot.org?part=2