Re: [PATCH 1/3] Input: mms114 - fix multi-touch slot corruption
From: sashiko-bot@kernel.org
Date: 2026-07-04 06:11:01
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: Pre-existing issues: - [High] If `mms114_start()` fails during system resume, subsequent device close or suspend causes unbalanced `disable_irq()` and `regulator_disable()` calls, leaving the touchscreen permanently broken. - [High] The suspend callback releases all touches before disabling the device interrupt, creating a race window where touches can get permanently stuck. - [High] Stack-allocated buffers are passed directly to I2C transfer functions, which can cause DMA mapping failures or memory corruption. -- commit 6a55987ddd51d1330fd1636cd3e72b4d910ea42b Author: Dmitry Torokhov [off-list ref] Input: mms114 - fix multi-touch slot corruption This commit fixes a multi-touch slot corruption bug in the mms114 driver. When the touchscreen controller reports a touch ID of 0, the driver calculates the slot ID as touch->id - 1, which underflows to UINT_MAX. The driver fixes this by rejecting touch reports with ID 0.
quoted hunk ↗ jump to hunk
diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c index 006dded17eb80..23e0283bc6b8b 100644 --- a/drivers/input/touchscreen/mms114.c +++ b/drivers/input/touchscreen/mms114.c@@ -248,7 +248,7 @@ static void mms114_process_mt(struct mms114_data *data, struct mms114_touch *tou unsigned int x; unsigned int y; - if (touch->id > MMS114_MAX_TOUCH) { + if (touch->id == 0 || touch->id > MMS114_MAX_TOUCH) { dev_err(&client->dev, "Wrong touch id (%d)\n", touch->id); return; }
[Severity: High]
This isn't a bug introduced by this patch, but I have a question about the
error handling in mms114_resume().
If mms114_start() fails (e.g. mms114_setup_regs() returns an error) it cleans
up the regulators but doesn't call enable_irq():
static int mms114_start(struct mms114_data *data)
{
...
error = mms114_setup_regs(data);
if (error < 0) {
regulator_disable(data->io_reg);
regulator_disable(data->core_reg);
return error;
}
enable_irq(client->irq);
return 0;
}
In mms114_resume(), the error is passed back up but the input device remains
logically open:
static int mms114_resume(struct device *dev)
{
...
if (input_device_enabled(input_dev)) {
error = mms114_start(data);
if (error)
return error;
}
...
}
Because the input device is still logically enabled, won't a subsequent
suspend or input_close call mms114_stop(), which unconditionally calls
disable_irq() and regulator_disable()? Does this cause an unbalanced IRQ
disable depth that permanently masks the interrupt?
[Severity: High]
This is a pre-existing issue, but I noticed a potential race condition in
mms114_suspend().
The suspend callback releases all active touch slots before stopping the
device and disabling the interrupt:
static int mms114_suspend(struct device *dev)
{
...
/* Release all touch */
for (id = 0; id < MMS114_MAX_TOUCH; id++) {
input_mt_slot(input_dev, id);
input_mt_report_slot_inactive(input_dev);
}
input_mt_report_pointer_emulation(input_dev, true);
input_sync(input_dev);
guard(mutex)(&input_dev->mutex);
if (input_device_enabled(input_dev))
mms114_stop(data);
...
}
If a touch interrupt fires between the synthetic slot release and the call to
mms114_stop() (which disables the IRQ), could the physical touches be reported
as active again just as the device is suspended, leaving the touches
permanently stuck on resume?
[Severity: High]
This isn't a bug introduced by this patch, but there seem to be multiple
places where stack-allocated buffers are used for I2C transfers.
For example, in __mms114_read_reg(), a single byte stack variable is passed:
static int __mms114_read_reg(struct mms114_data *data, u8 reg,
unsigned int len, void *val)
{
...
u8 buf = reg;
...
xfer[0].buf = &buf;
...
error = i2c_transfer(client->adapter, xfer, 2);
...
}
Similarly in mms114_write_reg():
static int mms114_write_reg(struct mms114_data *data, u8 reg, u8 val)
{
...
u8 buf[2];
buf[0] = reg;
buf[1] = val;
error = i2c_master_send(client, buf, 2);
...
}
And in mms114_interrupt():
struct mms114_touch touch[MMS114_MAX_TOUCH];
...
error = __mms114_read_reg(data, MMS114_INFORMATION, packet_size, touch);
...
Since many I2C controllers use DMA, doesn't passing stack memory to
i2c_transfer() risk DMA mapping failures or memory corruption when
CONFIG_VMAP_STACK is enabled?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260704060115.353049-1-dmitry.torokhov@gmail.com?part=1