Re: [PATCH v2 11/12] HID: hid-core: sanitize user input in 'new_id_store'

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Restricting `driver_data` to strictly match an existing `id_table` entry breaks backward compatibility and defeats the purpose of the `new_id` interface.
--

commit 613c086c5db811baec4d7b28a4e3757a62048b24
Author: Pawel Zalewski (The Capable Hub) <pzalewski@thegoodpenguin.co.uk>
Subject: HID: hid-core: sanitize user input in 'new_id_store'

This commit modifies the new_id sysfs interface for HID drivers. It adds a
check to ensure that any provided driver_data value strictly matches an
existing entry within the driver's id_table, preventing arbitrary values
from being passed.

> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 41a79e43c82b3..e060e5ec4ea62 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2647,6 +2647,8 @@ static ssize_t new_id_store(struct device_driver *drv, const char *buf,
>  		size_t count)
>  {
>  	struct hid_driver *hdrv = to_hid_driver(drv);
> +	const struct hid_device_id *ids = hdrv->id_table;
> +
>  	struct hid_dynid *dynid;
>  	__u32 bus, vendor, product;
>  	unsigned long driver_data = 0;
> @@ -2657,6 +2659,22 @@ static ssize_t new_id_store(struct device_driver *drv, const char *buf,
>  	if (ret < 3)
>  		return -EINVAL;
>  
> +	/* Only accept driver_data values that match an
> +	 * existing id_table entry
> +	 */
> +	if (ids) {
> +		ret = -EINVAL;
> +		while (ids->bus) {
> +			if (driver_data == ids->driver_data) {
> +				ret = 0;
> +				break;
> +			}
> +			ids++;
> +		}
> +		if (ret)	/* No match */
> +			return ret;
> +	}
> +

[Severity: High]
Does this change break backward compatibility and the intended purpose of the
new_id interface?

Previously, users could write just the bus, vendor, and product to new_id,
leaving driver_data to default to 0. With this patch, if a driver's id_table
only contains entries with non-zero driver_data (such as quirk bitmasks in
hid-apple), wouldn't the sysfs write now fail with -EINVAL?

Additionally, does this prevent developers from testing unsupported hardware
by passing custom quirk bitmasks (or passing 0 to test without quirks) via
the driver_data field?

>  	dynid = kzalloc_obj(*dynid);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610-mod-devicetable-hid_device_id-v2-0-a1d7473ccd9c@thegoodpenguin.co.uk?part=11