Re: [PATCH 2/3] Input: mms114 - fix endianness portability in I2C packet layout
From: sashiko-bot@kernel.org
Date: 2026-07-04 06:12:02
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 [off-list ref] 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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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