[PATCH 1/3] Input: mms114 - fix multi-touch slot corruption

[PATCH 1/3] Input: mms114 - fix multi-touch slot corruption

[Dmitry Torokhov]

If the touchscreen controller reports a touch ID of 0, the driver
calculates the slot ID as touch->id - 1, which underflows to UINT_MAX.
This is passed to input_mt_slot() as -1.

Since the input core ignores negative slot values, the active slot remains
unchanged. The driver then reports the touch coordinates for the previously
active slot, corrupting its state.

Fix this by rejecting touch reports with ID 0.

Fixes: 07b8481d4aff ("Input: add MELFAS mms114 touchscreen driver")
Cc: stable@vger.kernel.org
Reported-by: sashiko-bot@kernel.org
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/mms114.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index 006dded17eb8..23e0283bc6b8 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;
 	}
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH 2/3] Input: mms114 - fix endianness portability in I2C packet layout

[Dmitry Torokhov]

The driver defines the I2C packet layout using C bitfields in struct
mms114_touch. This is not portable as the layout of bitfields within a
byte is compiler-dependent and varies with endianness. On Big Endian
systems, the fields will be parsed incorrectly.

Fix this by redefining struct mms114_touch with plain u8 fields and
introducing bitwise macros to extract the values portably.

Reported-by: sashiko-bot@kernel.org
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/mms114.c | 52 +++++++++++++++++++-----------
 1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index 23e0283bc6b8..84afdadb3bcc 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -4,6 +4,8 @@
 // Copyright (c) 2012 Samsung Electronics Co., Ltd.
 // Author: Joonyoung Shim <jy0922.shim@samsung.com>
 
+#include <linux/bitfield.h>
+#include <linux/bits.h>
 #include <linux/module.h>
 #include <linux/delay.h>
 #include <linux/of.h>
@@ -76,9 +78,16 @@ struct mms_chip {
 	int (*get_version)(struct mms114_data *data);
 };
 
+#define MMS114_FLAGS_ID_MASK		GENMASK(3, 0)
+#define MMS114_FLAGS_TYPE_MASK		GENMASK(6, 5)
+#define MMS114_FLAGS_PRESSED_MASK	BIT(7)
+
+#define MMS114_XY_HI_X_MASK		GENMASK(3, 0)
+#define MMS114_XY_HI_Y_MASK		GENMASK(7, 4)
+
 struct mms114_touch {
-	u8 id:4, reserved_bit4:1, type:2, pressed:1;
-	u8 x_hi:4, y_hi:4;
+	u8 flags;
+	u8 xy_hi;
 	u8 x_lo;
 	u8 y_lo;
 	u8 width;
@@ -244,28 +253,30 @@ static void mms114_process_mt(struct mms114_data *data, struct mms114_touch *tou
 {
 	struct i2c_client *client = data->client;
 	struct input_dev *input_dev = data->input_dev;
-	unsigned int id;
+	unsigned int id = FIELD_GET(MMS114_FLAGS_ID_MASK, touch->flags);
+	unsigned int type = FIELD_GET(MMS114_FLAGS_TYPE_MASK, touch->flags);
+	bool pressed = FIELD_GET(MMS114_FLAGS_PRESSED_MASK, touch->flags);
 	unsigned int x;
 	unsigned int y;
 
-	if (touch->id == 0 || touch->id > MMS114_MAX_TOUCH) {
-		dev_err(&client->dev, "Wrong touch id (%d)\n", touch->id);
+	if (id == 0 || id > MMS114_MAX_TOUCH) {
+		dev_err(&client->dev, "Wrong touch id (%d)\n", id);
 		return;
 	}
 
-	id = touch->id - 1;
-	x = touch->x_lo | touch->x_hi << 8;
-	y = touch->y_lo | touch->y_hi << 8;
+	id--;
+	x = touch->x_lo | FIELD_GET(MMS114_XY_HI_X_MASK, touch->xy_hi) << 8;
+	y = touch->y_lo | FIELD_GET(MMS114_XY_HI_Y_MASK, touch->xy_hi) << 8;
 
 	dev_dbg(&client->dev,
 		"id: %d, type: %d, pressed: %d, x: %d, y: %d, width: %d, strength: %d\n",
-		id, touch->type, touch->pressed,
+		id, type, pressed,
 		x, y, touch->width, touch->strength);
 
 	input_mt_slot(input_dev, id);
-	input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, touch->pressed);
+	input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, pressed);
 
-	if (touch->pressed) {
+	if (pressed) {
 		touchscreen_report_pos(input_dev, &data->props, x, y, true);
 		input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, touch->width);
 		input_report_abs(input_dev, ABS_MT_PRESSURE, touch->strength);
@@ -278,21 +289,23 @@ static void mms114_process_touchkey(struct mms114_data *data,
 	struct i2c_client *client = data->client;
 	struct input_dev *input_dev = data->input_dev;
 	unsigned int keycode_id;
+	unsigned int id = FIELD_GET(MMS114_FLAGS_ID_MASK, touch->flags);
+	bool pressed = FIELD_GET(MMS114_FLAGS_PRESSED_MASK, touch->flags);
 
-	if (touch->id == 0)
+	if (id == 0)
 		return;
 
-	if (touch->id > data->num_keycodes) {
+	if (id > data->num_keycodes) {
 		dev_err(&client->dev, "Wrong touch id for touchkey (%d)\n",
-			touch->id);
+			id);
 		return;
 	}
 
-	keycode_id = touch->id - 1;
+	keycode_id = id - 1;
 	dev_dbg(&client->dev, "keycode id: %d, pressed: %d\n", keycode_id,
-		touch->pressed);
+		pressed);
 
-	input_report_key(input_dev, data->keycodes[keycode_id], touch->pressed);
+	input_report_key(input_dev, data->keycodes[keycode_id], pressed);
 }
 
 static irqreturn_t mms114_interrupt(int irq, void *dev_id)
@@ -325,8 +338,9 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
 
 	for (index = 0; index < touch_size; index++) {
 		t = (struct mms114_touch *)((u8 *)touch + index * event_size);
+		unsigned int type = FIELD_GET(MMS114_FLAGS_TYPE_MASK, t->flags);
 
-		switch (t->type) {
+		switch (type) {
 		case MMS114_TYPE_TOUCHSCREEN:
 			mms114_process_mt(data, t);
 			break;
@@ -337,7 +351,7 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
 
 		default:
 			dev_err(&client->dev, "Wrong touch type (%d)\n",
-				t->type);
+				type);
 			break;
 		}
 	}
-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH 2/3] Input: mms114 - fix endianness portability in I2C packet layout

[sashiko-bot]

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

Pre-existing issues:
- [High] Race condition in suspend causes stuck touches.
- [High] Unsafe DMA buffer allocation (stack-allocated memory passed to I2C transfers).
--

commit 2e96699ad380d6d47c8e998bc41268e9b77bac90
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: mms114 - fix endianness portability in I2C packet layout

The driver defines the I2C packet layout using C bitfields in struct
mms114_touch. This is not portable as the layout of bitfields within a
byte is compiler-dependent and varies with endianness. On Big Endian
systems, the fields will be parsed incorrectly.

> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index 23e0283bc6b8b..84afdadb3bccc 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c

[ ... ]

> @@ -325,8 +338,9 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
>  
>  	for (index = 0; index < touch_size; index++) {
>  		t = (struct mms114_touch *)((u8 *)touch + index * event_size);

[Severity: High]
This isn't a bug introduced by this patch, but does passing stack-allocated
buffers to I2C transfer functions risk memory corruption if the I2C adapter
uses DMA?

Since kernel stacks are often allocated in vmalloc space (when
CONFIG_VMAP_STACK is enabled), they cannot be safely mapped for DMA.

In mms114_interrupt(), the touch array is stack-allocated and used as a
receive buffer:

	struct mms114_touch touch[MMS114_MAX_TOUCH];
	// ...
	error = __mms114_read_reg(data, MMS114_INFORMATION, packet_size, touch);

Similarly, __mms114_read_reg() and mms114_write_reg() pass local stack
variables (u8 buf and u8 buf[2]) to I2C transfers.

> +		unsigned int type = FIELD_GET(MMS114_FLAGS_TYPE_MASK, t->flags);
>  
> -		switch (t->type) {
> +		switch (type) {
>  		case MMS114_TYPE_TOUCHSCREEN:
>  			mms114_process_mt(data, t);
>  			break;

[ ... ]

[Severity: High]
This is a pre-existing issue, but could the sequence in mms114_suspend()
have a race condition that results in stuck touches?

In mms114_suspend(), the driver injects artificial release events before
disabling the IRQ:

	/* 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_sync(input_dev);

	guard(mutex)(&input_dev->mutex);

	if (input_device_enabled(input_dev))
		mms114_stop(data);

If a touch interrupt fires and its threaded handler (mms114_interrupt)
executes after these release events are injected but before mms114_stop()
disables the IRQ, might it report the touch as active again and leave it
stuck indefinitely after suspend?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704060115.353049-1-dmitry.torokhov@gmail.com?part=2

[PATCH 3/3] Input: mms114 - fix Y-resolution configuration

[Dmitry Torokhov]

In mms114_setup_regs(), the driver mistakenly uses props->max_x instead
of props->max_y when configuring the low bits of the Y resolution
(MMS114_Y_RESOLUTION).

Fix this by using the correct property.

Fixes: 07b8481d4aff ("Input: add MELFAS mms114 touchscreen driver")
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/mms114.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index 84afdadb3bcc..27911a9f4e9e 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -408,7 +408,7 @@ static int mms114_setup_regs(struct mms114_data *data)
 	if (error < 0)
 		return error;
 
-	val = props->max_x & 0xff;
+	val = props->max_y & 0xff;
 	error = mms114_write_reg(data, MMS114_Y_RESOLUTION, val);
 	if (error < 0)
 		return error;
-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH 3/3] Input: mms114 - fix Y-resolution configuration

[sashiko-bot]

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

Pre-existing issues:
- [High] Unsafe DMA buffer allocation on the stack for I2C transfers.
--

commit b2ece0d05aab20ebd4014035e04b87c4d9127c9f
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Subject: Input: mms114 - fix Y-resolution configuration

This commit fixes a copy-paste error in mms114_setup_regs() where
props->max_x was being used instead of props->max_y to configure the
lower bits of the Y resolution.

> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index 84afdadb3bccc..27911a9f4e9eb 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -408,7 +408,7 @@ static int mms114_setup_regs(struct mms114_data *data)
>  	if (error < 0)
>  		return error;
>  
> -	val = props->max_x & 0xff;
> +	val = props->max_y & 0xff;
>  	error = mms114_write_reg(data, MMS114_Y_RESOLUTION, val);

[Severity: High]
This is a pre-existing issue, but since mms114_write_reg() is called here,
is it safe for the driver to use stack-allocated buffers for I2C transfers?

Looking at mms114_write_reg():

static int mms114_write_reg(struct mms114_data *data, u8 reg, u8 val)
{
        struct i2c_client *client = data->client;
        u8 buf[2];
        ...
        buf[0] = reg;
        buf[1] = val;

        error = i2c_master_send(client, buf, 2);

If the underlying I2C adapter uses DMA for transfers and CONFIG_VMAP_STACK is
enabled, passing a stack-allocated buffer to i2c_master_send() can cause memory
corruption or kernel panics because stack memory is not DMA-safe.

A similar pattern appears in __mms114_read_reg() and mms114_interrupt():

static int __mms114_read_reg(struct mms114_data *data, unsigned int reg, ...)
{
        ...
        u8 buf = reg;
        ...
        xfer[0].buf = &buf;
        ...
        error = i2c_transfer(client->adapter, xfer, 2);

and in mms114_interrupt(), a large array is allocated on the stack and passed
down through __mms114_read_reg():

        struct mms114_touch touch[MMS114_MAX_TOUCH];
        ...
        error = __mms114_read_reg(data, MMS114_INFORMATION,
                                  MMS114_MAX_TOUCH * sizeof(struct mms114_touch),
                                  (u8 *)touch);

Could these stack-allocated buffers lead to instability on DMA-capable I2C
controllers? Should these buffers be moved to the mms114_data struct so they
are dynamically allocated on the heap?

>  	if (error < 0)
>  		return error;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704060115.353049-1-dmitry.torokhov@gmail.com?part=3

Re: [PATCH 1/3] Input: mms114 - fix multi-touch slot corruption

[sashiko-bot]

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 <dmitry.torokhov@gmail.com>

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.

> 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