Re: [PATCH v3 1/5] Input: zinitix - check all available fingers for every touch event
flat view
From: sashiko-bot@kernel.org
Date: 2026-07-30 20:32:02
Also in:
linux-devicetree
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: New issues: - [High] Unconditionally iterating all slots without a mask reinstates a known phantom fingers bug on certain devices. Pre-existing issues: - [High] The input device is registered and exposed to userspace before its operational properties are fully parsed and initialized. - [High] Failing to read icon events discards already successfully read touch events, bypassing synchronization. - [High] Stack-allocated variables are used directly as data buffers for I2C transfers, violating DMA safety requirements. --
--- Patch [1]: [PATCH v3 1/5] Input: zinitix - check all available fingers for every touch event ---commit 324dca54b2be4e95e88ba6bec7c1bec867e0e687 Author: Kaustabh Chakraborty [off-list ref] Input: zinitix - check all available fingers for every touch event This patch changes the touch reporting to iterate unconditionally over all slots rather than relying on the finger_mask field. It relies on the SUB_BIT_EXIST bit in the finger status to filter valid touches. Link: https://android.googlesource.com/kernel/bcm/+/23d376ef33aa4c500a5ea24a290f029d5f8e2de3/drivers/input/touchscreen/zinitix_touch.c#1942 [1]
quoted hunk ↗ jump to hunk
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 touch_event is allocated on
the stack and then passed to zinitix_read_data(), which feeds it directly
to the I2C subsystem. Since this buffer might be used for DMA transfers on
certain I2C host controllers, can this cause memory corruption or a kernel
panic when CONFIG_VMAP_STACK is enabled?
Similarly, in zinitix_read_data():
zinitix_read_data() {
__le16 reg_le = cpu_to_le16(reg);
...
ret = i2c_master_send(client, (u8 *)®_le, sizeof(reg_le));
...
}
Are there similar DMA safety concerns when using the stack-allocated reg_le
buffer for I2C sending?
quoted hunk ↗ jump to hunk
- 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)); }
[Severity: High]
This is a pre-existing issue, but I noticed something about the error handling
just above this block:
zinitix_ts_irq_handler() {
...
if (le16_to_cpu(touch_event.status) & BIT_ICON_EVENT) {
error = zinitix_read_data(bt541->client, bt541->icon_status_reg,
&icon_events, sizeof(icon_events));
if (error) {
dev_err(&client->dev, "Failed to read icon events\n");
goto out;
}
...
}
...
out:
zinitix_write_cmd(bt541->client, ZINITIX_CLEAR_INT_STATUS_CMD);
return IRQ_HANDLED;
}
If zinitix_read_data() for the icon events fails, the code jumps directly to
out. Will this discard the successfully read touch_event data and skip the
input_mt_sync_frame() and input_sync() calls, potentially leaving stuck touches
in the input core?
quoted hunk ↗ jump to hunk
- 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++) { const struct point_coord *p = &touch_event.point_coord[i]; /* Only process contacts that are actually reported */
[Severity: High]
The commit message mentions that phantom fingers will be ignored because
zinitix_report_finger() checks the SUB_BIT_EXIST bit. However, the
finger_mask check was originally introduced (in commit e941dc13fd37) because
some hardware erroneously sets SUB_BIT_EXIST for phantom fingers.
By removing the finger_mask iteration, is it possible this reinstates the
known phantom touches bug on affected hardware (like the BT404) where a
single physical touch causes multiple active slots?
[Severity: High]
This isn't a bug introduced by this patch, but looking at the device
initialization in zinitix_ts_probe():
zinitix_ts_probe() {
...
error = zinitix_init_input_dev(bt541);
if (error) {
...
}
error = device_property_read_u32(&client->dev, "zinitix,mode",
&bt541->zinitix_mode);
...
}
Since zinitix_init_input_dev() calls input_register_device(), the input
device becomes immediately visible to userspace. If userspace opens the device
instantly, zinitix_input_open() -> zinitix_start() will run before the
zinitix,mode property is read.
Could this cause the hardware to be configured with an incorrect mode
(0 instead of the requested or default 2)?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-zinitix-modes-v3-0-2c8de712997a@disroot.org?part=1