Re: [PATCH 3/5] Input: zinitix - add support for modes 0 and 1

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The input device is registered and exposed to userspace before its driver private data (`zinitix_mode`) is fully parsed and initialized, creating a race condition.
- [High] Stack-allocated buffers are passed to I2C transfer functions, which is unsafe and can lead to memory corruption or kernel panics if the I2C adapter relies on DMA.
--

--- Patch [3]: [PATCH 3/5] Input: zinitix - add support for modes 0 and 1 ---
commit 0a0b84e68f36f7bf2e3b751801c1b10ca100ae2e
Author: Kaustabh Chakraborty <kauschluss@disroot.org>

Input: zinitix - add support for modes 0 and 1

This commit implements support for touchscreen modes 0 and 1 in the zinitix
driver by introducing mode-specific event structures. It maintains a common
canonical event structure and adds functions to convert mode-specific event
data into this common format.

> diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
> index fdcb80f52c917..42010f23445e3 100644
> --- a/drivers/input/touchscreen/zinitix.c
> +++ b/drivers/input/touchscreen/zinitix.c

[ ... ]

> @@ -440,6 +470,103 @@ static void zinitix_report_keys(struct bt541_ts_data *bt541, u16 icon_events)
>  				 bt541->keycodes[i], icon_events & BIT(i));
>  }
>  
> +static int zinitix_read_point_status_mode0(struct bt541_ts_data *bt541,
> +					   struct touch_event *event)
> +{
> +	struct touch_event_mode0 event_mode0;
> +	int ret;
> +	int i;
> +
> +	ret = zinitix_read_data(bt541->client, ZINITIX_POINT_STATUS_REG,
> +				&event_mode0, sizeof(struct touch_event_mode0));

[Severity: High]
This isn't a bug introduced by this patch, but does passing stack-allocated
buffers to I2C transfer functions risk memory corruption?

Looking at zinitix_read_point_status_mode0(), the stack-allocated
event_mode0 structure is passed via zinitix_read_data() to i2c_master_recv().
Because kernel stacks are typically virtually mapped with CONFIG_VMAP_STACK
and are not physically contiguous, if the underlying I2C controller driver
attempts to DMA-map this memory, it can lead to memory corruption or kernel
panics.

This pattern is pre-existing in the driver, but the new mode implementations
continue to use it.

> +	if (ret)
> +		return ret;

[ ... ]

> @@ -682,13 +808,9 @@ static int zinitix_ts_probe(struct i2c_client *client)
>  		bt541->zinitix_mode = DEFAULT_TOUCH_POINT_MODE;
>  	}
>  
> -	if (bt541->zinitix_mode != 2) {
> -		/*
> -		 * If there are devices that don't support mode 2, support
> -		 * for other modes (0, 1) will be needed.
> -		 */
> +	if (bt541->zinitix_mode > 2) {
>  		dev_err(&client->dev,
> -			"Malformed zinitix,mode property, must be 2 (supplied: %d)\n",
> +			"Malformed zinitix,mode property, must be 0, 1, or 2 (supplied: %d)\n",
>  			bt541->zinitix_mode);

[Severity: High]
This is a pre-existing issue, but could parsing and validating zinitix_mode
after the input device is registered cause a race condition with userspace?

In zinitix_ts_probe(), zinitix_init_input_dev() is called earlier, which
registers the input device and makes it available to userspace. If userspace
opens the device immediately (e.g., via udev uevents), the hardware might
be initialized with an uninitialized zinitix_mode (0 instead of the intended
mode) before this device property parsing executes. This can lead to the
interrupt handler processing garbage data.

Would it be better to read and validate the "zinitix,mode" property before
calling zinitix_init_input_dev()?

>  		return -EINVAL;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724-zinitix-modes-v1-0-f6df7d86a878@disroot.org?part=3