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

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

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