Re: [PATCH 1/3] HID: haptic: Replace create_singlethread_workqueue with alloc_ordered_workqueue
flat view
From: sashiko-bot@kernel.org
Date: 2026-10-28 11:33:52
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] HID: haptic: format string bug in hid_haptic_init() workqueue allocation
- [Medium] HID: haptic: misuse of WQ_MEM_RECLAIM in hid_haptic_init()
--
commit 7db50def49522def9f10d7cdd2d640a9ac167fa3
Author: George Karagounis [off-list ref]
HID: haptic: Replace create_singlethread_workqueue with alloc_ordered_workqueue
This commit replaces the deprecated create_singlethread_workqueue() call in
hid_haptic_init() with alloc_ordered_workqueue() using the WQ_MEM_RECLAIM
flag.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c index 8760eeb08b2cd..2f4fcb56aba13 100644 --- a/drivers/hid/hid-haptic.c +++ b/drivers/hid/hid-haptic.c@@ -460,7 +460,7 @@ int hid_haptic_init(struct hid_device *hdev, name = kmalloc(strlen(prefix) + strlen(hdev->name) + 2, GFP_KERNEL); if (name) { sprintf(name, "%s %s", prefix, hdev->name); - haptic->wq = create_singlethread_workqueue(name); + haptic->wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
[Severity: Critical]
Does passing name directly as the first argument to alloc_ordered_workqueue()
in hid_haptic_init() introduce a format string regression when hdev->name
contains % characters?
Previously, create_singlethread_workqueue(name) expanded to:
include/linux/workqueue.h:
#define create_singlethread_workqueue(name) \
alloc_ordered_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, name)
With alloc_ordered_workqueue(name, WQ_MEM_RECLAIM), name is passed as the
printf format string fmt to alloc_workqueue() -> __alloc_workqueue() ->
vsnprintf(wq->name, sizeof(wq->name), fmt, args) with no variadic format
arguments. Because hdev->name comes from external HID device descriptors or
userspace via uhid, any % specifiers in hdev->name will be interpreted by
vsnprintf().
Since alloc_ordered_workqueue() accepts a format string and variadic
arguments directly, could this pass "%s %s", 0, prefix, hdev->name and avoid
the intermediate kmalloc(), sprintf(), and kfree() of name altogether?
[Severity: Medium]
Also, is WQ_MEM_RECLAIM appropriate for haptic->wq in hid_haptic_init()?
The commit message notes that WQ_MEM_RECLAIM is added to ensure hardware
events can be processed under memory pressure, but WQ_MEM_RECLAIM is intended
for workqueues depended upon during memory reclaim paths and allocates a
dedicated rescuer kthread per device.
haptic->wq only runs haptic_work_handler() -> play_effect() to transmit
force-feedback output reports via hid_hw_output_report() and
hid_hw_raw_request(), which is not part of memory reclaim. Dropping
__WQ_LEGACY while keeping WQ_MEM_RECLAIM also enables check_flush_dependency()
warnings in kernel/workqueue.c if the underlying transport flushes a
non-WQ_MEM_RECLAIM workqueue. Should the flags argument be 0 instead?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261028111853.22429-1-mail@taterr.org?part=1