Commit 252ed1f7 authored by Dan Carpenter's avatar Dan Carpenter Committed by Jiri Kosina
Browse files

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: 75e16c8c ("HID: hid-goodix: Add Goodix HID-over-SPI driver")
Signed-off-by: default avatarDan Carpenter <dan.carpenter@linaro.org>
Reviewed-by: default avatarDmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: default avatarJiri Kosina <jkosina@suse.com>
parent 9184b17f

drivers/hid/hid-goodix-spi.c

+15 −7
Original line number Diff line number Diff line
@@ -335,11 +335,12 @@ static void goodix_hid_close(struct hid_device *hid) 
}



/* Return date length of response data */

static int goodix_hid_check_ack_status(struct goodix_ts_data *ts)

static int goodix_hid_check_ack_status(struct goodix_ts_data *ts, u32 *resp_len)

{

	struct goodix_hid_report_header hdr;

	int retry = 20;

	int error;

	int len;



	while (retry--) {

		/*
@@ -349,8 +350,15 @@ static int goodix_hid_check_ack_status(struct goodix_ts_data *ts) 
		 */

		error = goodix_spi_read(ts, ts->hid_report_addr,

					&hdr, sizeof(hdr));

		if (!error && (hdr.flag & GOODIX_HID_ACK_READY_FLAG))

			return le16_to_cpu(hdr.size);

		if (!error && (hdr.flag & GOODIX_HID_ACK_READY_FLAG)) {

			len = le16_to_cpu(hdr.size);

			if (len < GOODIX_HID_PKG_LEN_SIZE) {

				dev_err(ts->dev, "hrd.size too short: %d", len);

				return -EINVAL;

			}

			*resp_len = len;

			return 0;

		}



		/* Wait 10ms for another try */

		usleep_range(10000, 11000);
@@ -383,7 +391,7 @@ static int goodix_hid_get_raw_report(struct hid_device *hid, 
	u16 cmd_register = le16_to_cpu(ts->hid_desc.cmd_register);

	u8 tmp_buf[GOODIX_HID_MAX_INBUF_SIZE];

	int tx_len = 0, args_len = 0;

	int response_data_len;

	u32 response_data_len;

	u8 args[3];

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



	/* Step2: check response data status */

	response_data_len = goodix_hid_check_ack_status(ts);

	if (response_data_len <= GOODIX_HID_PKG_LEN_SIZE)

		return -EINVAL;

	error = goodix_hid_check_ack_status(ts, &response_data_len);

	if (error)

		return error;



	len = min(len, response_data_len - GOODIX_HID_PKG_LEN_SIZE);

	/* Step3: read response data(skip 2bytes of hid pkg length) */