Re: [PATCH 1/5] Input: zinitix - check all available fingers for every touch event

[sashiko-bot@kernel.org]

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

New issues:
- [High] The patch reintroduces the 'shadow fingers' bug on BT404 touchscreens by reverting the core logic of commit e941dc13fd37.

Pre-existing issues:
- [High] The driver passes stack-allocated variables as buffers to I2C transfer functions (`i2c_master_send`, `i2c_master_recv`), which violates the I2C API contract and is unsafe if the subsystem uses DMA.
--

--- Patch [1]: [PATCH 1/5] Input: zinitix - check all available fingers for every touch event ---
commit 3c2f3ba29bf59a45063f7eee69e37d4b205c7c37
Author: Kaustabh Chakraborty <kauschluss@disroot.org>

Input: zinitix - check all available fingers for every touch event

This commit modifies the zinitix touchscreen driver to unconditionally iterate
over all available finger slots up to MAX_SUPPORTED_FINGER_NUM. It relies
only on SUB_BIT_EXIST instead of using the finger_mask field.

Link: https://android.googlesource.com/kernel/bcm/+/23d376ef33aa4c500a5ea24a290f029d5f8e2de3/drivers/input/touchscreen/zinitix_touch.c#1942 [1]

> diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
> index 0c36765bd79f8..3421b8ffb19b2 100644
> --- a/drivers/input/touchscreen/zinitix.c
> +++ b/drivers/input/touchscreen/zinitix.c
> @@ -445,7 +445,6 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
>  	struct bt541_ts_data *bt541 = bt541_handler;
>  	struct i2c_client *client = bt541->client;
>  	struct touch_event touch_event;

[Severity: High]
This isn't a bug introduced by this patch, but does passing this
stack-allocated touch_event directly to zinitix_read_data() cause problems
with I2C DMA transfers?

Since zinitix_read_data() wraps i2c_master_recv() and passes this buffer:

zinitix_ts_irq_handler() {
    ...
    error = zinitix_read_data(bt541->client, ZINITIX_POINT_STATUS_REG,
                              &touch_event, sizeof(struct touch_event));
    ...
}

Could this lead to memory corruption or kernel panics when CONFIG_VMAP_STACK
is enabled, as stack memory isn't physically contiguous for DMA? This same
pattern appears with local variables passed to i2c_master_send() in
zinitix_read_data() and zinitix_write_u16().

> -	unsigned long finger_mask;
>  	__le16 icon_events;
>  	int error;
>  	int i;

[ ... ]

> @@ -470,8 +469,7 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
>  		zinitix_report_keys(bt541, le16_to_cpu(icon_events));
>  	}
>  
> -	finger_mask = touch_event.finger_mask;
> -	for_each_set_bit(i, &finger_mask, MAX_SUPPORTED_FINGER_NUM) {
> +	for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++) {

[Severity: High]
Does this change reintroduce the shadow fingers regression on BT404
touchscreens?

By unconditionally iterating over all slots and relying solely on
SUB_BIT_EXIST instead of the finger_mask:

zinitix_ts_irq_handler() {
    ...
    for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++) {
        const struct point_coord *p = &touch_event.point_coord[i];

        if (p->sub_status & SUB_BIT_EXIST)
            zinitix_report_finger(bt541, i, p);
    ...
}

Won't this break functionality on BT404 firmwares which erroneously populate
multiple slots with SUB_BIT_EXIST set for a single physical touch? This
appears to revert the core logic explicitly introduced in commit
e941dc13fd37 to filter out those phantom fingers.

>  		const struct point_coord *p = &touch_event.point_coord[i];
>  
>  		/* Only process contacts that are actually reported */

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