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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Race condition in probe leading to NULL pointer dereference in IRQ handler
--

--- Patch [3]: [PATCH v3 3/5] Input: zinitix - add support for modes 0 and 1 ---
commit d8a438a256bc9e4876b89aa10b882e0dd303cf72
Author: Kaustabh Chakraborty <[email protected]>

Input: zinitix - add support for modes 0 and 1

The commit adds support for touch coordinate modes 0 and 1 for zinitix
touchscreens. It abstracts the reading function into a function pointer
read_point_status inside the device's private struct, assigned based on
the zinitix,mode property.

> diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
> index a0a89d25e25bb..aafd0b7d8edbd 100644
> --- a/drivers/input/touchscreen/zinitix.c
> +++ b/drivers/input/touchscreen/zinitix.c
> @@ -451,8 +563,7 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
>  
>  	memset(&touch_event, 0, sizeof(struct touch_event));
>  
> -	error = zinitix_read_data(bt541->client, ZINITIX_POINT_STATUS_REG,
> -				  &touch_event, sizeof(struct touch_event));
> +	error = bt541->read_point_status(bt541, &touch_event);

[Severity: High]
Is there a possibility of a NULL pointer dereference here?

In zinitix_ts_probe(), zinitix_init_input_dev() is called before the
read_point_status function pointer is assigned.

zinitix_init_input_dev() registers the input device, making it visible to
userspace. If a userspace process opens the device immediately and an interrupt
fires, zinitix_ts_irq_handler() could run and dereference this function pointer
while it is still NULL.

>  	if (error) {
>  		dev_err(&client->dev, "Failed to read in touchpoint struct\n");
>  		goto out;

[ ... ]

> @@ -682,13 +793,19 @@ 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.
> -		 */
> +	switch (bt541->zinitix_mode) {
> +	case 0:
> +		bt541->read_point_status = zinitix_read_point_status_mode0;
> +		break;
> +	case 1:
> +		bt541->read_point_status = zinitix_read_point_status_mode1;
> +		break;
> +	case 2:
> +		bt541->read_point_status = zinitix_read_point_status_mode2;
> +		break;
> +	default:
>  		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);
>  		return -EINVAL;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3