[PATCH] HID: ft260: validate report size in raw_event handler

[PATCH] HID: ft260: validate report size in raw_event handler

[Sebastian Josue Alba Vives]

ft260_raw_event() casts the raw data buffer to a
ft260_i2c_input_report struct and accesses its fields without
validating the size parameter. Since __hid_input_report() invokes
the driver's raw_event callback before hid_report_raw_event()
performs its own report-size validation, a device sending a
truncated HID report can cause out-of-bounds heap reads in the
kernel.

In the I2C response path, xfer->length (data[1]) is used as the
length for a memcpy into dev->read_buf. While xfer->length is
checked against dev->read_len, there is no check that size is large
enough to actually contain xfer->length bytes of data starting at
offset 2. A malicious USB device could therefore cause an OOB read
from the kernel heap, with the result accessible from userspace via
the I2C read interface.

FT260 devices use 64-byte HID reports. Add a check at the top of
the handler to reject any report shorter than expected, and log a
warning to aid debugging.

Cc: stable@vger.kernel.org
Signed-off-by: Sebastian Josue Alba Vives <sebasjosue84@gmail.com>
---
 drivers/hid/hid-ft260.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 333341e80..7ca323992 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1068,6 +1068,12 @@ 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;
 
+	/* FT260 always sends 64-byte reports */
+	if (size < 64) {
+		hid_warn(hdev, "report too short: %d < 64\n", size);
+		return 0;
+	}
+
 	if (xfer->report >= FT260_I2C_REPORT_MIN &&
 	    xfer->report <= FT260_I2C_REPORT_MAX) {
 		ft260_dbg("i2c resp: rep %#02x len %d\n", xfer->report,
-- 
2.43.0

Re: [PATCH] HID: ft260: validate report size in raw_event handler

[Michael Zaidman]

Hi Sebastian,

Thanks for the patch. The report size validation gap in ft260_raw_event()
is a valid concern - the raw_event callback is indeed invoked before
hid_report_raw_event() validates the report size, so a truncated report
from a malicious or buggy device could cause OOB reads.

However, I have a couple of comments on the proposed fix:

Please use the existing FT260_REPORT_MAX_LENGTH macro instead of the
hardcoded 64.

More importantly, the size < 64 check alone is insufficient. It prevents
accessing struct fields in a truncated buffer, but does not guard against
a corrupted xfer->length field in an otherwise full-sized report.

Consider: a device sends a valid 64-byte report (passes the size check),
but with xfer->length set to, say, 100. The data payload starts at offset 2,
so only 62 bytes are available in the buffer. The existing check at line 1077
validates against the destination buffer (dev->read_len - dev->read_idx),
not the source. If read_len is large enough (e.g., 180), the check passes,
and the memcpy reads 100 bytes from a 62-byte region - a 38-byte OOB heap
read from the source side.

A more complete fix would validate xfer->length against the actual report size:

    struct ft260_i2c_input_report *xfer = (void *)data;
    if (size < FT260_REPORT_MAX_LENGTH) {
        hid_warn(hdev, "short report: %d\n", size);
        return 0;
    }
    if (xfer->length > size -
        offsetof(struct ft260_i2c_input_report, data)) {
        hid_warn(hdev, "payload %d exceeds report size %d\n",
             xfer->length, size);
        return 0;
    }
This catches both truncated reports and corrupted length fields.

Would you like to send a v2 addressing the above?

Thanks, Michael

On Tue, Mar 24, 2026 at 7:35 PM Sebastian Josue Alba Vives
<sebasjosue84@gmail.com> wrote:
>
> ft260_raw_event() casts the raw data buffer to a
> ft260_i2c_input_report struct and accesses its fields without
> validating the size parameter. Since __hid_input_report() invokes
> the driver's raw_event callback before hid_report_raw_event()
> performs its own report-size validation, a device sending a
> truncated HID report can cause out-of-bounds heap reads in the
> kernel.
>
> In the I2C response path, xfer->length (data[1]) is used as the
> length for a memcpy into dev->read_buf. While xfer->length is
> checked against dev->read_len, there is no check that size is large
> enough to actually contain xfer->length bytes of data starting at
> offset 2. A malicious USB device could therefore cause an OOB read
> from the kernel heap, with the result accessible from userspace via
> the I2C read interface.
>
> FT260 devices use 64-byte HID reports. Add a check at the top of
> the handler to reject any report shorter than expected, and log a
> warning to aid debugging.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Sebastian Josue Alba Vives <sebasjosue84@gmail.com>
> ---
>  drivers/hid/hid-ft260.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 333341e80..7ca323992 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
> @@ -1068,6 +1068,12 @@ 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;
>
> +       /* FT260 always sends 64-byte reports */
> +       if (size < 64) {
> +               hid_warn(hdev, "report too short: %d < 64\n", size);
> +               return 0;
> +       }
> +
>         if (xfer->report >= FT260_I2C_REPORT_MIN &&
>             xfer->report <= FT260_I2C_REPORT_MAX) {
>                 ft260_dbg("i2c resp: rep %#02x len %d\n", xfer->report,
> --
> 2.43.0
>

[PATCH v2] HID: ft260: validate report size and payload length in raw_event

[Sebastian Josue Alba Vives]

ft260_raw_event() casts the raw data buffer to a
ft260_i2c_input_report struct and accesses its fields without
validating the size parameter. Since __hid_input_report() invokes
the driver's raw_event callback before hid_report_raw_event()
performs its own report-size validation, a device sending a
truncated HID report can cause out-of-bounds heap reads.

Additionally, even with a full-sized report, a corrupted
xfer->length field can cause memcpy to read beyond the report
buffer. The existing check only validates against the destination
buffer size, not the source data available in the report.

Add two checks: reject reports shorter than FT260_REPORT_MAX_LENGTH,
and verify that xfer->length does not exceed the actual data
available in the report. Log warnings to aid debugging.

Cc: stable@vger.kernel.org
Signed-off-by: Sebastian Josue Alba Vives <sebasjosue84@gmail.com>
---
 drivers/hid/hid-ft260.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 333341e80..68008a423 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1068,6 +1068,17 @@ 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 < FT260_REPORT_MAX_LENGTH) {
+		hid_warn(hdev, "short report: %d\n", size);
+		return 0;
+	}
+
+	if (xfer->length > size - offsetof(struct ft260_i2c_input_report, data)) {
+		hid_warn(hdev, "payload %d exceeds report size %d\n",
+			 xfer->length, size);
+		return 0;
+	}
+
 	if (xfer->report >= FT260_I2C_REPORT_MIN &&
 	    xfer->report <= FT260_I2C_REPORT_MAX) {
 		ft260_dbg("i2c resp: rep %#02x len %d\n", xfer->report,
-- 
2.43.0

Re: [PATCH v2] HID: ft260: validate report size and payload length in raw_event

[Jiri Kosina]

On Tue, 24 Mar 2026, Sebastian Josue Alba Vives wrote:

> ft260_raw_event() casts the raw data buffer to a
> ft260_i2c_input_report struct and accesses its fields without
> validating the size parameter. Since __hid_input_report() invokes
> the driver's raw_event callback before hid_report_raw_event()
> performs its own report-size validation, a device sending a
> truncated HID report can cause out-of-bounds heap reads.
> 
> Additionally, even with a full-sized report, a corrupted
> xfer->length field can cause memcpy to read beyond the report
> buffer. The existing check only validates against the destination
> buffer size, not the source data available in the report.
> 
> Add two checks: reject reports shorter than FT260_REPORT_MAX_LENGTH,
> and verify that xfer->length does not exceed the actual data
> available in the report. Log warnings to aid debugging.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Sebastian Josue Alba Vives <sebasjosue84@gmail.com>
> ---
>  drivers/hid/hid-ft260.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 333341e80..68008a423 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
> @@ -1068,6 +1068,17 @@ 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 < FT260_REPORT_MAX_LENGTH) {
> +		hid_warn(hdev, "short report: %d\n", size);
> +		return 0;

Michael, can you please confirm whether the device can never legitimately 
send shorter than FT260_REPORT_MAX_LENGTH reports?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v2] HID: ft260: validate report size and payload length in raw_event

[Michael Zaidman]

On Thu, Apr 9, 2026 at 6:50 PM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Tue, 24 Mar 2026, Sebastian Josue Alba Vives wrote:
>
> > ft260_raw_event() casts the raw data buffer to a
> > ft260_i2c_input_report struct and accesses its fields without
> > validating the size parameter. Since __hid_input_report() invokes
> > the driver's raw_event callback before hid_report_raw_event()
> > performs its own report-size validation, a device sending a
> > truncated HID report can cause out-of-bounds heap reads.
> >
> > Additionally, even with a full-sized report, a corrupted
> > xfer->length field can cause memcpy to read beyond the report
> > buffer. The existing check only validates against the destination
> > buffer size, not the source data available in the report.
> >
> > Add two checks: reject reports shorter than FT260_REPORT_MAX_LENGTH,
> > and verify that xfer->length does not exceed the actual data
> > available in the report. Log warnings to aid debugging.
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Sebastian Josue Alba Vives <sebasjosue84@gmail.com>
> > ---
> >  drivers/hid/hid-ft260.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> > index 333341e80..68008a423 100644
> > --- a/drivers/hid/hid-ft260.c
> > +++ b/drivers/hid/hid-ft260.c
> > @@ -1068,6 +1068,17 @@ 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 < FT260_REPORT_MAX_LENGTH) {
> > +             hid_warn(hdev, "short report: %d\n", size);
> > +             return 0;
>
> Michael, can you please confirm whether the device can never legitimately
> send shorter than FT260_REPORT_MAX_LENGTH reports?
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>

Hi Jiri,

The FT260 uses different report IDs (0xD0 through 0xDE) for different payload
lengths, with each report ID defining a different report size in the HID
descriptor. So yes, the device can legitimately send reports shorter than
FT260_REPORT_MAX_LENGTH, and a blanket size < 64 check would break valid
short transfers.

Looking at __hid_input_report(), the HID core could validate size against
hid_compute_report_size(report) before the raw_event call - essentially
moving the check that hid_report_raw_event() already does to happen earlier.
That would handle truncated reports generically for all HID drivers. However,
such a change would affect all HID drivers and require broad testing, so that
is your call.

What the HID core cannot validate is driver-specific payload semantics. In
ft260, the I2C input report has a length field at byte 1 that indicates the
payload size, and the driver uses it as the memcpy length without checking
it against the actual report size or against the expected data capacity for
the specific report ID.

I will submit a per-driver fix with two checks, both essential:
First, a minimum size check before accessing any header fields. Currently,
the HID core does not validate report size before calling raw_event, so a
1-byte report would cause an OOB read just from accessing the length field
at byte 1. This check is necessary regardless of the second check.

Second, a validation of xfer->length against the expected data capacity for
the given report ID. Each I2C input report ID defines a specific data capacity
(report 0xD0 holds up to 4 bytes, 0xDE up to 60 bytes). A corrupted length
field exceeding this capacity would cause an OOB read from the source buffer
during memcpy, even if the report itself is full-sized. Only the driver knows
these per-report-ID limits.

I have the hardware to test this change. I will credit Sebastian with
Reported-by for identifying the issue.

Thanks,
Michael

Re: [PATCH v2] HID: ft260: validate report size and payload length in raw_event

[Jiri Kosina]

On Thu, 9 Apr 2026, Michael Zaidman wrote:

> The FT260 uses different report IDs (0xD0 through 0xDE) for different payload
> lengths, with each report ID defining a different report size in the HID
> descriptor. So yes, the device can legitimately send reports shorter than
> FT260_REPORT_MAX_LENGTH, and a blanket size < 64 check would break valid
> short transfers.

Perfect, thanks a lot for the detailed writeup! I was rather suspicious 
about the bold statement in the changelog.

Similarly to other Sebastián's fixes to various other drivers. This will 
need more thorough check.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v2] HID: ft260: validate report size and payload length in raw_event

[Michael Zaidman]

On Thu, Apr 9, 2026 at 9:29 PM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Thu, 9 Apr 2026, Michael Zaidman wrote:
>
> > The FT260 uses different report IDs (0xD0 through 0xDE) for different payload
> > lengths, with each report ID defining a different report size in the HID
> > descriptor. So yes, the device can legitimately send reports shorter than
> > FT260_REPORT_MAX_LENGTH, and a blanket size < 64 check would break valid
> > short transfers.
>
> Perfect, thanks a lot for the detailed writeup! I was rather suspicious
> about the bold statement in the changelog.
>
> Similarly to other Sebastián's fixes to various other drivers. This will
> need more thorough check.
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>

Hi Jiri,

Indeed. The original patch would have been easily caught by testing on actual
FT260 hardware - short transfers using report IDs 0xD0 through 0xD3 carry well
under 64 bytes and are part of normal I2C operation. A blanket size < 64 check
would break them immediately.

I'll submit a proper fix with per-report-ID capacity validation based on the
HID descriptor.

[PATCH] HID: ft260: validate i2c input report length

[Michael Zaidman]

Validate xfer->length against the actual HID report size in
ft260_raw_event() before using it as the memcpy length. A malicious
or malfunctioning device could send a report with xfer->length
exceeding the data actually present in the HID report, causing an
out-of-bounds read.

Each I2C data report ID (0xD0 through 0xDE) defines a different
report size in the HID descriptor, so the available payload varies
per report. Validate against the actual received report size rather
than a fixed maximum to avoid breaking valid short transfers.

Reported-by: Sebastián Josué Alba Vives <sebasjosue84@gmail.com>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
Tested on FT260 with I2C-attached EEPROM (24c02) behind PCA9548
mux switches. Verified short reads (1-4 bytes, report ID 0xD0)
and multi-report reads with debug tracing enabled, confirming
xfer->length is correctly validated against the HID report size
for each report ID.
---
 drivers/hid/hid-ft260.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 333341e80b0e..b31c43353249 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1070,8 +1070,15 @@ 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) {
-		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.25.1