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

[sashiko-bot@kernel.org]

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

New issues:
- [Low] The validation check in `zinitix_report_finger()` is mathematically impossible to trigger, turning the validation block into dead code and bypassing all event validation.

Pre-existing issues:
- [High] The input device is registered before all driver private data is fully initialized, exposing incomplete state to userspace and violating subsystem rules.
- [High] Regulators enabled in `zinitix_start()` are not disabled on initialization failure paths, leading to a leaked reference count.
- [High] I2C transfers are performed using stack-allocated buffers, which violates kernel safety guidelines for DMA-mapped memory.
--

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

Input: zinitix - do not ignore non-moving fingers

This commit addresses an issue with the ZT7548 touchscreen where non-moving
fingers are ignored. It updates the validation mask in zinitix_report_finger()
to include the SUB_BIT_EXIST flag. This ensures fingers that haven't moved
since the last interrupt are properly recognized.

> diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
> index 3421b8ffb19b2..fdcb80f52c917 100644
> --- a/drivers/input/touchscreen/zinitix.c
> +++ b/drivers/input/touchscreen/zinitix.c
> @@ -406,7 +406,7 @@ static void zinitix_report_finger(struct bt541_ts_data *bt541, int slot,
>  	u16 x, y;
>  
>  	if (unlikely(!(p->sub_status &
> -		       (SUB_BIT_UP | SUB_BIT_DOWN | SUB_BIT_MOVE)))) {
> +		       (SUB_BIT_EXIST | SUB_BIT_UP | SUB_BIT_DOWN | SUB_BIT_MOVE)))) {
[Severity: Low]
Is this validation check mathematically impossible to trigger now?

The caller zinitix_ts_irq_handler() already verifies if SUB_BIT_EXIST is set:

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

Since SUB_BIT_EXIST is strictly guaranteed to be present when this function
is called, the bitwise AND always evaluates to a non-zero value.

This means the negated condition will always evaluate to false, rendering
the entire event validation block and its debug log dead code.

>  		dev_dbg(&bt541->client->dev, "unknown finger event %#02x\n",
>  			p->sub_status);
>  		return;

[Severity: High]
This isn't a bug introduced by this patch, but does the input device
registration happen before all driver private data is fully initialized?

In zinitix_ts_probe():
    ...
    error = zinitix_init_input_dev(bt541);
    ...
    error = device_property_read_u32(&client->dev, "zinitix,mode",
                                     &bt541->zinitix_mode);
    ...

If a userspace process opens the input device immediately upon registration
in zinitix_init_input_dev(), the driver could configure the hardware using
an uninitialized bt541->zinitix_mode.

Could this place the touchscreen into an unsupported mode?

[Severity: High]
This is a pre-existing issue, but are the regulators enabled in
zinitix_start() correctly disabled on initialization failure paths?

zinitix_start() {
    ...
    error = regulator_bulk_enable(ARRAY_SIZE(bt541->supplies),
                                  bt541->supplies);
    ...
    error = zinitix_send_power_on_sequence(bt541);
    if (error) {
        dev_err(&bt541->client->dev,
                "Error while sending power-on sequence: %d\n", error);
        return error;
    }
    ...
}

If zinitix_send_power_on_sequence() or zinitix_init_touch() fail, the
function returns without disabling the regulators.

Since the input subsystem considers the device closed on open failure, it
will not call zinitix_input_close().

Could repeated open attempts leak the regulator references?

[Severity: High]
This is a pre-existing issue, but do the I2C transfers use stack-allocated
buffers?

zinitix_write_cmd() {
    __le16 reg_le = cpu_to_le16(reg);
    int ret;

    ret = i2c_master_send(client, (u8 *)&reg_le, sizeof(reg_le));
    ...
}

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));
    ...
}

Passing stack-allocated buffers to I2C transfer functions can lead to
memory corruption or DMA API warnings if the underlying I2C controller
driver attempts to use DMA.

Are these buffers safe given that stack memory is not guaranteed to be
DMA-coherent?

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