Re: [PATCH] Input: raydium_i2c_ts - validate report parameters
flat view
From: Muhammad Bilal <hidden>
Date: 2026-09-27 17:30:16
Also in:
lkml
Hi Pooyan, Nice fix, the size validation and resize-on-update handling both look correct. Two quick questions: 1. Does the IRQ handler independently clamp the per-packet touch count against RM_MAX_TOUCH_NUM, or does it trust the count once these static sizes pass? 2. raydium_i2c_query_ts_info() can rerun after a firmware update. Is the devm_krealloc() of ts->report_data protected from a racing IRQ handler? Reviewed-by: Muhammad Bilal <redacted> Thanks, Muhammad On Sun, Sep 27, 2026 at 4:44 PM Pooyan Azad [off-list ref] wrote:
quoted hunk ↗ jump to hunk
The controller supplies packet and per-contact sizes used to allocate and parse touch reports. The driver trusts these values without validation. A packet size smaller than the two-byte checksum makes report_size wrap, allowing the IRQ handler to read beyond the report buffer. A zero or undersized contact size can cause a divide by zero or make the contact parser read beyond a record. Validate both sizes before publishing them, and reject reports that describe more contacts than the input device has slots. Allocate the report buffer once valid main firmware information is available, and resize it if a firmware update changes the packet size. This also handles devices that probe in bootloader mode, where the packet size is not known yet. Finally, return main firmware query failures from initialization so probe and firmware update do not continue with invalid report parameters. Keep bootloader HWID query failures non-fatal so the recovery interface remains available. Fixes: 48a2b783483b ("Input: add Raydium I2C touchscreen driver") Link: https://lore.kernel.org/all/20260728135127.48971-1-meatuni001@gmail.com/ (local) Cc: stable@vger.kernel.org Signed-off-by: Pooyan Azad <redacted> --- This partially overlaps Muhammad Bilal's earlier patch linked above. That patch propagates both main and bootloader query errors. This version keeps bootloader HWID errors non-fatal so recovery remains available, and moves report-buffer allocation into the main query path so size changes can be handled safely. Compile-tested with: make O=/tmp/raydium-build W=1 -j$(nproc) \ drivers/input/touchscreen/raydium_i2c_ts.o drivers/input/touchscreen/raydium_i2c_ts.c | 64 +++++++++++++--------- 1 file changed, 39 insertions(+), 25 deletions(-)diff --git a/drivers/input/touchscreen/raydium_i2c_ts.c b/drivers/input/touchscreen/raydium_i2c_ts.c index 0256055abcef..1d7f53d0b9fe 100644 --- a/drivers/input/touchscreen/raydium_i2c_ts.c +++ b/drivers/input/touchscreen/raydium_i2c_ts.c@@ -64,6 +64,7 @@ #define RM_CONTACT_PRESSURE_POS 5 #define RM_CONTACT_WIDTH_X_POS 6 #define RM_CONTACT_WIDTH_Y_POS 7 +#define RM_MIN_CONTACT_SIZE (RM_CONTACT_WIDTH_Y_POS + 1) /* Bootloader relative info */ #define RM_BL_WRT_CMD_SIZE 3 /* bl flash wrt cmd size */@@ -331,7 +332,10 @@ static int raydium_i2c_query_ts_info(struct raydium_data *ts) { struct i2c_client *client = ts->client; struct raydium_data_info data_info; + struct raydium_info info; __le32 query_bank_addr; + u8 *report_data; + u8 report_size; int error, retry_cnt;@@ -341,26 +345,22 @@ static int raydium_i2c_query_ts_info(struct raydium_data *ts) if (error) continue; - /* - * Warn user if we already allocated memory for reports and - * then the size changed (due to firmware update?) and keep - * old size instead. - */ - if (ts->report_data && ts->pkg_size != data_info.pkg_size) { - dev_warn(&client->dev, - "report size changes, was: %d, new: %d\n", - ts->pkg_size, data_info.pkg_size); - } else { - ts->pkg_size = data_info.pkg_size; - ts->report_size = ts->pkg_size - RM_PACKET_CRC_SIZE; + if (data_info.pkg_size < RM_PACKET_CRC_SIZE) { + dev_err(&client->dev, + "invalid report sizes: packet=%u contact=%u\n", + data_info.pkg_size, data_info.tp_info_size); + return -EINVAL; } - ts->contact_size = data_info.tp_info_size; - ts->data_bank_addr = le32_to_cpu(data_info.data_bank_addr); - - dev_dbg(&client->dev, - "data_bank_addr: %#08x, report_size: %d, contact_size: %d\n", - ts->data_bank_addr, ts->report_size, ts->contact_size); + report_size = data_info.pkg_size - RM_PACKET_CRC_SIZE; + if (data_info.tp_info_size < RM_MIN_CONTACT_SIZE || + data_info.tp_info_size > report_size || + report_size / data_info.tp_info_size > RM_MAX_TOUCH_NUM) { + dev_err(&client->dev, + "invalid report sizes: packet=%u contact=%u\n", + data_info.pkg_size, data_info.tp_info_size); + return -EINVAL; + } error = raydium_i2c_read(client, RM_CMD_QUERY_BANK, &query_bank_addr,@@ -369,10 +369,29 @@ static int raydium_i2c_query_ts_info(struct raydium_data *ts) continue; error = raydium_i2c_read(client, le32_to_cpu(query_bank_addr), - &ts->info, sizeof(ts->info)); + &info, sizeof(info)); if (error) continue; + if (!ts->report_data || ts->pkg_size != data_info.pkg_size) { + report_data = devm_krealloc(&client->dev, ts->report_data, + data_info.pkg_size, GFP_KERNEL); + if (!report_data) + return -ENOMEM; + + ts->report_data = report_data; + } + + ts->pkg_size = data_info.pkg_size; + ts->report_size = report_size; + ts->contact_size = data_info.tp_info_size; + ts->data_bank_addr = le32_to_cpu(data_info.data_bank_addr); + ts->info = info; + + dev_dbg(&client->dev, + "data_bank_addr: %#08x, report_size: %d, contact_size: %d\n", + ts->data_bank_addr, ts->report_size, ts->contact_size); + return 0; }@@ -428,7 +447,7 @@ static int raydium_i2c_initialize(struct raydium_data *ts) if (ts->boot_mode == RAYDIUM_TS_BLDR) raydium_i2c_query_ts_bootloader_info(ts); else - raydium_i2c_query_ts_info(ts); + error = raydium_i2c_query_ts_info(ts); return error; }@@ -1116,11 +1135,6 @@ static int raydium_i2c_probe(struct i2c_client *client) return error; } - ts->report_data = devm_kmalloc(&client->dev, - ts->pkg_size, GFP_KERNEL); - if (!ts->report_data) - return -ENOMEM; - ts->input = devm_input_allocate_device(&client->dev); if (!ts->input) { dev_err(&client->dev, "Failed to allocate input device\n"); --2.43.0