Re: [PATCH v9 07/12] HID: asus: add joysticks anti-deadzone configuration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] sysfs: unconditional range attribute exposure for unsupported features
--

commit 82e5ab56095fcef34b4dde73bbddbf61750ae376
Author: Denis Benato <denis.benato@linux.dev>

HID: asus: add joysticks anti-deadzone configuration

ROG Ally devices allow configuring the anti-deadzone parameter for
the resistive joysticks devices as over time those develops drift,
therefore allow userspace to configure the anti-deadzone by exposing
relevant sysfs attributes.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index b692c180cd706..0d1ead3dcb6ef 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> @@ -1382,6 +1382,197 @@ static struct device_attribute dev_attr_right_joystick_outer_threshold =
[ ... ]
> +static ssize_t left_joystick_anti_deadzone_show(struct device *dev, struct device_attribute *attr,
> +						char *buf)
> +{
[ ... ]
> +	if (!cfg->anti_deadzone_support) {
> +		hid_dbg(hdev, "Anti-deadzone not supported on this device\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return sysfs_emit(buf, "%u\n", cfg->left_anti_deadzone);
> +}
[ ... ]
> +static ssize_t left_joystick_anti_deadzone_range_show(struct device *dev,
> +						      struct device_attribute *attr,
> +						      char *buf)
> +{
> +	return sysfs_emit(buf, "0 100\n");
> +}

[Severity: Low]
Does this code expose range values unconditionally, even when the underlying
feature is not supported?

Looking at ally_config_create(), the sysfs group is created if either
user_cal_support or anti_deadzone_support is present. If a device has
user_cal_support but not anti_deadzone_support, left_joystick_anti_deadzone_show()
correctly rejects accesses and returns -EOPNOTSUPP.

However, this static range handler completely lacks the support check, meaning
it will successfully return "0 100\n" instead of -EOPNOTSUPP.

[ ... ]
> +static ssize_t right_joystick_anti_deadzone_range_show(struct device *dev,
> +						       struct device_attribute *attr,
> +						       char *buf)
> +{
> +	return sysfs_emit(buf, "0 100\n");
> +}

[Severity: Low]
Does this code have the same issue as left_joystick_anti_deadzone_range_show(),
where the range values are returned unconditionally even when the device does
not support the anti-deadzone feature?

[ ... ]
> @@ -1894,8 +2089,11 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al
>  		}
>  	}
>  
> -	/* Skip the calibration groups when the capability is missing. */
> -	if (cfg->user_cal_support) {
> +	/*
> +	 * Skip the calibration and anti-deadzone groups when none of the
> +	 * features they expose is supported.
> +	 */
> +	if (cfg->user_cal_support || cfg->anti_deadzone_support) {
>  		int cal_i;
>  
>  		for (cal_i = 0; cal_i < ARRAY_SIZE(ally_cal_attr_groups);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009122820.7603-1-denis.benato@linux.dev?part=7