History

workqueue: fix devm_alloc_workqueue() va_list misuse

devm_alloc_workqueue() built a va_list and passed it as a single positional argument to the variadic alloc_workqueue() macro: va_start(args, max_active); wq = alloc_workqueue(fmt, flags, max_active, args); va_end(args); C does not allow forwarding a va_list through a ... parameter. alloc_workqueue() expands to alloc_workqueue_noprof(), which runs its own va_start() over its ... params, so the inner vsnprintf(wq->name, sizeof(wq->name), fmt, args) in __alloc_workqueue() received the outer va_list object as the first variadic slot rather than the caller's actual format arguments. Add a new static helper alloc_workqueue_va() that wraps __alloc_workqueue() and runs wq_init_lockdep() on success, and fold both alloc_workqueue_noprof() and devm_alloc_workqueue_noprof() onto it as suggested by Tejun. The wq_init_lockdep() step is required on the devm path too, otherwise __flush_workqueue()'s on-stack COMPLETION_INITIALIZER_ONSTACK_MAP would NULL-deref wq->lockdep_map. No caller changes are required. devm_alloc_ordered_workqueue() is a macro forwarding to devm_alloc_workqueue() and inherits the fix. Two in-tree callers actively trigger the broken path on every probe: drivers/power/supply/mt6370-charger.c:889 drivers/power/supply/max77705_charger.c:649 both of which use devm_alloc_ordered_workqueue(dev, "%s", 0, dev_name(dev)). A standalone reproducer module is available at[1].

Breno Leitao committed 6 months ago

Commit 0de4cb41 parent dca922e2 files · +23 −11Browse files

Showing the changes to kernel/workqueue.c (1 of 2 files). Show all

Unified · Split

kernel/workqueue.c+19−9
@@ -5906,6 +5906,20 @@ err_destroy: · 5905 unmodified lines
59065906 return NULL;
59075907 }
59085908
5909+static struct workqueue_struct *alloc_workqueue_va(const char *fmt,
5910+ unsigned int flags,
5911+ int max_active,
5912+ va_list args)
5913+{
5914+ struct workqueue_struct *wq;
5915+
5916+ wq = __alloc_workqueue(fmt, flags, max_active, args);
5917+ if (wq)
5918+ wq_init_lockdep(wq);
5919+
5920+ return wq;
5921+}
5922+
59095923 __printf(1, 4)
59105924 struct workqueue_struct *alloc_workqueue_noprof(const char *fmt,
59115925 unsigned int flags,
@@ -5915,12 +5929,8 @@ struct workqueue_struct *alloc_workqueue_noprof(const char *fmt, · 3 unmodified lines
59155929 va_list args;
59165930
59175931 va_start(args, max_active);
5918− wq = __alloc_workqueue(fmt, flags, max_active, args);
5932+ wq = alloc_workqueue_va(fmt, flags, max_active, args);
59195933 va_end(args);
5920− if (!wq)
5921− return NULL;
5922−
5923− wq_init_lockdep(wq);
59245934
59255935 return wq;
59265936 }
@@ -5932,15 +5942,15 @@ static void devm_workqueue_release(void *res) · 5 unmodified lines
59325942 }
59335943
59345944 __printf(2, 5) struct workqueue_struct *
5935−devm_alloc_workqueue(struct device *dev, const char *fmt, unsigned int flags,
5936− int max_active, ...)
5945+devm_alloc_workqueue_noprof(struct device *dev, const char *fmt,
5946+ unsigned int flags, int max_active, ...)
59375947 {
59385948 struct workqueue_struct *wq;
59395949 va_list args;
59405950 int ret;
59415951
59425952 va_start(args, max_active);
5943− wq = alloc_workqueue(fmt, flags, max_active, args);
5953+ wq = alloc_workqueue_va(fmt, flags, max_active, args);
59445954 va_end(args);
59455955 if (!wq)
59465956 return NULL;
@@ -5951,7 +5961,7 @@ devm_alloc_workqueue(struct device *dev, const char *fmt, unsigned int flags, · 4 unmodified lines
59515961
59525962 return wq;
59535963 }
5954−EXPORT_SYMBOL_GPL(devm_alloc_workqueue);
5964+EXPORT_SYMBOL_GPL(devm_alloc_workqueue_noprof);
59555965
59565966 #ifdef CONFIG_LOCKDEP
59575967 __printf(1, 5)