public inbox for linux-input@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] HID: core: Mitigate potential OOB by removing bogus memset()
@ 2026-02-27 16:30 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:29 ` [PATCH 1/2] HID: core: Mitigate potential OOB by removing bogus memset() Benjamin Tissoires
  0 siblings, 2 replies; 5+ messages in thread
From: Lee Jones @ 2026-02-27 16:30 UTC (permalink / raw)
  To: lee, Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel

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);
-	}
-
 	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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] HID: multitouch: Check to ensure report responses match the request
  2026-02-27 16:30 [PATCH 1/2] HID: core: Mitigate potential OOB by removing bogus memset() Lee Jones
@ 2026-02-27 16:30 ` Lee Jones
  2026-03-02 14:31   ` Benjamin Tissoires
  2026-03-17 14:03   ` (subset) " Benjamin Tissoires
  2026-03-02 14:29 ` [PATCH 1/2] HID: core: Mitigate potential OOB by removing bogus memset() Benjamin Tissoires
  1 sibling, 2 replies; 5+ messages in thread
From: Lee Jones @ 2026-02-27 16:30 UTC (permalink / raw)
  To: lee, Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel

It is possible for a malicious (or clumsy) device to respond to a
specific report's feature request using a completely different report
ID.  This can cause confusion in the HID core resulting in nasty
side-effects such as OOB writes.

Add a check to ensure that the report ID in the response, matches the
one that was requested.  If it doesn't, omit reporting the raw event and
return early.

Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/hid/hid-multitouch.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index b1c3ef129058..834c1a334887 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -499,12 +499,19 @@ static void mt_get_feature(struct hid_device *hdev, struct hid_report *report)
 		dev_warn(&hdev->dev, "failed to fetch feature %d\n",
 			 report->id);
 	} else {
+		/* The report ID in the request and the response should match */
+		if (report->id != buf[0]) {
+			hid_err(hdev, "Returned feature report did not match the request\n");
+			goto free;
+		}
+
 		ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, buf,
 					   size, 0);
 		if (ret)
 			dev_warn(&hdev->dev, "failed to report feature\n");
 	}
 
+free:
 	kfree(buf);
 }
 
-- 
2.53.0.473.g4a7958ca14-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] HID: core: Mitigate potential OOB by removing bogus memset()
  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:29 ` Benjamin Tissoires
  1 sibling, 0 replies; 5+ messages in thread
From: Benjamin Tissoires @ 2026-03-02 14:29 UTC (permalink / raw)
  To: Lee Jones; +Cc: Jiri Kosina, linux-input, linux-kernel

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
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] HID: multitouch: Check to ensure report responses match the request
  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
  1 sibling, 0 replies; 5+ messages in thread
From: Benjamin Tissoires @ 2026-03-02 14:31 UTC (permalink / raw)
  To: Lee Jones; +Cc: Jiri Kosina, linux-input, linux-kernel


On Feb 27 2026, Lee Jones wrote:
> It is possible for a malicious (or clumsy) device to respond to a
> specific report's feature request using a completely different report
> ID.  This can cause confusion in the HID core resulting in nasty
> side-effects such as OOB writes.
> 
> Add a check to ensure that the report ID in the response, matches the
> one that was requested.  If it doesn't, omit reporting the raw event and
> return early.
> 
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
>  drivers/hid/hid-multitouch.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index b1c3ef129058..834c1a334887 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -499,12 +499,19 @@ static void mt_get_feature(struct hid_device *hdev, struct hid_report *report)
>  		dev_warn(&hdev->dev, "failed to fetch feature %d\n",
>  			 report->id);
>  	} else {
> +		/* The report ID in the request and the response should match */
> +		if (report->id != buf[0]) {
> +			hid_err(hdev, "Returned feature report did not match the request\n");
> +			goto free;
> +		}
> +

AFAICT, hid-vivaldi-common.c is doing the same calls to
hid_report_raw_event() in vivaldi_feature_mapping() and would suffer
from the same bug.

It would be nice if we could have the matching fix.

Cheers,
Benjamin

>  		ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, buf,
>  					   size, 0);
>  		if (ret)
>  			dev_warn(&hdev->dev, "failed to report feature\n");
>  	}
>  
> +free:
>  	kfree(buf);
>  }
>  
> -- 
> 2.53.0.473.g4a7958ca14-goog
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: (subset) [PATCH 2/2] HID: multitouch: Check to ensure report responses match the request
  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   ` Benjamin Tissoires
  1 sibling, 0 replies; 5+ messages in thread
From: Benjamin Tissoires @ 2026-03-17 14:03 UTC (permalink / raw)
  To: Jiri Kosina, linux-input, linux-kernel, Lee Jones


On Fri, 27 Feb 2026 16:30:25 +0000, Lee Jones wrote:
> It is possible for a malicious (or clumsy) device to respond to a
> specific report's feature request using a completely different report
> ID.  This can cause confusion in the HID core resulting in nasty
> side-effects such as OOB writes.
> 
> Add a check to ensure that the report ID in the response, matches the
> one that was requested.  If it doesn't, omit reporting the raw event and
> return early.
> 
> [...]

Applied, thanks!

[2/2] HID: multitouch: Check to ensure report responses match the request
      commit: e716edafedad4952fe3a4a273d2e039a84e8681a

Best regards,
-- 
Benjamin Tissoires <bentiss@kernel.org>


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-03-17 14:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 ` [PATCH 1/2] HID: core: Mitigate potential OOB by removing bogus memset() Benjamin Tissoires

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox