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

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

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