* [PATCH 6.1.y 2/7] HID: ft260: improve i2c large reads performance
2026-08-26 17:48 ` [PATCH 6.1.y 1/7] HID: ft260: improve i2c write performance Sasha Levin
@ 2026-08-26 17:48 ` Sasha Levin
2026-08-26 17:48 ` [PATCH 6.1.y 3/7] HID: ft260: skip unexpected HID input reports Sasha Levin
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2026-08-26 17:48 UTC (permalink / raw)
To: stable; +Cc: Michael Zaidman, Enrik Berkhan, Jiri Kosina, Sasha Levin
From: Michael Zaidman <michael.zaidman@gmail.com>
[ Upstream commit 54410c14800ad652c77e5c6fc5c17baad6e42cb6 ]
The patch increases the read buffer size to 180 bytes. It reduces
the number of ft260_i2c_read() calls by three, improving the big
reads performance.
$ sudo i2ctransfer -y -f 13 w2@0x51 0x0 0x0 r180
Before:
[ +4.071878] ft260_i2c_write_read: off 0x0 rlen 180 wlen 2
[ +0.000005] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0
[ +0.001097] ft260_xfer_status: bus_status 0x41, clock 100
[ +0.000175] ft260_xfer_status: bus_status 0x40, clock 100
[ +0.000004] ft260_i2c_read: rep 0xc2 addr 0x51 len 180 rlen 60 flag 0x3
[ +0.008579] ft260_raw_event: i2c resp: rep 0xde len 60
[ +0.000208] ft260_xfer_status: bus_status 0x40, clock 100
[ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 120 rlen 60 flag 0x0
[ +0.008794] ft260_raw_event: i2c resp: rep 0xde len 60
[ +0.000181] ft260_xfer_status: bus_status 0x40, clock 100
[ +0.000002] ft260_i2c_read: rep 0xc2 addr 0x51 len 60 rlen 60 flag 0x4
[ +0.008817] ft260_raw_event: i2c resp: rep 0xde len 60
[ +0.000223] ft260_xfer_status: bus_status 0x20, clock 100
After:
[ +11.611642] ft260_i2c_write_read: off 0x0 rlen 180 wlen 2
[ +0.000005] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0
[ +0.008001] ft260_xfer_status: bus_status 0x20, clock 100
[ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 180 rlen 180 flag 0x7
[ +0.008994] ft260_raw_event: i2c resp: rep 0xde len 60
[ +0.007987] ft260_raw_event: i2c resp: rep 0xde len 60
[ +0.007992] ft260_raw_event: i2c resp: rep 0xde len 60
[ +0.000206] ft260_xfer_status: bus_status 0x20, clock 100
Suggested-by: Enrik Berkhan <Enrik.Berkhan@inka.de>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Stable-dep-of: bf3e39df3a39 ("HID: ft260: fix stack-use-after-return write in I2C read race")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ft260.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 539d311bb9b9d..854876b16880d 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -30,12 +30,19 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages");
#define FT260_REPORT_MAX_LENGTH (64)
#define FT260_I2C_DATA_REPORT_ID(len) (FT260_I2C_REPORT_MIN + (len - 1) / 4)
+
/*
- * The input report format assigns 62 bytes for the data payload, but ft260
- * returns 60 and 2 in two separate transactions. To minimize transfer time
- * in reading chunks mode, set the maximum read payload length to 60 bytes.
- */
-#define FT260_RD_DATA_MAX (60)
+ * The ft260 input report format defines 62 bytes for the data payload, but
+ * when requested 62 bytes, the controller returns 60 and 2 in separate input
+ * reports. To achieve better performance with the multi-report read data
+ * transfers, we set the maximum read payload length to a multiple of 60.
+ * With a 100 kHz I2C clock, one 240 bytes read takes about 1/27 second,
+ * which is excessive; On the other hand, some higher layer drivers like at24
+ * or optoe limit the i2c reads to 128 bytes. To not block other drivers out
+ * of I2C for potentially troublesome amounts of time, we select the maximum
+ * read payload length to be 180 bytes.
+*/
+#define FT260_RD_DATA_MAX (180)
#define FT260_WR_DATA_MAX (60)
/*
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 6.1.y 3/7] HID: ft260: skip unexpected HID input reports
2026-08-26 17:48 ` [PATCH 6.1.y 1/7] HID: ft260: improve i2c write performance Sasha Levin
2026-08-26 17:48 ` [PATCH 6.1.y 2/7] HID: ft260: improve i2c large reads performance Sasha Levin
@ 2026-08-26 17:48 ` Sasha Levin
2026-08-26 17:48 ` [PATCH 6.1.y 4/7] HID: ft260: wake up device from power saving mode Sasha Levin
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2026-08-26 17:48 UTC (permalink / raw)
To: stable; +Cc: Michael Zaidman, Enrik Berkhan, Jiri Kosina, Sasha Levin
From: Michael Zaidman <michael.zaidman@gmail.com>
[ Upstream commit b7121e3c04440cc2af9cabbabb24efd23741294a ]
The FT260 is not supposed to generate unexpected HID reports. However,
in theory, the unsolicited HID Input reports can be issued by a specially
crafted malicious USB device masquerading as FT260 when the attacker has
physical access to the USB port. In this case, the read_buf pointer points
to the final data portion of the previous I2C Read transfer, and the memcpy
invoked in the ft260_raw_event() will try copying the content of the
unexpected report into the wrong location.
This commit sets the Read buffer pointer to NULL on the I2C Read
transaction completion and checks it in the ft260_raw_event() to detect
and skip the unsolicited Input report.
Reported-by: Enrik Berkhan <Enrik.Berkhan@inka.de>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Stable-dep-of: bf3e39df3a39 ("HID: ft260: fix stack-use-after-return write in I2C read race")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ft260.c | 41 +++++++++++++++++++++++++++--------------
1 file changed, 27 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 854876b16880d..4c6c7dfdd6a71 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -471,17 +471,13 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
struct ft260_i2c_read_request_report rep;
struct hid_device *hdev = dev->hdev;
int timeout;
- int ret;
+ int ret = 0;
if (len > FT260_RD_DATA_MAX) {
hid_err(hdev, "%s: unsupported rd len: %d\n", __func__, len);
return -EINVAL;
}
- dev->read_idx = 0;
- dev->read_buf = data;
- dev->read_len = len;
-
rep.report = FT260_I2C_READ_REQ;
rep.length = cpu_to_le16(len);
rep.address = addr;
@@ -492,25 +488,36 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
reinit_completion(&dev->wait);
+ dev->read_idx = 0;
+ dev->read_buf = data;
+ dev->read_len = len;
+
ret = ft260_hid_output_report(hdev, (u8 *)&rep, sizeof(rep));
if (ret < 0) {
hid_err(hdev, "%s: failed to start transaction, ret %d\n",
__func__, ret);
- return ret;
+ goto ft260_i2c_read_exit;
}
timeout = msecs_to_jiffies(5000);
if (!wait_for_completion_timeout(&dev->wait, timeout)) {
+ ret = -ETIMEDOUT;
ft260_i2c_reset(hdev);
- return -ETIMEDOUT;
+ goto ft260_i2c_read_exit;
}
+ dev->read_buf = NULL;
+
ret = ft260_xfer_status(dev);
- if (ret == 0)
- return 0;
+ if (ret < 0) {
+ ret = -EIO;
+ ft260_i2c_reset(hdev);
+ goto ft260_i2c_read_exit;
+ }
- ft260_i2c_reset(hdev);
- return -EIO;
+ft260_i2c_read_exit:
+ dev->read_buf = NULL;
+ return ret;
}
/*
@@ -1033,6 +1040,13 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
ft260_dbg("i2c resp: rep %#02x len %d\n", xfer->report,
xfer->length);
+ if ((dev->read_buf == NULL) ||
+ (xfer->length > dev->read_len - dev->read_idx)) {
+ hid_err(hdev, "unexpected report %#02x, length %d\n",
+ xfer->report, xfer->length);
+ return -1;
+ }
+
memcpy(&dev->read_buf[dev->read_idx], &xfer->data,
xfer->length);
dev->read_idx += xfer->length;
@@ -1041,10 +1055,9 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
complete(&dev->wait);
} else {
- hid_err(hdev, "unknown report: %#02x\n", xfer->report);
- return 0;
+ hid_err(hdev, "unhandled report %#02x\n", xfer->report);
}
- return 1;
+ return 0;
}
static struct hid_driver ft260_driver = {
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 6.1.y 4/7] HID: ft260: wake up device from power saving mode
2026-08-26 17:48 ` [PATCH 6.1.y 1/7] HID: ft260: improve i2c write performance Sasha Levin
2026-08-26 17:48 ` [PATCH 6.1.y 2/7] HID: ft260: improve i2c large reads performance Sasha Levin
2026-08-26 17:48 ` [PATCH 6.1.y 3/7] HID: ft260: skip unexpected HID input reports Sasha Levin
@ 2026-08-26 17:48 ` Sasha Levin
2026-08-26 17:48 ` [PATCH 6.1.y 5/7] HID: ft260: missed NACK from busy device Sasha Levin
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2026-08-26 17:48 UTC (permalink / raw)
To: stable; +Cc: Michael Zaidman, Enrik Berkhan, Jiri Kosina, Sasha Levin
From: Michael Zaidman <michael.zaidman@gmail.com>
[ Upstream commit 4b3da6853a619a952e8caf2e8393264dd42ffa27 ]
The FT260 can enter a power saving mode after being idle for longer
than 5 seconds.
When being woken up from power saving mode by an I2C write request,
a possible NACK is not correctly reported by the controller. As a
workaround, the driver will issue an I2C status report two times in
ft260_xfer_status() after the chip has been idle for more than 5s.
Co-developed-by: Enrik Berkhan <Enrik.Berkhan@inka.de>
Signed-off-by: Enrik Berkhan <Enrik.Berkhan@inka.de>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Stable-dep-of: bf3e39df3a39 ("HID: ft260: fix stack-use-after-return write in I2C read race")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ft260.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 4c6c7dfdd6a71..3c4ca0016659b 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -31,6 +31,8 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages");
#define FT260_REPORT_MAX_LENGTH (64)
#define FT260_I2C_DATA_REPORT_ID(len) (FT260_I2C_REPORT_MIN + (len - 1) / 4)
+#define FT260_WAKEUP_NEEDED_AFTER_MS (4800) /* 5s minus 200ms margin */
+
/*
* The ft260 input report format defines 62 bytes for the data payload, but
* when requested 62 bytes, the controller returns 60 and 2 in separate input
@@ -237,6 +239,7 @@ struct ft260_device {
struct completion wait;
struct mutex lock;
u8 write_buf[FT260_REPORT_MAX_LENGTH];
+ unsigned long need_wakeup_at;
u8 *read_buf;
u16 read_idx;
u16 read_len;
@@ -306,6 +309,20 @@ static int ft260_xfer_status(struct ft260_device *dev)
struct ft260_get_i2c_status_report report;
int ret;
+ if (time_is_before_jiffies(dev->need_wakeup_at)) {
+ ret = ft260_hid_feature_report_get(hdev, FT260_I2C_STATUS,
+ (u8 *)&report, sizeof(report));
+ if (unlikely(ret < 0)) {
+ hid_err(hdev, "failed to retrieve status: %d, no wakeup\n",
+ ret);
+ } else {
+ dev->need_wakeup_at = jiffies +
+ msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS);
+ ft260_dbg("bus_status %#02x, wakeup\n",
+ report.bus_status);
+ }
+ }
+
ret = ft260_hid_feature_report_get(hdev, FT260_I2C_STATUS,
(u8 *)&report, sizeof(report));
if (unlikely(ret < 0)) {
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 6.1.y 5/7] HID: ft260: missed NACK from busy device
2026-08-26 17:48 ` [PATCH 6.1.y 1/7] HID: ft260: improve i2c write performance Sasha Levin
` (2 preceding siblings ...)
2026-08-26 17:48 ` [PATCH 6.1.y 4/7] HID: ft260: wake up device from power saving mode Sasha Levin
@ 2026-08-26 17:48 ` Sasha Levin
2026-08-26 17:48 ` [PATCH 6.1.y 6/7] HID: ft260: validate i2c input report length Sasha Levin
2026-08-26 17:48 ` [PATCH 6.1.y 7/7] HID: ft260: fix stack-use-after-return write in I2C read race Sasha Levin
5 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2026-08-26 17:48 UTC (permalink / raw)
To: stable; +Cc: Michael Zaidman, Germain Hebert, Jiri Kosina, Sasha Levin
From: Michael Zaidman <michael.zaidman@gmail.com>
[ Upstream commit 5afac727defa0b2a3dffb2abd5fb5f594b98d217 ]
When writing into a slow device like an EEPROM chip, the
controller may exit the busy state before the device releases
the bus. In this case, the ft260_xfer_status returns success
before the data transfer completion.
The patch fixes it by returning from the ft260_xfer_status()
with the "-EAGAIN" on both controller and bus busy status when
appropriate.
It does not apply to the i2c combined transactions when after
the write IO, the controller keeps the bus busy until the read
IO and then between reading IOs to ensure an atomic operation.
Co-developed-by: Germain Hebert <germain.hebert@ca.abb.com>
Signed-off-by: Germain Hebert <germain.hebert@ca.abb.com>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Stable-dep-of: bf3e39df3a39 ("HID: ft260: fix stack-use-after-return write in I2C read race")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ft260.c | 27 ++++++++++++++++++++++-----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 3c4ca0016659b..30d7f014df05c 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -303,7 +303,7 @@ static int ft260_i2c_reset(struct hid_device *hdev)
return ret;
}
-static int ft260_xfer_status(struct ft260_device *dev)
+static int ft260_xfer_status(struct ft260_device *dev, u8 bus_busy)
{
struct hid_device *hdev = dev->hdev;
struct ft260_get_i2c_status_report report;
@@ -334,7 +334,7 @@ static int ft260_xfer_status(struct ft260_device *dev)
ft260_dbg("bus_status %#02x, clock %u\n", report.bus_status,
dev->clock);
- if (report.bus_status & FT260_I2C_STATUS_CTRL_BUSY)
+ if (report.bus_status & (FT260_I2C_STATUS_CTRL_BUSY | bus_busy))
return -EAGAIN;
if (report.bus_status & FT260_I2C_STATUS_BUS_BUSY)
@@ -379,8 +379,11 @@ static int ft260_hid_output_report(struct hid_device *hdev, u8 *data,
static int ft260_hid_output_report_check_status(struct ft260_device *dev,
u8 *data, int len)
{
+ u8 bus_busy;
int ret, usec, try = 100;
struct hid_device *hdev = dev->hdev;
+ struct ft260_i2c_write_request_report *rep =
+ (struct ft260_i2c_write_request_report *)data;
ret = ft260_hid_output_report(hdev, data, len);
if (ret < 0) {
@@ -398,8 +401,18 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev,
ft260_dbg("wait %d usec, len %d\n", usec, len);
}
+ /*
+ * Do not check the busy bit for combined transactions
+ * since the controller keeps the bus busy between writing
+ * and reading IOs to ensure an atomic operation.
+ */
+ if (rep->flag == FT260_FLAG_START)
+ bus_busy = 0;
+ else
+ bus_busy = FT260_I2C_STATUS_BUS_BUSY;
+
do {
- ret = ft260_xfer_status(dev);
+ ret = ft260_xfer_status(dev, bus_busy);
if (ret != -EAGAIN)
break;
} while (--try);
@@ -487,6 +500,7 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
{
struct ft260_i2c_read_request_report rep;
struct hid_device *hdev = dev->hdev;
+ u8 bus_busy = 0;
int timeout;
int ret = 0;
@@ -525,7 +539,10 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
dev->read_buf = NULL;
- ret = ft260_xfer_status(dev);
+ if (flag & FT260_FLAG_STOP)
+ bus_busy = FT260_I2C_STATUS_BUS_BUSY;
+
+ ret = ft260_xfer_status(dev, bus_busy);
if (ret < 0) {
ret = -EIO;
ft260_i2c_reset(hdev);
@@ -1004,7 +1021,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
mutex_init(&dev->lock);
init_completion(&dev->wait);
- ret = ft260_xfer_status(dev);
+ ret = ft260_xfer_status(dev, FT260_I2C_STATUS_BUS_BUSY);
if (ret)
ft260_i2c_reset(hdev);
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 6.1.y 6/7] HID: ft260: validate i2c input report length
2026-08-26 17:48 ` [PATCH 6.1.y 1/7] HID: ft260: improve i2c write performance Sasha Levin
` (3 preceding siblings ...)
2026-08-26 17:48 ` [PATCH 6.1.y 5/7] HID: ft260: missed NACK from busy device Sasha Levin
@ 2026-08-26 17:48 ` Sasha Levin
2026-08-26 17:48 ` [PATCH 6.1.y 7/7] HID: ft260: fix stack-use-after-return write in I2C read race Sasha Levin
5 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2026-08-26 17:48 UTC (permalink / raw)
To: stable
Cc: Michael Zaidman, Sebastián Josué Alba Vives,
Jiri Kosina, Sasha Levin
From: Michael Zaidman <michael.zaidman@gmail.com>
[ Upstream commit 80c4bbb2b38513e9c3d84805fa61a0ee16d79c45 ]
Add two checks to ft260_raw_event() to prevent out-of-bounds reads
from malicious or malfunctioning devices:
First, reject reports shorter than the 2-byte header (report ID +
length fields). Without this, even accessing xfer->length on a
1-byte report is an OOB read.
Second, validate xfer->length against the actual data capacity of
the received HID report. Each I2C data report ID (0xD0 through
0xDE) defines a different report size in the HID descriptor, so the
available payload varies per report. A corrupted length field could
cause memcpy to read beyond the report buffer.
Reported-by: Sebastián Josué Alba Vives <sebasjosue84@gmail.com>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Stable-dep-of: bf3e39df3a39 ("HID: ft260: fix stack-use-after-return write in I2C read race")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ft260.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 30d7f014df05c..2b8db682fa4a9 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1069,10 +1069,22 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
struct ft260_device *dev = hid_get_drvdata(hdev);
struct ft260_i2c_input_report *xfer = (void *)data;
+ if (size < offsetof(struct ft260_i2c_input_report, data)) {
+ hid_err(hdev, "short report %d\n", size);
+ return -1;
+ }
+
if (xfer->report >= FT260_I2C_REPORT_MIN &&
xfer->report <= FT260_I2C_REPORT_MAX) {
- ft260_dbg("i2c resp: rep %#02x len %d\n", xfer->report,
- xfer->length);
+ ft260_dbg("i2c resp: rep %#02x len %d size %d\n",
+ xfer->report, xfer->length, size);
+
+ if (xfer->length > size -
+ offsetof(struct ft260_i2c_input_report, data)) {
+ hid_err(hdev, "report %#02x: length %d exceeds HID report size\n",
+ xfer->report, xfer->length);
+ return -1;
+ }
if ((dev->read_buf == NULL) ||
(xfer->length > dev->read_len - dev->read_idx)) {
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 6.1.y 7/7] HID: ft260: fix stack-use-after-return write in I2C read race
2026-08-26 17:48 ` [PATCH 6.1.y 1/7] HID: ft260: improve i2c write performance Sasha Levin
` (4 preceding siblings ...)
2026-08-26 17:48 ` [PATCH 6.1.y 6/7] HID: ft260: validate i2c input report length Sasha Levin
@ 2026-08-26 17:48 ` Sasha Levin
5 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2026-08-26 17:48 UTC (permalink / raw)
To: stable; +Cc: Raman Varabets, Michael Zaidman, Jiri Kosina, Sasha Levin
From: Raman Varabets <kernel-linux-20260610-80b7ab08@raman.v1.sg>
[ Upstream commit bf3e39df3a397fd82967a31d17c4e02c7feab221 ]
ft260_i2c_read() points dev->read_buf at a caller-supplied buffer
(often an on-stack variable), arms a completion and waits up to five
seconds for the device to return the data. The HID input callback
ft260_raw_event() runs in the input/IRQ path, independent of the
dev->lock mutex held by the read path, and copies the device-supplied
payload into dev->read_buf after a plain NULL check.
These two paths share read_buf, read_idx and read_len with no
serialization. If the device delays its response until the read
times out, ft260_i2c_read() resets the controller, clears read_buf
and returns, unwinding the stack frame the buffer lived in. A
response that arrives at that moment lets ft260_raw_event() pass the
NULL check and then memcpy() the device-controlled payload into the
now-freed stack location, a bounded but attacker-influenced
stack-use-after-return write triggerable by malicious or
malfunctioning hardware.
Add a dedicated spinlock that serializes every access to read_buf,
read_idx and read_len. ft260_raw_event() now holds it across the
NULL check, the memcpy and the index update, while the read path
takes it when arming and when clearing the buffer, so the teardown
can no longer slip between the check and the copy.
Fixes: 6a82582d9fa4 ("HID: ft260: add usb hid to i2c host bridge driver")
Cc: stable@vger.kernel.org
Signed-off-by: Raman Varabets <kernel-linux-20260610-80b7ab08@raman.v1.sg>
Reviewed-by: Michael Zaidman <michael.zaidman@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
[ adapted per-iteration locking to the flat single-shot `ft260_i2c_read()` body (no chunking loop), dropping one indentation level. ]
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ft260.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 2b8db682fa4a9..c9e5e65fa032c 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -240,6 +240,8 @@ struct ft260_device {
struct mutex lock;
u8 write_buf[FT260_REPORT_MAX_LENGTH];
unsigned long need_wakeup_at;
+ /* Protects read_buf, read_idx and read_len against ft260_raw_event() */
+ spinlock_t read_lock;
u8 *read_buf;
u16 read_idx;
u16 read_len;
@@ -500,6 +502,7 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
{
struct ft260_i2c_read_request_report rep;
struct hid_device *hdev = dev->hdev;
+ unsigned long irqflags;
u8 bus_busy = 0;
int timeout;
int ret = 0;
@@ -519,9 +522,11 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
reinit_completion(&dev->wait);
+ spin_lock_irqsave(&dev->read_lock, irqflags);
dev->read_idx = 0;
dev->read_buf = data;
dev->read_len = len;
+ spin_unlock_irqrestore(&dev->read_lock, irqflags);
ret = ft260_hid_output_report(hdev, (u8 *)&rep, sizeof(rep));
if (ret < 0) {
@@ -537,7 +542,9 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
goto ft260_i2c_read_exit;
}
+ spin_lock_irqsave(&dev->read_lock, irqflags);
dev->read_buf = NULL;
+ spin_unlock_irqrestore(&dev->read_lock, irqflags);
if (flag & FT260_FLAG_STOP)
bus_busy = FT260_I2C_STATUS_BUS_BUSY;
@@ -550,7 +557,9 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
}
ft260_i2c_read_exit:
+ spin_lock_irqsave(&dev->read_lock, irqflags);
dev->read_buf = NULL;
+ spin_unlock_irqrestore(&dev->read_lock, irqflags);
return ret;
}
@@ -1019,6 +1028,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
((struct hidraw *)hdev->hidraw)->minor);
mutex_init(&dev->lock);
+ spin_lock_init(&dev->read_lock);
init_completion(&dev->wait);
ret = ft260_xfer_status(dev, FT260_I2C_STATUS_BUS_BUSY);
@@ -1068,6 +1078,7 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
{
struct ft260_device *dev = hid_get_drvdata(hdev);
struct ft260_i2c_input_report *xfer = (void *)data;
+ unsigned long irqflags;
if (size < offsetof(struct ft260_i2c_input_report, data)) {
hid_err(hdev, "short report %d\n", size);
@@ -1076,6 +1087,8 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
if (xfer->report >= FT260_I2C_REPORT_MIN &&
xfer->report <= FT260_I2C_REPORT_MAX) {
+ bool complete_read;
+
ft260_dbg("i2c resp: rep %#02x len %d size %d\n",
xfer->report, xfer->length, size);
@@ -1086,8 +1099,15 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
return -1;
}
+ /*
+ * Hold read_lock so a timed-out ft260_i2c_read() cannot
+ * clear read_buf between the NULL check and the memcpy.
+ */
+ spin_lock_irqsave(&dev->read_lock, irqflags);
+
if ((dev->read_buf == NULL) ||
(xfer->length > dev->read_len - dev->read_idx)) {
+ spin_unlock_irqrestore(&dev->read_lock, irqflags);
hid_err(hdev, "unexpected report %#02x, length %d\n",
xfer->report, xfer->length);
return -1;
@@ -1096,8 +1116,11 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
memcpy(&dev->read_buf[dev->read_idx], &xfer->data,
xfer->length);
dev->read_idx += xfer->length;
+ complete_read = dev->read_idx == dev->read_len;
+
+ spin_unlock_irqrestore(&dev->read_lock, irqflags);
- if (dev->read_idx == dev->read_len)
+ if (complete_read)
complete(&dev->wait);
} else {
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread