From: Enrik Berkhan <Enrik.Berkhan@inka.de>
To: linux-input@vger.kernel.org
Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
Rishi Gupta <gupt21@gmail.com>,
Enrik Berkhan <Enrik.Berkhan@inka.de>
Subject: [PATCH v1 4/4] HID: mcp2221: avoid stale rxbuf pointer
Date: Mon, 26 Sep 2022 22:22:39 +0200 [thread overview]
Message-ID: <20220926202239.16379-5-Enrik.Berkhan@inka.de> (raw)
In-Reply-To: <20220926202239.16379-1-Enrik.Berkhan@inka.de>
In case the MCP2221 driver receives an unexpected read complete report
from the device, the data should not be copied to mcp->rxbuf. The
pointer might be NULL or even stale, having been set during an earlier
transaction.
Further, some bounds checking has been added.
Signed-off-by: Enrik Berkhan <Enrik.Berkhan@inka.de>
---
drivers/hid/hid-mcp2221.c | 44 +++++++++++++++++++++++++++++++--------
1 file changed, 35 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index d17839e09ebc..faccb3c03d33 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -94,6 +94,7 @@ struct mcp2221 {
u8 *rxbuf;
u8 txbuf[64];
int rxbuf_idx;
+ int rxbuf_len;
int status;
u8 cur_i2c_clk_div;
struct gpio_chip *gc;
@@ -286,15 +287,13 @@ static int mcp_i2c_smbus_read(struct mcp2221 *mcp,
total_len = smbus_len;
mcp->rxbuf = smbus_buf;
}
+ mcp->rxbuf_len = total_len;
+ mcp->rxbuf_idx = 0;
spin_unlock_bh(&mcp->raw_event_lock);
ret = mcp_send_data_req_status(mcp, mcp->txbuf, 4);
if (ret)
- return ret;
-
- spin_lock_bh(&mcp->raw_event_lock);
- mcp->rxbuf_idx = 0;
- spin_unlock_bh(&mcp->raw_event_lock);
+ goto out_invalidate_rxbuf;
do {
spin_lock_bh(&mcp->raw_event_lock);
@@ -304,15 +303,22 @@ static int mcp_i2c_smbus_read(struct mcp2221 *mcp,
ret = mcp_send_data_req_status(mcp, mcp->txbuf, 1);
if (ret)
- return ret;
+ goto out_invalidate_rxbuf;
ret = mcp_chk_last_cmd_status(mcp);
if (ret)
- return ret;
+ goto out_invalidate_rxbuf;
usleep_range(980, 1000);
} while (mcp->rxbuf_idx < total_len);
+out_invalidate_rxbuf:
+ spin_lock_bh(&mcp->raw_event_lock);
+ mcp->rxbuf = NULL;
+ mcp->rxbuf_len = 0;
+ mcp->rxbuf_idx = 0;
+ spin_unlock_bh(&mcp->raw_event_lock);
+
return ret;
}
@@ -500,9 +506,15 @@ static int mcp_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
spin_lock_bh(&mcp->raw_event_lock);
mcp->rxbuf_idx = 0;
mcp->rxbuf = data->block;
+ mcp->rxbuf_len = sizeof(data->block);
mcp->txbuf[0] = MCP2221_I2C_GET_DATA;
spin_unlock_bh(&mcp->raw_event_lock);
ret = mcp_send_data_req_status(mcp, mcp->txbuf, 1);
+ spin_lock_bh(&mcp->raw_event_lock);
+ mcp->rxbuf_idx = 0;
+ mcp->rxbuf = NULL;
+ mcp->rxbuf_len = 0;
+ spin_unlock_bh(&mcp->raw_event_lock);
if (ret)
goto exit;
} else {
@@ -525,9 +537,15 @@ static int mcp_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
spin_lock_bh(&mcp->raw_event_lock);
mcp->rxbuf_idx = 0;
mcp->rxbuf = data->block;
+ mcp->rxbuf_len = sizeof(data->block);
mcp->txbuf[0] = MCP2221_I2C_GET_DATA;
spin_unlock_bh(&mcp->raw_event_lock);
ret = mcp_send_data_req_status(mcp, mcp->txbuf, 1);
+ spin_lock_bh(&mcp->raw_event_lock);
+ mcp->rxbuf_idx = 0;
+ mcp->rxbuf = NULL;
+ mcp->rxbuf_len = 0;
+ spin_unlock_bh(&mcp->raw_event_lock);
if (ret)
goto exit;
} else {
@@ -756,6 +774,7 @@ static int mcp2221_raw_event(struct hid_device *hdev,
struct hid_report *report, u8 *data, int size)
{
u8 *buf;
+ int len;
struct mcp2221 *mcp = hid_get_drvdata(hdev);
spin_lock_bh(&mcp->raw_event_lock);
@@ -813,9 +832,15 @@ static int mcp2221_raw_event(struct hid_device *hdev,
break;
}
if (data[2] == MCP2221_I2C_READ_COMPL) {
+ if (mcp->rxbuf == NULL || mcp->rxbuf_idx >= mcp->rxbuf_len)
+ goto out; /* no complete() in this case */
+
buf = mcp->rxbuf;
- memcpy(&buf[mcp->rxbuf_idx], &data[4], data[3]);
- mcp->rxbuf_idx = mcp->rxbuf_idx + data[3];
+ len = data[3];
+ if (len > mcp->rxbuf_len - mcp->rxbuf_idx)
+ len = mcp->rxbuf_len - mcp->rxbuf_idx;
+ memcpy(&buf[mcp->rxbuf_idx], &data[4], len);
+ mcp->rxbuf_idx = mcp->rxbuf_idx + len;
mcp->status = 0;
break;
}
@@ -865,6 +890,7 @@ static int mcp2221_raw_event(struct hid_device *hdev,
complete(&mcp->wait_in_report);
}
+out:
spin_unlock_bh(&mcp->raw_event_lock);
return 1;
--
2.34.1
next prev parent reply other threads:[~2022-09-26 21:20 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-26 20:22 [RESEND PATCH v1 0/4] Fixes for the mcp2221 HID-to-I2C-bridge driver Enrik Berkhan
2022-09-26 20:22 ` [PATCH v1 1/4] HID: mcp2221: don't connect hidraw Enrik Berkhan
2022-09-26 20:22 ` [PATCH v1 2/4] HID: mcp2221: enable HID I/O during GPIO probe Enrik Berkhan
2022-09-26 20:22 ` [PATCH v1 3/4] HID: mcp2221: protect shared data with spin lock Enrik Berkhan
2022-09-26 20:22 ` Enrik Berkhan [this message]
2022-11-03 22:27 ` [PATCH v2 0/3] Fixes for the mcp2221 HID-to-I2C-bridge driver Enrik Berkhan
2022-11-03 22:27 ` [PATCH v2 1/3] HID: mcp2221: don't connect hidraw Enrik Berkhan
2022-12-19 14:11 ` Benjamin Tissoires
2022-11-03 22:27 ` [PATCH v2 2/3] HID: mcp2221: enable HID I/O during GPIO probe Enrik Berkhan
2022-11-03 22:27 ` [PATCH v2 3/3] HID: mcp2221: avoid stale rxbuf pointer Enrik Berkhan
-- strict thread matches above, loose matches on Subject: below --
2022-08-30 18:02 [PATCH v1 0/4] Fixes for the mcp2221 HID-to-I2C-bridge driver Enrik Berkhan
2022-08-30 18:02 ` [PATCH v1 4/4] HID: mcp2221: avoid stale rxbuf pointer Enrik Berkhan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220926202239.16379-5-Enrik.Berkhan@inka.de \
--to=enrik.berkhan@inka.de \
--cc=gupt21@gmail.com \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.