Re: [RFC v3 1/2] HID: core: Mitigate potential OOB by removing bogus memset()

[Benjamin Tissoires]

On Mar 09 2026, Lee Jones wrote:
> The memset() in hid_report_raw_event() has the good intention of
> clearing out bogus data by zeroing the area from the end of the incoming
> data string to the assumed end of the buffer.  However, as we have
> previously seen, doing so can easily result in OOB reads and writes in
> the subsequent thread of execution.
> 
> The current suggestion from one of the HID maintainers is to remove the
> memset() and simply return if the incoming event buffer size is not
> large enough to fill the associated report.
> 
> Suggested-by Benjamin Tissoires <bentiss@xxxxxxxxxx>
> Signed-off-by: Lee Jones <lee@xxxxxxxxxx>
> ---
> v2 -> v3: Instead of removing the check entirely, show a warning and return early
> 
> RFC query: Is it better to return SUCCESS or -EINVAL?

I'd say -EINVAL is better.

Also, one brain fart I had today was that this works in 99% of the cases
because the transport layer allocates a big enough buffer.

So that means that if this function, and hid_input_report() both have
the allocated size as parameter, we could make a smarter decision and
do the memset when we have enough space.

This would require an API change, and probably to keep things under
control adding a new API with the buffer_allocated_size that would be
used by uhid, usbhid, i2c-hid, and others when we can.

But let's go with the current state of the patch, and say sorry if we
break some devices later on. (patch is currently in my queue, no need to
resend it).

Cheers,
Benjamin

> 
>  drivers/hid/hid-core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index a5b3a8ca2fcb..da9231ca42bc 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2057,9 +2057,9 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
>  		rsize = max_buffer_size;
>  
>  	if (csize < rsize) {
> -		dbg_hid("report %d is too short, (%d < %d)\n", report->id,
> -				csize, rsize);
> -		memset(cdata + csize, 0, rsize - csize);
> +		hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %d)\n",
> +				     report->id, rsize, csize);
> +		goto out;
>  	}
>  
>  	if ((hid->claimed & HID_CLAIMED_HIDDEV) && hid->hiddev_report_event)
> -- 
> 2.53.0.473.g4a7958ca14-goog
> 
>