[PATCH v2] HID: mcp2221: validate raw report length

Pengpeng Hou <[email protected]>
Newsgroups org.kernel.vger.linux-i2c,org.kernel.vger.linux-input,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
The MCP2221A data sheet specifies 64-byte HID command and response
reports. mcp2221_raw_event() reads fixed response fields up to byte 55
and can copy a 60-byte I2C payload from byte 4 through byte 63 without
first checking the received size.

HID invokes raw_event() before generic report validation. A short report
can therefore be interpreted beyond its received length. Reject reports
shorter than the fixed protocol size before reading any response fields.

Complete the pending command with -EINVAL for a truncated report so that
a malformed response does not turn into an unrelated timeout.

Fixes: 67a95c21463d ("HID: mcp2221: add usb to i2c-smbus host bridge")
Signed-off-by: Pengpeng Hou <[email protected]>
---
Changes since v1: https://lore.kernel.org/all/[email protected]/
- validate the fixed 64-byte protocol size instead of the
  descriptor-derived report length
- use a named report-size constant and remove matching magic lengths
---
 drivers/hid/hid-mcp2221.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index e4ddd8e..c910f66 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -90,7 +90,8 @@ enum {
 	MCP2221_DIR_IN = 0x01,
 };
 
-#define MCP_NGPIO	4
+#define MCP2221_REPORT_SIZE	64
+#define MCP_NGPIO		4
 
 /* MCP GPIO set command layout */
 struct mcp_set_gpio {
@@ -126,7 +127,7 @@ struct mcp2221 {
 	struct completion wait_in_report;
 	struct delayed_work init_work;
 	u8 *rxbuf;
-	u8 txbuf[64];
+	u8 txbuf[MCP2221_REPORT_SIZE];
 	int rxbuf_idx;
 	int rxbuf_size;
 	int status;
@@ -639,11 +640,11 @@ static int mcp_gpio_read_sram(struct mcp2221 *mcp)
 {
 	int ret;
 
-	memset(mcp->txbuf, 0, 64);
+	memset(mcp->txbuf, 0, sizeof(mcp->txbuf));
 	mcp->txbuf[0] = MCP2221_GET_SRAM_SETTINGS;
 
 	mutex_lock(&mcp->lock);
-	ret = mcp_send_data_req_status(mcp, mcp->txbuf, 64);
+	ret = mcp_send_data_req_status(mcp, mcp->txbuf, sizeof(mcp->txbuf));
 	mutex_unlock(&mcp->lock);
 
 	return ret;
@@ -682,7 +683,7 @@ static int mcp2221_check_gpio_pinfunc(struct mcp2221 *mcp)
 	 * Set all bytes to 0, so Bit 7 is not set. The chip
 	 * only changes content of a register when bit 7 is set.
 	 */
-	memset(mcp->txbuf, 0, 64);
+	memset(mcp->txbuf, 0, sizeof(mcp->txbuf));
 	mcp->txbuf[0] = MCP2221_SET_SRAM_SETTINGS;
 
 	/*
@@ -703,7 +704,7 @@ static int mcp2221_check_gpio_pinfunc(struct mcp2221 *mcp)
 	}
 
 	mutex_lock(&mcp->lock);
-	ret = mcp_send_data_req_status(mcp, mcp->txbuf, 64);
+	ret = mcp_send_data_req_status(mcp, mcp->txbuf, sizeof(mcp->txbuf));
 	mutex_unlock(&mcp->lock);
 
 	return ret;
@@ -861,6 +862,12 @@ static int mcp2221_raw_event(struct hid_device *hdev,
 	u8 *buf;
 	struct mcp2221 *mcp = hid_get_drvdata(hdev);
 
+	if (size < MCP2221_REPORT_SIZE) {
+		mcp->status = -EINVAL;
+		complete(&mcp->wait_in_report);
+		return 1;
+	}
+
 	switch (data[0]) {
 
 	case MCP2221_I2C_WR_DATA:
-- 
2.50.1 (Apple Git-155)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.