Re: [PATCH] Input: zinitix - support the 6-byte/count touch report used by bt532

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The `zinitix,mode` property is read and validated after `input_register_device()` is called. This creates a race condition where userspace can open and use the input device before its private state is fully configured, violating input subsystem initialization rules.
- [High] I2C transactions use stack-allocated buffers, which are unsafe for DMA.
--

commit 25626131785e9a530425fa65d2a03db62641f88e
Author: Kavan Smith <kavansmith82@gmail.com>

Input: zinitix - support the 6-byte/count touch report used by bt532

This commit adds a per-compatible struct zinitix_chip_data to select
the report stride and how byte[2] is interpreted. It provides support
for firmware packing contacts at a 6-byte stride and reporting a finger
count, fixing multitouch behavior on devices like the Samsung Galaxy Tab
A 8.0.

> diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
> index 0c36765bd79f8..04ee4d804b511 100644
> --- a/drivers/input/touchscreen/zinitix.c
> +++ b/drivers/input/touchscreen/zinitix.c

[ ... ]

> @@ -452,8 +475,11 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
>  
>  	memset(&touch_event, 0, sizeof(struct touch_event));
>  
> +	/* header (status, finger_mask, time_stamp) + one stride per contact */
>  	error = zinitix_read_data(bt541->client, ZINITIX_POINT_STATUS_REG,
> -				  &touch_event, sizeof(struct touch_event));
> +				  &touch_event,
> +				  offsetof(struct touch_event, coords) +
> +					  MAX_SUPPORTED_FINGER_NUM * stride);

[Severity: High]
This is a pre-existing issue, but does zinitix_read_data() perform safe I2C
transactions here?

The &touch_event pointer points to stack memory and is passed to
i2c_master_recv() inside zinitix_read_data(). Passing stack-allocated buffers
to I2C transfer functions can violate DMA safety rules. If the I2C host
controller driver maps these buffers for DMA, it can corrupt adjacent stack
variables on non-cache-coherent architectures.

This same issue seems to exist with &reg_le in zinitix_read_data() and the
packet array in zinitix_write_u16(). Would it be possible to allocate these
buffers dynamically to avoid potential DMA stack corruption?

[ ... ]

> @@ -627,6 +671,7 @@ static int zinitix_ts_probe(struct i2c_client *client)
>  		return -ENOMEM;
>  
>  	bt541->client = client;
> +	bt541->chip = device_get_match_data(&client->dev);
>  	i2c_set_clientdata(client, bt541);
>  
>  	error = zinitix_init_regulators(bt541);

[Severity: High]
This isn't a bug introduced by this patch, but looking further down in
zinitix_ts_probe(), zinitix_init_input_dev() is called to register the input
device before reading the zinitix,mode property:

	error = zinitix_init_input_dev(bt541);
	if (error) {
		dev_err(&client->dev,
			"Failed to initialize input device: %d\n", error);
		return error;
	}

	error = device_property_read_u32(&client->dev, "zinitix,mode",
					 &bt541->zinitix_mode);

Does this create a race condition where userspace could open and use the
input device before its private state is fully configured?

If userspace opens the device in this window, zinitix_input_open() might use
bt541->zinitix_mode while it is still 0 (from kzalloc). This could configure
the hardware in an invalid state instead of the device-tree specified mode.
Should the property be read and validated before calling
input_register_device()?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706180826.408989-1-kavansmith82@gmail.com?part=1