linux-kernel - Re: [PATCH] HID: asus: Add check for cancelling fn_lock_sync

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <273d8509-3d2e-4686-b56f-a12c5c203291@rowland.harvard.edu>
Date: Mon, 2 Feb 2026 22:27:33 -0500
From: Alan Stern <stern@...land.harvard.edu>
To: Sahil Chandna <chandna.sahil@...il.com>, Jiri Kosina <jikos@...nel.org>
Cc: bentiss@...nel.org, connorbelli2003@...il.com,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] HID: asus: Add check for cancelling fn_lock_sync_work

I just noticed this because of a related message in a different thread.

On Fri, Jan 26, 2026 at 09:22:04PM +0530, Sahil Chandna wrote:

> syzbot reported a workqueue warning where fn_lock_sync_work is cancelled
> during device removal before the work has been initialized. This can
> happen when the device is disconnected while initialization is still
> in progress.
> Fix this by initializing fn_lock_sync_work before marking fn_lock as
> enabled, and by using fn_lock as a flag in the remove path. This
> ensures cancel_work_sync() is only called for initialized work.
> 
> Fixes: f631011e36b8 ("HID: hid-asus: Implement fn lock for Asus ProArt P16")
> Reported-by: syzbot+13f8286fa2de04a7cd48@...kaller.appspotmail.com
> Signed-off-by: Sahil Chandna <chandna.sahil@...il.com>
> ---
>  drivers/hid/hid-asus.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 1b9793f7c07e..bb85a36de14f 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -960,8 +960,8 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
>  	}
>  
>  	if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
> -		drvdata->fn_lock = true;
>  		INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
> +		drvdata->fn_lock = true;
>  		asus_kbd_set_fn_lock(hdev, true);
>  	}
>  
> @@ -1343,7 +1343,7 @@ static void asus_remove(struct hid_device *hdev)
>  		cancel_work_sync(&drvdata->kbd_backlight->work);
>  	}
>  
> -	if (drvdata->quirks & QUIRK_HID_FN_LOCK)
> +	if ((drvdata->quirks & QUIRK_HID_FN_LOCK) && drvdata->fn_lock)
>  		cancel_work_sync(&drvdata->fn_lock_sync_work);
>  
>  	hid_hw_stop(hdev);

With no synchronization between the two routines, this patch cannot 
possibly be correct.  There's nothing to prevent the CPU running 
asus_input_configured() from executing the assignment to 
drvdata->fn_lock before doing the INIT_WORK() (unless the INIT_WORK() 
call itself contains some synchronization -- but obviously the code 
shouldn't depend on that).

At the very least there should be a pair of memory barriers.  A more 
palatable substitute would be to protect both regions of code with a 
mutex.

Alan Stern