Re: [PATCH] Input: raydium_i2c_ts - validate report parameters

[Muhammad Bilal]

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 <meatuni001@xxxxxxxxx>

Thanks,
Muhammad


On Sun, Sep 27, 2026 at 4:44 PM Pooyan Azad <pooyan.azadparvar@xxxxxxxxx> wrote:
>
> 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@xxxxxxxxx/
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Pooyan Azad <pooyan.azadparvar@xxxxxxxxx>
> ---
> 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