[PATCH v2] HID: asus: synchronize fn_lock_sync_work initialization and teardown

[PATCH v2] HID: asus: synchronize fn_lock_sync_work initialization and teardown

[Sahil Chandna]

Syzbot reported a workqueue warning where cancel_work_sync() was
called on an uninitialized work_struct. Fix this by adding a mutex lock and
initializing fn_lock_sync_work before marking fn_lock as enabled.
This ensures cancel_work_sync() is only called for an initialized work.

Fixes: f631011e36b8 ("HID: hid-asus: Implement fn lock for Asus ProArt P16")
Reported-by: syzbot+13f8286fa2de04a7cd48@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=13f8286fa2de04a7cd48
Suggested-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Sahil Chandna <chandna.sahil@gmail.com>
---
link to v1: https://lore.kernel.org/all/20260130155204.96831-1-chandna.sahil@gmail.com/
changes since v1:
- Add mutex locking between init_work and cancel_work.

Testing:
- Reproduced bug on local setup and original syz reproducer.
- Tested with fix and original C reproducer, issue is not reproduced
locally.
---
 drivers/hid/hid-asus.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 1b9793f7c07e..2d6b6e363bde 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -147,6 +147,7 @@ struct asus_drvdata {
 	unsigned long battery_next_query;
 	struct work_struct fn_lock_sync_work;
 	bool fn_lock;
+	struct mutex lock;
 };
 
 static int asus_report_battery(struct asus_drvdata *, u8 *, int);
@@ -960,8 +961,10 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
 	}
 
 	if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
-		drvdata->fn_lock = true;
+		mutex_lock(&drvdata->lock);
 		INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
+		drvdata->fn_lock = true;
+		mutex_unlock(&drvdata->lock);
 		asus_kbd_set_fn_lock(hdev, true);
 	}
 
@@ -1258,6 +1261,7 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
 
 	drvdata->hdev = hdev;
+	mutex_init(&drvdata->lock);
 
 	if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI)) {
 		ret = asus_battery_probe(hdev);
@@ -1343,8 +1347,10 @@ static void asus_remove(struct hid_device *hdev)
 		cancel_work_sync(&drvdata->kbd_backlight->work);
 	}
 
-	if (drvdata->quirks & QUIRK_HID_FN_LOCK)
+	mutex_lock(&drvdata->lock);
+	if ((drvdata->quirks & QUIRK_HID_FN_LOCK) && drvdata->fn_lock)
 		cancel_work_sync(&drvdata->fn_lock_sync_work);
+	mutex_unlock(&drvdata->lock);
 
 	hid_hw_stop(hdev);
 }
-- 
2.50.1

Re: [PATCH v2] HID: asus: synchronize fn_lock_sync_work initialization and teardown

[Sahil Chandna]

On Wed, Feb 04, 2026 at 11:50:29PM +0530, Sahil Chandna wrote:
>Syzbot reported a workqueue warning where cancel_work_sync() was
>called on an uninitialized work_struct. Fix this by adding a mutex lock and
>initializing fn_lock_sync_work before marking fn_lock as enabled.
>This ensures cancel_work_sync() is only called for an initialized work.
>
>Fixes: f631011e36b8 ("HID: hid-asus: Implement fn lock for Asus ProArt P16")
>Reported-by: syzbot+13f8286fa2de04a7cd48@syzkaller.appspotmail.com
>Closes: https://syzkaller.appspot.com/bug?extid=13f8286fa2de04a7cd48
>Suggested-by: Alan Stern <stern@rowland.harvard.edu>
>Signed-off-by: Sahil Chandna <chandna.sahil@gmail.com>
>---
>link to v1: https://lore.kernel.org/all/20260130155204.96831-1-chandna.sahil@gmail.com/
>changes since v1:
>- Add mutex locking between init_work and cancel_work.
>
>Testing:
>- Reproduced bug on local setup and original syz reproducer.
>- Tested with fix and original C reproducer, issue is not reproduced
>locally.
>---
> drivers/hid/hid-asus.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>index 1b9793f7c07e..2d6b6e363bde 100644
>--- a/drivers/hid/hid-asus.c
>+++ b/drivers/hid/hid-asus.c
>@@ -147,6 +147,7 @@ struct asus_drvdata {
> 	unsigned long battery_next_query;
> 	struct work_struct fn_lock_sync_work;
> 	bool fn_lock;
>+	struct mutex lock;
> };
>
> static int asus_report_battery(struct asus_drvdata *, u8 *, int);
>@@ -960,8 +961,10 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
> 	}
>
> 	if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
>-		drvdata->fn_lock = true;
>+		mutex_lock(&drvdata->lock);
> 		INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
>+		drvdata->fn_lock = true;
>+		mutex_unlock(&drvdata->lock);
> 		asus_kbd_set_fn_lock(hdev, true);
> 	}
>
>@@ -1258,6 +1261,7 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> 		hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
>
> 	drvdata->hdev = hdev;
>+	mutex_init(&drvdata->lock);
>
> 	if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI)) {
> 		ret = asus_battery_probe(hdev);
>@@ -1343,8 +1347,10 @@ static void asus_remove(struct hid_device *hdev)
> 		cancel_work_sync(&drvdata->kbd_backlight->work);
> 	}
>
>-	if (drvdata->quirks & QUIRK_HID_FN_LOCK)
>+	mutex_lock(&drvdata->lock);
>+	if ((drvdata->quirks & QUIRK_HID_FN_LOCK) && drvdata->fn_lock)
> 		cancel_work_sync(&drvdata->fn_lock_sync_work);
>+	mutex_unlock(&drvdata->lock);
>
> 	hid_hw_stop(hdev);
> }
>-- 
>2.50.1
>
Hi, 
just checking if there is any feedback regarding this patch.

regards,
Sahil