Re: [PATCH] Input: elants_i2c - prevent divide-by-zero on invalid EKTH data
From: sashiko-bot@kernel.org
Date: 2026-07-04 12:28:35
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 [off-list ref]
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
quoted hunk ↗ jump to hunk
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