Re: [PATCH] Input: elants_i2c - prevent divide-by-zero on invalid EKTH data

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] A 0-byte firmware file causes out-of-bounds reads when verifying the remark ID and when sending firmware pages to the device.
- [High] Stack-allocated arrays are passed to i2c_transfer(), which is unsafe if the underlying I2C controller uses DMA.
--

commit 4ca9e1ffa7cb9eddbc7a0070d2fc1df01f66c3cd
Author: Laxman Acharya Padhya <acharyalaxman8848@gmail.com>

Input: elants_i2c - prevent divide-by-zero on invalid EKTH data

This commit adds checks for physical dimensions in the eKTH query function.
It treats zero values as invalid data and skips resolution calculations
to prevent a divide-by-zero error during probe.

Fixes: 66aee90088da ("Input: add support for Elan eKTH I2C touchscreens")
Assisted-by: Codex:gpt-5

> diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
> index 17175adcaebbd..68c5ee3aa1904 100644
> --- a/drivers/input/touchscreen/elants_i2c.c
> +++ b/drivers/input/touchscreen/elants_i2c.c
> @@ -566,10 +566,11 @@ static int elants_i2c_query_ts_info_ekth(struct elants_data *ts)
>  
>  	dev_dbg(&client->dev, "phy_x=%d, phy_y=%d\n", phy_x, phy_y);
>  
> -	if (rows == 0 || cols == 0 || osr == 0) {
> +	if (rows == 0 || cols == 0 || osr == 0 ||
> +	    phy_x == 0 || phy_y == 0) {
>  		dev_warn(&client->dev,
> -			 "invalid trace number data: %d, %d, %d\n",
> -			 rows, cols, osr);
> +			 "invalid data: rows=%u cols=%u osr=%u phy_x=%u phy_y=%u\n",
> +			 rows, cols, osr, phy_x, phy_y);
>  	} else {
>  		/* translate trace number to TS resolution */
>  		ts->x_max = ELAN_TS_RESOLUTION(rows, osr);

[Severity: High]
This isn't a bug introduced by this patch, but while looking at
elants_i2c_query_ts_info_ekth(), could there be an issue with how the command
buffers are passed to the I2C subsystem?

drivers/input/touchscreen/elants_i2c.c:elants_i2c_query_ts_info_ekth() {
...
	u8 resp[17];
	const u8 get_resolution_cmd[] = {
		CMD_HEADER_6B_READ, 0x00, 0x00, 0x00, 0x00, 0x00
...
	error = elants_i2c_execute_command(client,
					   get_resolution_cmd,
					   sizeof(get_resolution_cmd),
					   resp, sizeof(resp), 1,
					   "get resolution");
}

When elants_i2c_execute_command() assigns these buffers directly to the I2C
message:

drivers/input/touchscreen/elants_i2c.c:elants_i2c_execute_command() {
...
		msgs[0].buf = (u8 *)cmd;
...
		msgs[1].buf = resp;

		ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
}

If the I2C controller uses DMA, doesn't passing stack-allocated buffers like
get_resolution_cmd and resp cause a DMA mapping failure since stack memory
is virtually mapped?

[Severity: High]
This is a pre-existing issue, but does the firmware update routine properly
validate an empty firmware file?

If a 0-byte firmware file is loaded, the modulo check in
elants_i2c_fw_update() evaluates to 0:

drivers/input/touchscreen/elants_i2c.c:elants_i2c_fw_update() {
...
	if (fw->size % ELAN_FW_PAGESIZE) {
		dev_err(&client->dev, "Invalid firmware format: %zu\n",
			fw->size);
...
}

This would allow the empty firmware to proceed. Later, when validating the
remark ID, wouldn't the size calculation underflow and read out of bounds?

drivers/input/touchscreen/elants_i2c.c:elants_i2c_validate_remark_id() {
...
	fw_remark_id = get_unaligned_le16(&fw->data[fw->size - 4]);
...
}

Even if validation is skipped, could the actual update routine read out of
bounds when it unconditionally sends the first page?

drivers/input/touchscreen/elants_i2c.c:elants_i2c_do_update_firmware() {
...
	error = elants_i2c_send(client, fw->data, ELAN_FW_PAGESIZE);
...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704121738.48071-1-acharyalaxman8848@gmail.com?part=1