| Message ID | 20250419200434.39661-1-purvayeshi550@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | input: iqs5xx: Fix incorrect argument passed to hex2bin | expand |
Hi Purva, On Sun, Apr 20, 2025 at 01:34:34AM +0530, Purva Yeshi wrote: > Fix Smatch-detected issue: > drivers/input/touchscreen/iqs5xx.c:747 iqs5xx_fw_file_parse() > error: hex2bin() 'rec->len' too small (2 vs 4) > > Fix incorrect second argument to hex2bin() when parsing firmware records. > > Pass a pointer to the ASCII hex data instead of the u8 record length to > hex2bin(), which expects a pointer, not an integer. The previous code > passed rec->len as the second argument, leading to undefined behavior > as hex2bin() attempted to read from an unintended memory address. > > Cast the entire rec structure to a const char * using a new pointer > rec_bytes. Skip the initial ':' character in the Intel HEX format by > passing rec_bytes + 1 to hex2bin(). This allows the function to decode > the 4-byte record header (length, address high, address low, and type) > correctly from its ASCII hex representation into binary form. > > Preserve the original code flow while ensuring correctness and resolving > the issue detected by Smatch. > > Signed-off-by: Purva Yeshi <purvayeshi550@gmail.com> > --- > drivers/input/touchscreen/iqs5xx.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/input/touchscreen/iqs5xx.c b/drivers/input/touchscreen/iqs5xx.c > index 4ebd7565ae6e..e8140a54685f 100644 > --- a/drivers/input/touchscreen/iqs5xx.c > +++ b/drivers/input/touchscreen/iqs5xx.c > @@ -744,7 +744,9 @@ static int iqs5xx_fw_file_parse(struct i2c_client *client, > break; > } > > - error = hex2bin(rec_hdr, rec->len, sizeof(rec_hdr)); > + const char *rec_bytes = (const char *)rec; > + > + error = hex2bin(rec_hdr, rec_bytes + 1, sizeof(rec_hdr)); > + > if (error) { > dev_err(&client->dev, "Invalid header at record %u\n", > rec_num); > -- > 2.34.1 > > Thank you for the patch! I appreciate your having investigated this warning, but this patch is a NAK. I can't speak to why Smatch thinks there is a problem here, but we can see from the definition of the struct 'iqs5xx_ihex_rec' that 'len' is indeed a pointer: char len[2]; I also checked with actual HW on latest kernel that FW updates still work just fine. The following line ensures we are looking at a valid memory location when locating the 'src' pointer: rec = (struct iqs5xx_ihex_rec *)(fw->data + pos); In case I have misunderstood, please let me know. Kind regards, Jeff LaBundy
diff --git a/drivers/input/touchscreen/iqs5xx.c b/drivers/input/touchscreen/iqs5xx.c index 4ebd7565ae6e..e8140a54685f 100644 --- a/drivers/input/touchscreen/iqs5xx.c +++ b/drivers/input/touchscreen/iqs5xx.c @@ -744,7 +744,9 @@ static int iqs5xx_fw_file_parse(struct i2c_client *client, break; } - error = hex2bin(rec_hdr, rec->len, sizeof(rec_hdr)); + const char *rec_bytes = (const char *)rec; + + error = hex2bin(rec_hdr, rec_bytes + 1, sizeof(rec_hdr)); + if (error) { dev_err(&client->dev, "Invalid header at record %u\n", rec_num);
Fix Smatch-detected issue: drivers/input/touchscreen/iqs5xx.c:747 iqs5xx_fw_file_parse() error: hex2bin() 'rec->len' too small (2 vs 4) Fix incorrect second argument to hex2bin() when parsing firmware records. Pass a pointer to the ASCII hex data instead of the u8 record length to hex2bin(), which expects a pointer, not an integer. The previous code passed rec->len as the second argument, leading to undefined behavior as hex2bin() attempted to read from an unintended memory address. Cast the entire rec structure to a const char * using a new pointer rec_bytes. Skip the initial ':' character in the Intel HEX format by passing rec_bytes + 1 to hex2bin(). This allows the function to decode the 4-byte record header (length, address high, address low, and type) correctly from its ASCII hex representation into binary form. Preserve the original code flow while ensuring correctness and resolving the issue detected by Smatch. Signed-off-by: Purva Yeshi <purvayeshi550@gmail.com> --- drivers/input/touchscreen/iqs5xx.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)