Re: [PATCH 2/3] Input: Add driver for ESWIN EPH8621 touchscreen IC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Double-fetch / TOCTOU vulnerability in packet length parsing allows massive out-of-bounds read and an infinite loop.
- [High] Missing length validation before parsing contact events leads to heap out-of-bounds reads.
- [Medium] Overlapping memory regions in memcpy() due to identical source and destination pointers.
- [Medium] Unvalidated slot ID silently corrupts input multi-touch state for valid contacts.
--

commit 6fa34851ae5b4f1c363bd77f86f1478eb5ce13e6
Author: Luca Weiss <luca.weiss@fairphone.com>

Input: Add driver for ESWIN EPH8621 touchscreen IC

Adds a driver for the ESWIN EPH8621 touchscreens connected via SPI.

> diff --git a/drivers/input/touchscreen/eswin_eph8621.c b/drivers/input/touchscreen/eswin_eph8621.c
> new file mode 100644
> index 0000000000000..12b90cc89052e
> --- /dev/null
> +++ b/drivers/input/touchscreen/eswin_eph8621.c
[ ... ]
> static int eswin_spi_read(struct eswin_touch *ts, u16 len, u8 *buf)
> {
> 	struct spi_device *spi = to_spi_device(ts->dev);
> 	struct spi_transfer xfer = {
> 		.tx_buf = ts->tx_buf,
> 		.rx_buf = ts->rx_buf,
> 		.len = len,
> 	};
> 	int error;
> 
> 	if (len > ESWIN_COMMS_BUF_SIZE)
> 		return -EINVAL;
> 
> 	/* ESWIN requires 0xFF dummy bytes clocked out to read data */
> 	memset(ts->tx_buf, 0xFF, len);
> 	error = spi_sync_transfer(spi, &xfer, 1);
> 	if (error)
> 		return error;
> 
> 	memcpy(buf, ts->rx_buf, len);

[Severity: Medium]
Is this memcpy() safe from overlapping memory regions?

Looking at eswin_interrupt(), it passes ts->rx_buf as the buf argument to
eswin_comms_two_stage_read(), which subsequently passes it to this function:

eswin_interrupt() {
        error = eswin_comms_two_stage_read(ts, ts->rx_buf);
...

This means buf and ts->rx_buf point to the exact same memory address. Since
the SPI transfer writes data directly into ts->rx_buf, does this memcpy()
trigger undefined behavior due to the identical source and destination
pointers?

[ ... ]
> static void eswin_report_contact(struct eswin_touch *ts, u8 *payload)
> {
> 	u8 touch_type = (payload[0] & EVENT_REPORT_TYPE_MASK) >> EVENT_REPORT_TYPE_OFFSET;
> 	u8 slot = payload[1];

[Severity: Medium]
Is the slot ID extracted from the device payload validated anywhere before
use?

If the hardware reports an invalid slot index (e.g., slot >=
ESWIN_MAX_TOUCHES), input_mt_slot() safely ignores it, leaving the active
mt->slot unchanged. However, the subsequent code unconditionally proceeds
to report X/Y coordinates and touch properties.

Could this cause the driver to apply malformed event data to the
previously active, valid slot, corrupting the multi-touch state?

[ ... ]
> static void eswin_process_report(struct eswin_touch *ts, u8 *buf)
> {
> 	u16 total_len = buf[TLV_LENGTH_FIELD] | (buf[TLV_LENGTH_FIELD + 1] << 8);

[Severity: High]
Does this recalculation introduce a double-fetch vulnerability?

The SPI controller initially reads the 3-byte header into ts->rx_buf and
safely validates payload_len. However, the second SPI transfer in
eswin_comms_two_stage_read() overwrites ts->rx_buf with the full packet.

Here, total_len is blindly recalculated from this newly fetched, unvalidated
header data. If a glitchy or malicious device provides a valid length for
the first read and a huge length (e.g., 0xFFFF) for the second, total_len
is set to an unchecked value.

> 	u16 offset = TLV_HEADER_SIZE;
> 
> 	if (buf[TLV_TYPE_FIELD] != TLV_REPORT_DATA)
> 		return;
> 
> 	while (offset < total_len + TLV_HEADER_SIZE) {

[Severity: High]
Can this loop condition result in an infinite loop and out-of-bounds reads?

If total_len was manipulated by the double-fetch issue above to be 0xFFFF,
total_len + TLV_HEADER_SIZE becomes 65538. Since offset is a u16, it can
never reach 65538, causing the condition to evaluate to true forever.

> 		u8 ev_len = buf[offset] & EVENT_REPORT_LENGTH_MASK;
> 
> 		eswin_report_contact(ts, &buf[offset]);

[Severity: High]
Can this function call trigger an out-of-bounds heap read?

The loop verifies that offset is less than the packet end, but does not
check if at least 8 bytes remain to safely parse a full contact event.

If offset is near the end of the 3072-byte buffer (e.g., offset = 3070),
eswin_report_contact() unconditionally reads up to payload[7] for the
height, which reads past the end of the heap allocation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-eswin-eph8621-v1-0-db447ff9e445@fairphone.com?part=2