[PATCH] Input: mms114 - reject an oversized device packet size

[PATCH] Input: mms114 - reject an oversized device packet size

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

mms114_interrupt() reads a packet of touch data from the device into a
fixed-size on-stack buffer

	struct mms114_touch touch[MMS114_MAX_TOUCH];

which holds MMS114_MAX_TOUCH (10) events of MMS114_EVENT_SIZE (8) bytes,
i.e. 80 bytes. The length of the I2C read into it is taken verbatim from
the device:

	packet_size = mms114_read_reg(data, MMS114_PACKET_SIZE);
	if (packet_size <= 0)
		goto out;
	...
	error = __mms114_read_reg(data, MMS114_INFORMATION, packet_size,
			(u8 *)touch);

packet_size is a single device register byte (0x0F) and the only check
is the lower bound packet_size <= 0; it is never bounded against the
size of touch[]. A malfunctioning, malicious or counterfeit controller
(or an attacker tampering with the I2C bus) can report a packet_size of
up to 255, so __mms114_read_reg() writes up to 175 bytes past the end of
touch[] on the IRQ-thread stack: a stack out-of-bounds write that can
overwrite the stack canary, saved registers and the return address.

A well-formed device never reports more than the buffer holds, so reject
an oversized packet and drop the report, consistent with the handler's
other error paths, rather than reading past the buffer.

Fixes: 07b8481d4aff ("Input: add MELFAS mms114 touchscreen driver")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 drivers/input/touchscreen/mms114.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index af462086a65c..4c75f16c503d 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -226,6 +226,13 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
 	if (packet_size <= 0)
 		goto out;
 
+	/* the device controls packet_size; reject anything too big for touch[] */
+	if (packet_size > (int)sizeof(touch)) {
+		dev_err(&client->dev, "Invalid packet size %d (max %zu)\n",
+			packet_size, sizeof(touch));
+		goto out;
+	}
+
 	/* MMS136 has slightly different event size */
 	if (data->type == TYPE_MMS134S || data->type == TYPE_MMS136)
 		touch_size = packet_size / MMS136_EVENT_SIZE;

---
base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
change-id: 20260612-b4-disp-dc4b8dc4-8e39b9d64247

Best regards,
-- 
Bryam Vargas <hexlabsecurity@proton.me>

Re: [PATCH] Input: mms114 - reject an oversized device packet size

[sashiko-bot]

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

Pre-existing issues:
- [High] Stack out-of-bounds read and severe struct misalignment when parsing MMS136/MMS134S touch events, leading to a kernel memory information leak.
--

commit 2622e1ec203d4e3a475cf8bb039715b16ecc29d8
Author: Bryam Vargas <hexlabsecurity@proton.me>

Input: mms114 - reject an oversized device packet size

This commit adds a bounds check to reject packet sizes from the device that
exceed the size of the local touch array. This prevents a stack out-of-bounds
write caused by a malfunctioning or malicious device.

Cc: stable@vger.kernel.org

> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index af462086a65ca..4c75f16c503dc 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -226,6 +226,13 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
>  	if (packet_size <= 0)
>  		goto out;
>  
> +	/* the device controls packet_size; reject anything too big for touch[] */
> +	if (packet_size > (int)sizeof(touch)) {
> +		dev_err(&client->dev, "Invalid packet size %d (max %zu)\n",
> +			packet_size, sizeof(touch));
> +		goto out;
> +	}
> +
>  	/* MMS136 has slightly different event size */
>  	if (data->type == TYPE_MMS134S || data->type == TYPE_MMS136)
>  		touch_size = packet_size / MMS136_EVENT_SIZE;

[Severity: High]
This is a pre-existing issue, but does this calculation cause an out-of-bounds
read and structure misalignment later in the function?

If packet_size is up to 80 (the maximum allowed by the newly added bounds
check), touch_size for MMS136 and MMS134S devices will be 80 / 6 = 13.

In the loop further down in mms114_interrupt():

    for (index = 0; index < touch_size; index++) {
        switch (touch[index].type) {

When index reaches 12, touch[12] reads 8 bytes starting from offset 96,
because sizeof(struct mms114_touch) is 8 bytes. Since the touch array is only
80 bytes long, wouldn't this read past the end of the stack buffer?

Also, since the hardware sends contiguous 6-byte events but they are iterated
using an 8-byte array step, does this misalign the parsing for every event
after the first one?

This misaligned and out-of-bounds stack data is then processed in
mms114_process_mt(), where touch->width and touch->strength are passed to
input_report_abs(). Could this inadvertently leak uninitialized stack memory to
userspace?

>  	else
>  		touch_size = packet_size / MMS114_EVENT_SIZE;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-b4-disp-dc4b8dc4-v1-1-d7cb0a828d92@proton.me?part=1