All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Tissoires <bentiss@kernel.org>
To: Lee Jones <lee@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>,
	linux-input@vger.kernel.org,  linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] HID: core: Mitigate potential OOB by removing bogus memset()
Date: Mon, 2 Mar 2026 15:29:53 +0100	[thread overview]
Message-ID: <aaWYS3QhaCPLBfzH@beelink> (raw)
In-Reply-To: <20260227163031.1166560-1-lee@kernel.org>

On Feb 27 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
> recently seen, the size of the report buffer isn't always correct which
> can culminate in OOB writes.
> 
> The current suggestion from one of the HID maintainers is to remove the
> attempt completely.  The subsequent handling should be able to simply
> use the data size provided to prevent any potential overruns.
> 
> Suggested-by Benjamin Tissoires <bentiss@kernel.org>
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
>  drivers/hid/hid-core.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index a5b3a8ca2fcb..1d51042e4b1f 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2056,12 +2056,6 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
>  	else if (rsize > max_buffer_size)
>  		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);
> -	}
> -

Simply removing this check is not enough.

later we have a call to `hid_process_report(hid, report, cdata,
interrupt);`` which loses the size information and which will make an
OOB read while calling hid_input_fetch_field().

I think we should drop here the processing with a warning (maybe rate
limnited), and hope for the best.

Cheers,
Benjamin

>  	if ((hid->claimed & HID_CLAIMED_HIDDEV) && hid->hiddev_report_event)
>  		hid->hiddev_report_event(hid, report);
>  	if (hid->claimed & HID_CLAIMED_HIDRAW) {
> -- 
> 2.53.0.473.g4a7958ca14-goog
> 

      parent reply	other threads:[~2026-03-02 14:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-27 16:30 [PATCH 1/2] HID: core: Mitigate potential OOB by removing bogus memset() Lee Jones
2026-02-27 16:30 ` [PATCH 2/2] HID: multitouch: Check to ensure report responses match the request Lee Jones
2026-03-02 14:31   ` Benjamin Tissoires
2026-03-17 14:03   ` (subset) " Benjamin Tissoires
2026-03-02 14:29 ` Benjamin Tissoires [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aaWYS3QhaCPLBfzH@beelink \
    --to=bentiss@kernel.org \
    --cc=jikos@kernel.org \
    --cc=lee@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.