Re: [PATCH v3] HID: logitech-hidpp: Add support for HID++ Multi-Platform feature (0x4531)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Use-after-destroy of `hidpp->send_mutex` and operations on stopped hardware due to delayed sysfs group removal.
--

commit 55433f57873d505e62b905c239d0015edd306ae5
Author: Baraa Atta (Dev Exalt) <exalt.dev.team@gmail.com>

HID: logitech-hidpp: Add support for HID++ Multi-Platform feature (0x4531)

Add support in the Logitech HID++ driver for the HID++ Multi-Platform
feature (0x4531), which enables HID++ devices to adjust their behavior
based on the host operating system.

> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 90b0184df777f..33904f3093803 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
[ ... ]
> @@ -4780,6 +4780,26 @@ static void hidpp_multiplatform_init(struct hidpp_device *hidpp)
>  {
>  	struct hid_device *hdev = hidpp->hid_dev;
>  	u8 feat_index;
>  	int ret;
>  
>  	ret = hidpp_root_get_feature(hidpp, HIDPP_MULTIPLATFORM_FEAT_ID, &feat_index);
>  	if (ret)
>  		return;
>  
>  	hidpp->multiplatform_feature_index = feat_index;
>  
>  	ret = devm_mutex_init(&hdev->dev, &hidpp->multiplatform_lock);
>  	if (ret) {
>  		hid_warn(hdev,
>  			 "Multiplatform: Failed to init mutex (err=%d)\n", ret);
>  		hidpp->multiplatform_feature_index = 0;
>  		return;
>  	}
>  
>  	ret = devm_device_add_group(&hdev->dev, &multiplatform_attribute_group);

[Severity: High]
Does this create a use-after-destroy race condition during device unbind?

Because devm_device_add_group() is used, the sysfs group is scheduled for
removal during the devres phase, which runs after the driver's remove callback
returns.

In hidpp_remove(), the hardware state is explicitly torn down and the mutex is
destroyed:

drivers/hid/hid-logitech-hidpp.c:hidpp_remove() {
	...
	hid_hw_stop(hdev);
	cancel_work_sync(&hidpp->work);
	cancel_work_sync(&hidpp->reset_hi_res_work);
	mutex_destroy(&hidpp->send_mutex);
}

The sysfs attribute remains accessible to userspace during and shortly after
hidpp_remove() runs. A concurrent write to the platform sysfs attribute could
trigger platform_store(), which eventually attempts to lock the destroyed
hidpp->send_mutex and perform I/O on stopped hardware.

>  	if (ret) {
>  		hid_warn(hdev,
>  			 "Multiplatform: Failed to create sysfs group (err=%d)\n", ret);
>  		hidpp->multiplatform_feature_index = 0;
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630094838.50471-1-exalt.dev.team@gmail.com?part=1