Skip to content

Commit 0de4cb4

Browse files
leitaohtejun
authored andcommitted
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]. Link: https://github.com/leitao/debug/blob/main/workqueue/valist/wq_va_test.c [1] Fixes: 1dfc9d6 ("workqueue: devres: Add device-managed allocate workqueue") Signed-off-by: Breno Leitao <leitao@debian.org> Signed-off-by: Tejun Heo <tj@kernel.org>
1 parent dca922e commit 0de4cb4

2 files changed

Lines changed: 23 additions & 11 deletions

File tree

include/linux/workqueue.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -534,8 +534,10 @@ alloc_workqueue_noprof(const char *fmt, unsigned int flags, int max_active, ...)
534534
* Pointer to the allocated workqueue on success, %NULL on failure.
535535
*/
536536
__printf(2, 5) struct workqueue_struct *
537-
devm_alloc_workqueue(struct device *dev, const char *fmt, unsigned int flags,
538-
int max_active, ...);
537+
devm_alloc_workqueue_noprof(struct device *dev, const char *fmt,
538+
unsigned int flags, int max_active, ...);
539+
#define devm_alloc_workqueue(...) \
540+
alloc_hooks(devm_alloc_workqueue_noprof(__VA_ARGS__))
539541

540542
#ifdef CONFIG_LOCKDEP
541543
/**

kernel/workqueue.c

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5906,6 +5906,20 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt,
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,
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)
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,
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)

0 commit comments

Comments
 (0)