Skip to content

Commit 252ed1f

Browse files
Dan CarpenterJiri Kosina
authored andcommitted
HID: hid-goodix: Fix type promotion bug in goodix_hid_get_raw_report()
The issue is GOODIX_HID_PKG_LEN_SIZE is defined as sizeof(u16) which is type size_t. However, goodix_hid_check_ack_status() returns negative error codes or potentially a positive but invalid length which is too small. So when we compare "if ((response_data_len <= GOODIX_HID_PKG_LEN_SIZE)" then negative error codes are type promoted to size_t and counted as a positive large value and treated as valid. It would have been easy enough to add some casting to avoid the type promotion, however this patch takes a more thourough approach and moves the length check into goodix_hid_check_ack_status(). Now the function only return negative error codes or zero on success and the length pointer is never set to an invalid length. Fixes: 75e16c8 ("HID: hid-goodix: Add Goodix HID-over-SPI driver") Signed-off-by: Dan Carpenter <[email protected]> Reviewed-by: Dmitry Torokhov <[email protected]> Signed-off-by: Jiri Kosina <[email protected]>
1 parent 9184b17 commit 252ed1f

File tree

1 file changed

+15
-7
lines changed

1 file changed

+15
-7
lines changed

drivers/hid/hid-goodix-spi.c

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -335,11 +335,12 @@ static void goodix_hid_close(struct hid_device *hid)
335335
}
336336

337337
/* Return date length of response data */
338-
static int goodix_hid_check_ack_status(struct goodix_ts_data *ts)
338+
static int goodix_hid_check_ack_status(struct goodix_ts_data *ts, u32 *resp_len)
339339
{
340340
struct goodix_hid_report_header hdr;
341341
int retry = 20;
342342
int error;
343+
int len;
343344

344345
while (retry--) {
345346
/*
@@ -349,8 +350,15 @@ static int goodix_hid_check_ack_status(struct goodix_ts_data *ts)
349350
*/
350351
error = goodix_spi_read(ts, ts->hid_report_addr,
351352
&hdr, sizeof(hdr));
352-
if (!error && (hdr.flag & GOODIX_HID_ACK_READY_FLAG))
353-
return le16_to_cpu(hdr.size);
353+
if (!error && (hdr.flag & GOODIX_HID_ACK_READY_FLAG)) {
354+
len = le16_to_cpu(hdr.size);
355+
if (len < GOODIX_HID_PKG_LEN_SIZE) {
356+
dev_err(ts->dev, "hrd.size too short: %d", len);
357+
return -EINVAL;
358+
}
359+
*resp_len = len;
360+
return 0;
361+
}
354362

355363
/* Wait 10ms for another try */
356364
usleep_range(10000, 11000);
@@ -383,7 +391,7 @@ static int goodix_hid_get_raw_report(struct hid_device *hid,
383391
u16 cmd_register = le16_to_cpu(ts->hid_desc.cmd_register);
384392
u8 tmp_buf[GOODIX_HID_MAX_INBUF_SIZE];
385393
int tx_len = 0, args_len = 0;
386-
int response_data_len;
394+
u32 response_data_len;
387395
u8 args[3];
388396
int error;
389397

@@ -434,9 +442,9 @@ static int goodix_hid_get_raw_report(struct hid_device *hid,
434442
return 0;
435443

436444
/* Step2: check response data status */
437-
response_data_len = goodix_hid_check_ack_status(ts);
438-
if (response_data_len <= GOODIX_HID_PKG_LEN_SIZE)
439-
return -EINVAL;
445+
error = goodix_hid_check_ack_status(ts, &response_data_len);
446+
if (error)
447+
return error;
440448

441449
len = min(len, response_data_len - GOODIX_HID_PKG_LEN_SIZE);
442450
/* Step3: read response data(skip 2bytes of hid pkg length) */

0 commit comments

Comments
 (0)