Re: [PATCH] HID: asus: Add check for cancelling fn_lock_sync_work

[Sahil Chandna]

On Mon, Feb 02, 2026 at 10:27:33PM -0500, Alan Stern wrote:
> 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Signed-off-by: Sahil Chandna <chandna.sahil@xxxxxxxxx>
> > ---
> >  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
Thanks, I will test with mutex between INIT_WORK and cancel_work
and share v2.
Regards,
Sahil