Re: [PATCH] HID: lenovo: Restore Fn-lock LED state on resume for tp10ubkbd devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unprotected state modifications / TOCTOU Race leading to Deadlock
--

commit 7c663fefe9bb0e01bc7e377e27b713f9cd82c9d8
Author: Kean <rh_king@163.com>

HID: lenovo: Restore Fn-lock LED state on resume for tp10ubkbd devices

This commit updates the reset_resume callback to restore the Fn-lock LED
state for several ThinkPad tablet keyboards. It does this by reading the
driver's cached state and calling lenovo_led_set_tp10ubkbd to update the
hardware.

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

[ ... ]

> @@ -1424,6 +1424,8 @@ static int lenovo_probe(struct hid_device *hdev,
>  
>  static int lenovo_reset_resume(struct hid_device *hdev)
>  {
> +	struct lenovo_drvdata *data;
> +
>  	switch (hdev->product) {
>  	case USB_DEVICE_ID_LENOVO_CUSBKBD:
>  	case USB_DEVICE_ID_LENOVO_TPIIUSBKBD:
> @@ -1431,6 +1433,17 @@ static int lenovo_reset_resume(struct hid_device *hdev)
>  			lenovo_features_set_cptkbd(hdev);
>  
>  		break;
> +	case USB_DEVICE_ID_LENOVO_X12_TAB:
> +	case USB_DEVICE_ID_LENOVO_X12_TAB2:
> +	case USB_DEVICE_ID_LENOVO_TP10UBKBD:
> +	case USB_DEVICE_ID_LENOVO_X1_TAB:
> +	case USB_DEVICE_ID_LENOVO_X1_TAB2:
> +	case USB_DEVICE_ID_LENOVO_X1_TAB3:
> +		data = hid_get_drvdata(hdev);
> +		if (data)
> +			lenovo_led_set_tp10ubkbd(hdev, TP10UBKBD_FN_LOCK_LED,
> +						 data->fn_lock ? LED_ON : LED_OFF);

Could this new call to lenovo_led_set_tp10ubkbd() cause a deadlock during
runtime resume?

When userspace writes to the fn_lock sysfs attribute, it eventually calls
lenovo_led_set_tp10ubkbd() which acquires led_report_mutex before issuing
the raw HID request:

lenovo_led_set_tp10ubkbd()
    mutex_lock(&data->led_report_mutex);
    ...
    hid_hw_raw_request(...)

If the device is runtime-suspended and requires a reset-resume,
hid_hw_raw_request() will trigger usb_autopm_get_interface() to
synchronously wake the device. This causes the USB core to execute
lenovo_reset_resume() in the same thread:

lenovo_reset_resume()
    lenovo_led_set_tp10ubkbd()
        mutex_lock(&data->led_report_mutex);

Since led_report_mutex is already held by the initial sysfs write call,
will this result in an immediate self-deadlock?

Additionally, if a resume is initiated by a remote wakeup via a PM worker
thread while a sysfs write is concurrently holding led_report_mutex, could
this lead to an ABBA deadlock between the sysfs thread and the PM worker
thread?

> +		break;
>  	default:
>  		break;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512142229.4098386-1-rh_king@163.com?part=1