Re: [PATCH] HID: vivaldi: avoid redundant feature report reads per usage

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] hid: feature report fetch bypassed if usage_index == 0 fails early validation
--

commit 31203c2d78508ee9933c6f629cd9827611acfce0
Author: Zhengping Jiang <jiangzp@google.com>

HID: vivaldi: avoid redundant feature report reads per usage

This commit optimizes the vivaldi_feature_mapping() initialization loop by
fetching the hardware feature report only on the first usage. Subsequent
usages in the same field skip the hardware read and reuse the previously
fetched field values.

> diff --git a/drivers/hid/hid-vivaldi-common.c b/drivers/hid/hid-vivaldi-common.c
> index b12bb5cc091aa..69df3a64eb91a 100644
> --- a/drivers/hid/hid-vivaldi-common.c
> +++ b/drivers/hid/hid-vivaldi-common.c
> @@ -49,6 +49,12 @@ void vivaldi_feature_mapping(struct hid_device *hdev,
>  	if (fn_key > data->num_function_row_keys)
>  		data->num_function_row_keys = fn_key;
>  
> +	if (usage->usage_index) {
> +		data->function_row_physmap[fn_key - MIN_FN_ROW_KEY] =
> +			field->value[usage->usage_index];
> +		return;
> +	}
> +

[Severity: Medium]
Does this fast-path correctly handle cases where the first usage in the
field is bypassed by early validation checks?

Looking at the start of vivaldi_feature_mapping(), there are early returns
for non-ordinal pages and out-of-bounds fn_key values:

void vivaldi_feature_mapping(...)
{
	...
	if (field->logical != HID_USAGE_FN_ROW_PHYSMAP ||
	    (usage->hid & HID_USAGE_PAGE) != HID_UP_ORDINAL)
		return;

	fn_key = usage->hid & HID_USAGE;
	if (fn_key < MIN_FN_ROW_KEY || fn_key > MAX_FN_ROW_KEY)
		return;

If the usage at usage_index == 0 triggers one of these early returns, the
hardware fetch at the end of the function is bypassed.

When the loop processes a valid subsequent usage, such as usage_index == 1,
it will hit this new fast-path. Because the hardware fetch was never
performed, will this copy the initial zeroed memory from field->value into
the physical map and misconfigure the function keys?

>  	report_data = buf = hid_alloc_report_buf(report, GFP_KERNEL);
>  	if (!report_data)
>  		return;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012212902.3491839-1-jiangzp@google.com?part=1