Re: [PATCH v3 7/8] 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:
- [Critical] UAPI/ABI regression in sysfs `new_id` interface due to strict `driver_data` validation.
--

--- Patch [7]: [PATCH v3 7/8] HID: hid-core: sanitize user input in 'new_id_store' ---
commit af3e08832a7fc0ca5b1869fbb2a41b16e167f98f
Author: Pawel Zalewski (The Capable Hub) <pzalewski@thegoodpenguin.co.uk>

HID: hid-core: sanitize user input in 'new_id_store'

This commit restricts the driver_data field in the new_id sysfs interface
to only accept values that match an existing entry in the driver's id_table.
It aims to prevent overriding predefined values within the driver module
with arbitrary entries from the command line.

> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c

[ ... ]

> @@ -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: Critical]
Does this loop break the existing userspace ABI for drivers that do not have
an explicit driver_data == 0 entry?

The sysfs new_id interface historically allows users to bind devices dynamically
by writing 3 arguments (bus, vendor, product), which defaults driver_data to 0.
Because this loop terminates when ids->bus == 0, it skips the sentinel entry. 
If a driver's id_table lacks an explicit entry with driver_data == 0, writing
a 3-argument string will now be rejected with -EINVAL.

Can this be adjusted so it doesn't break existing scripts or udev rules that
rely on the default driver_data behavior, or prevent users from testing custom
combinations of quirks?

> +	}
> +
>  	dynid = kzalloc_obj(*dynid);
>  	if (!dynid)
>  		return -ENOMEM;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-mod-devicetable-hid_device_id-v3-0-980896ca283e@thegoodpenguin.co.uk?part=7