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