[PATCH] HID: vivaldi: Validate Report ID and length in feature mapping

[PATCH] HID: vivaldi: Validate Report ID and length in feature mapping

[Shashikirana H K via B4 Relay]

From: Shashikirana H K <shashikiranah@chromium.org>

vivaldi_feature_mapping() currently passes the descriptor-declared
report_len as the received data size to hid_report_raw_event() instead
of the actual number of bytes returned by hid_hw_raw_request(). It also
does not verify that the returned Report ID matches the requested
report->id, or handle a zero-byte return before decrementing the length
for unnumbered reports.

Validate that hid_hw_raw_request() returns at least one byte and that
the returned Report ID matches report->id, and pass the actual received
byte count (ret) to hid_report_raw_event().

Fixes: 33bbe04a15f2 ("HID: google: extract Vivaldi hid feature mapping for use in hid-hammer")
Cc: Tzung-Bi Shih <tzungbi@chromium.org>
Cc: Sean O'Brien <seobrien@chromium.org>
Signed-off-by: Shashikirana H K <shashikiranah@chromium.org>
---
Change-Id: I3cf03bd2457f44c6ed47835f3d603f235f5993de
---
 drivers/hid/hid-vivaldi-common.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-vivaldi-common.c b/drivers/hid/hid-vivaldi-common.c
index b12bb5cc091aa..ea2c661e623d1 100644
--- a/drivers/hid/hid-vivaldi-common.c
+++ b/drivers/hid/hid-vivaldi-common.c
@@ -69,12 +69,18 @@ void vivaldi_feature_mapping(struct hid_device *hdev,
 	ret = hid_hw_raw_request(hdev, report->id, report_data,
 				 report_len, HID_FEATURE_REPORT,
 				 HID_REQ_GET_REPORT);
-	if (ret < 0) {
+	if (ret <= 0) {
 		dev_warn(&hdev->dev, "failed to fetch feature %d\n",
 			 field->report->id);
 		goto out;
 	}
 
+	if (report->id && report_data[0] != report->id) {
+		dev_warn(&hdev->dev, "report ID mismatch: expected %d got %d\n",
+			 report->id, report_data[0]);
+		goto out;
+	}
+
 	if (!report->id) {
 		/*
 		 * Undo the damage from hid_hw_raw_request() for unnumbered
@@ -82,10 +88,11 @@ void vivaldi_feature_mapping(struct hid_device *hdev,
 		 */
 		report_data++;
 		report_len--;
+		ret--;
 	}
 
 	ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, report_data,
-				   report_len, report_len, 0);
+				   report_len, ret, 0);
 	if (ret) {
 		dev_warn(&hdev->dev, "failed to report feature %d\n",
 			 field->report->id);

---
base-commit: 388b607d107c07aaade04c7f22f344cab6bdccd3
change-id: 20261006-upstream-vivaldi-fix-950b29c98286

Best regards,
-- 
Shashikirana H K <shashikiranah@chromium.org>

Re: [PATCH] HID: vivaldi: Validate Report ID and length in feature mapping

[Dmitry Torokhov]

Hi Shashikirana,

On Tue, Oct 06, 2026 at 03:06:59PM +0000, Shashikirana H K via B4 Relay wrote:
> From: Shashikirana H K <shashikiranah@chromium.org>
> 
> vivaldi_feature_mapping() currently passes the descriptor-declared
> report_len as the received data size to hid_report_raw_event() instead
> of the actual number of bytes returned by hid_hw_raw_request(). It also
> does not verify that the returned Report ID matches the requested
> report->id, or handle a zero-byte return before decrementing the length
> for unnumbered reports.
> 
> Validate that hid_hw_raw_request() returns at least one byte and that
> the returned Report ID matches report->id, and pass the actual received
> byte count (ret) to hid_report_raw_event().
> 
> Fixes: 33bbe04a15f2 ("HID: google: extract Vivaldi hid feature mapping for use in hid-hammer")
> Cc: Tzung-Bi Shih <tzungbi@chromium.org>
> Cc: Sean O'Brien <seobrien@chromium.org>
> Signed-off-by: Shashikirana H K <shashikiranah@chromium.org>
> ---
> Change-Id: I3cf03bd2457f44c6ed47835f3d603f235f5993de
> ---
>  drivers/hid/hid-vivaldi-common.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-vivaldi-common.c b/drivers/hid/hid-vivaldi-common.c
> index b12bb5cc091aa..ea2c661e623d1 100644
> --- a/drivers/hid/hid-vivaldi-common.c
> +++ b/drivers/hid/hid-vivaldi-common.c
> @@ -69,12 +69,18 @@ void vivaldi_feature_mapping(struct hid_device *hdev,
>  	ret = hid_hw_raw_request(hdev, report->id, report_data,
>  				 report_len, HID_FEATURE_REPORT,
>  				 HID_REQ_GET_REPORT);
> -	if (ret < 0) {
> +	if (ret <= 0) {
>  		dev_warn(&hdev->dev, "failed to fetch feature %d\n",
>  			 field->report->id);
>  		goto out;
>  	}
>  
> +	if (report->id && report_data[0] != report->id) {

I do not think checking whether report is numbered is needed. While
transports handle numbered and non-numbered reports differently here I
believe we always have report number (0 for unnumbered) in first byte.

Also, shouldn't this check be in hid core if it is needed?

> +		dev_warn(&hdev->dev, "report ID mismatch: expected %d got %d\n",
> +			 report->id, report_data[0]);
> +		goto out;
> +	}
> +
>  	if (!report->id) {
>  		/*
>  		 * Undo the damage from hid_hw_raw_request() for unnumbered
> @@ -82,10 +88,11 @@ void vivaldi_feature_mapping(struct hid_device *hdev,
>  		 */
>  		report_data++;
>  		report_len--;
> +		ret--;
>  	}
>  
>  	ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, report_data,
> -				   report_len, report_len, 0);
> +				   report_len, ret, 0);

Devices typically do not return short reports. Could you please tell me
what prompted this change?

Thanks.

-- 
Dmitry

Re: [PATCH] HID: vivaldi: Validate Report ID and length in feature mapping

[Shashikirana H K]

Hi Dmitry,

Thank you for reviewing the patch!

> Devices typically do not return short reports. Could you please tell me
> what prompted this change?

This came up while testing with synthetic/uhid devices that return
malformed GET_REPORT responses (either fewer bytes than report_len or a
different Report ID in report_data[0] than requested). When that happens,
vivaldi_feature_mapping() still passes the buffer to hid_report_raw_event(),
which then looks up a different report via data[0] or processes unreceived
bytes from the buffer.

Since legitimate Vivaldi keyboards always return the full feature report,
rejecting short reads outright with `if (ret != report_len)` (similar to
hammer_get_folded_state() in hid-google-hammer.c) would be even cleaner
than passing `ret` to hid_report_raw_event().

> I do not think checking whether report is numbered is needed. While
> transports handle numbered and non-numbered reports differently here I
> believe we always have report number (0 for unnumbered) in first byte.
> Also, shouldn't this check be in hid core if it is needed?

Agreed on both points:
1. Dropping `report->id &&` and simply checking `report_data[0] != report->id`
   (like mt_get_feature() in hid-multitouch.c) works for both numbered and
   unnumbered reports since transports place 0 in byte 0 for unnumbered
   reports.
2. Checking `buf[0] != reportnum` on HID_REQ_GET_REPORT directly in hid-core
   (e.g. in __hid_hw_raw_request()) would protect all HID drivers at once.

Jiri, Benjamin, Dmitry -- would you prefer a v2 that checks
`ret != report_len` and `report_data[0] != report->id` in
vivaldi_feature_mapping(), or moving the `buf[0] != reportnum` check for
HID_REQ_GET_REPORT into hid-core?

Thanks,
Shashikirana

Re: [PATCH] HID: vivaldi: Validate Report ID and length in feature mapping

[Dmitry Torokhov]

Hi Shashikirana,

On Sun, Oct 11, 2026 at 05:29:22PM +0000, Shashikirana H K wrote:
> > I do not think checking whether report is numbered is needed. While
> > transports handle numbered and non-numbered reports differently here I
> > believe we always have report number (0 for unnumbered) in first byte.
> > Also, shouldn't this check be in hid core if it is needed?
>
> Agreed on both points:
> 1. Dropping `report->id &&` and simply checking `report_data[0] != report->id`
>    (like mt_get_feature() in hid-multitouch.c) works for both numbered and
>    unnumbered reports since transports place 0 in byte 0 for unnumbered
>    reports.
> 2. Checking `buf[0] != reportnum` on HID_REQ_GET_REPORT directly in hid-core
>    (e.g. in __hid_hw_raw_request()) would protect all HID drivers at once.
>
> Jiri, Benjamin, Dmitry -- would you prefer a v2 that checks
> `ret != report_len` and `report_data[0] != report->id` in
> vivaldi_feature_mapping(), or moving the `buf[0] != reportnum` check for
> HID_REQ_GET_REPORT into hid-core?

Yes, let's harden hid-core directly by rejecting descriptors that mix
numbered and unnumbered reports in hid_register_report() and validating
buf[0] against reportnum on HID_REQ_GET_REPORT transfers in
__hid_hw_raw_request(). I'll prepare a patch series addressing this and
a few other issues.

In vivaldi_feature_mapping() itself, though, there is no reason to go
through hid_report_raw_event() at all: we already have field, report,
and usage in hand, and nothing else consumes field->value[] for this
report. If we verify ret != report_len and extract the value directly
via hid_field_extract(), similarly to hammer_get_folded_state(), we
avoid re-looking up the report by ID altogether and do not even need a
separate report ID check there:

	u8 *buf __free(kfree) = hid_alloc_report_buf(report, GFP_KERNEL);
	if (!buf)
		return;

	report_len = hid_report_len(report) + !report->id;
	ret = hid_hw_raw_request(hdev, report->id, buf, report_len,
				 HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
	if (ret != report_len) {
		dev_warn(&hdev->dev, "failed to fetch feature %d: %d\n",
			 report->id, ret);
		return;
	}

	offset = field->report_offset + usage->usage_index * field->report_size;
	data->function_row_physmap[fn_key - MIN_FN_ROW_KEY] =
		hid_field_extract(hdev, buf + 1, offset, field->report_size);

Thanks.

-- 
Dmitry