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@xxxxxxxxx> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Sebastian Josue Alba Vives <sebasjosue84@xxxxxxxxx>
> ---
>  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
>