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

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

[Lee Jones]

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

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

[Lee Jones]

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

Re: [PATCH 2/2] HID: multitouch: Check to ensure report responses match the request

[Benjamin Tissoires]

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
> 

Re: (subset) [PATCH 2/2] HID: multitouch: Check to ensure report responses match the request

[Benjamin Tissoires]

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>

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

[Benjamin Tissoires]

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
>