[PATCH] workqueue: fix devm_alloc_workqueue() va_list misuse

[PATCH] workqueue: fix devm_alloc_workqueue() va_list misuse

[Breno Leitao]

devm_alloc_workqueue() builds a va_list from its own variadic
arguments and then passes that va_list 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);

alloc_workqueue() expands to alloc_workqueue_noprof(), which
performs its own va_start() over its ... parameters. The inner
vsnprintf(wq->name, sizeof(wq->name), fmt, args) inside
__alloc_workqueue() therefore receives the outer va_list object
as the first variadic slot, not the caller's actual format
arguments. On x86-64 SysV a "%s" conversion dereferences the
pointer inside that va_list as a char * and copies arbitrary
stack bytes from devm_alloc_workqueue()'s register-save area
into wq->name. The write is bounded by WQ_NAME_LEN but the
read is not, so this is an out-of-bounds stack read whose result
also surfaces via show_all_workqueues(), print_worker_info(),
the WQ_NAME_LEN pr_warn_once(), and lockdep lock names.

C does not allow forwarding a va_list through a ... parameter;
a v-style entry point is required. __alloc_workqueue() is
exactly that and is already used by alloc_workqueue_noprof().
Split devm_alloc_workqueue() into devm_alloc_workqueue_noprof()
plus an alloc_hooks() macro wrapper, mirroring the
alloc_workqueue / alloc_workqueue_noprof split. The _noprof
function calls __alloc_workqueue() directly with the va_list we
already hold, and runs wq_init_lockdep(wq) afterwards to mirror
alloc_workqueue_noprof() (otherwise wq->lockdep_map stays NULL
and __flush_workqueue()'s on-stack
COMPLETION_INITIALIZER_ONSTACK_MAP would NULL-deref it).

Keeping the alloc_hooks() wrapper preserves memory allocation
profiling attribution under CONFIG_MEM_ALLOC_PROFILING=y: the
kzalloc_noprof() and alloc_workqueue_attrs_noprof() inside
__alloc_workqueue() are charged to the driver call site rather
than to a single line in kernel/workqueue.c.

No caller changes are required. Drivers continue to write
devm_alloc_workqueue() and devm_alloc_ordered_workqueue() as
before; the new macro forwards through alloc_hooks() into the
renamed _noprof function transparently. Out-of-tree modules
need only be recompiled against the updated header to pick up
the renamed export, mirroring the precedent set by the existing
alloc_workqueue / alloc_workqueue_noprof split.

devm_alloc_ordered_workqueue() in include/linux/workqueue.h is
a macro that forwards to devm_alloc_workqueue(), so it inherits
this fix automatically. 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: 1dfc9d60a69e ("workqueue: devres: Add device-managed allocate workqueue")
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/linux/workqueue.h |  6 ++++--
 kernel/workqueue.c        | 16 ++++++++++++----
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index ab6cb70ca1a52..6177624539b3b 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -534,8 +534,10 @@ alloc_workqueue_noprof(const char *fmt, unsigned int flags, int max_active, ...)
  * Pointer to the allocated workqueue on success, %NULL on failure.
  */
 __printf(2, 5) struct workqueue_struct *
-devm_alloc_workqueue(struct device *dev, const char *fmt, unsigned int flags,
-		     int max_active, ...);
+devm_alloc_workqueue_noprof(struct device *dev, const char *fmt,
+			    unsigned int flags, int max_active, ...);
+#define devm_alloc_workqueue(...)	\
+	alloc_hooks(devm_alloc_workqueue_noprof(__VA_ARGS__))
 
 #ifdef CONFIG_LOCKDEP
 /**
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 5f747f241a5f1..ee5bd627bb52a 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -5932,26 +5932,34 @@ static void devm_workqueue_release(void *res)
 }
 
 __printf(2, 5) struct workqueue_struct *
-devm_alloc_workqueue(struct device *dev, const char *fmt, unsigned int flags,
-		     int max_active, ...)
+devm_alloc_workqueue_noprof(struct device *dev, const char *fmt,
+			    unsigned int flags, int max_active, ...)
 {
 	struct workqueue_struct *wq;
 	va_list args;
 	int ret;
 
 	va_start(args, max_active);
-	wq = alloc_workqueue(fmt, flags, max_active, args);
+	wq = __alloc_workqueue(fmt, flags, max_active, args);
 	va_end(args);
 	if (!wq)
 		return NULL;
 
+	/*
+	 * __alloc_workqueue() doesn't initialize wq->lockdep_map; only its
+	 * sibling alloc_workqueue_noprof() does. Mirror that here, otherwise
+	 * __flush_workqueue()'s COMPLETION_INITIALIZER_ONSTACK_MAP would
+	 * dereference a NULL wq->lockdep_map.
+	 */
+	wq_init_lockdep(wq);
+
 	ret = devm_add_action_or_reset(dev, devm_workqueue_release, wq);
 	if (ret)
 		return NULL;
 
 	return wq;
 }
-EXPORT_SYMBOL_GPL(devm_alloc_workqueue);
+EXPORT_SYMBOL_GPL(devm_alloc_workqueue_noprof);
 
 #ifdef CONFIG_LOCKDEP
 __printf(1, 5)

---
base-commit: dd6c438c3e64a5ff0b5d7e78f7f9be547803ef1b
change-id: 20260427-wq_fix_chris-e23e3423c9a3

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH] workqueue: fix devm_alloc_workqueue() va_list misuse

[Tejun Heo]

Hello, Breno.

Nice catch. Could you fold both noprof entries onto a shared helper?

	static struct workqueue_struct *
	alloc_workqueue_va(const char *fmt, unsigned int flags,
			   int max_active, va_list args)
	{
		struct workqueue_struct *wq;

		wq = __alloc_workqueue(fmt, flags, max_active, args);
		if (wq)
			wq_init_lockdep(wq);
		return wq;
	}

alloc_workqueue_lockdep_map() can stay on bare __alloc_workqueue() since
it supplies its own lockdep_map.

Thanks.

--
tejun

Re: [PATCH] workqueue: fix devm_alloc_workqueue() va_list misuse

[Andy Shevchenko]

On Mon, Apr 27, 2026 at 09:47:44AM -0700, Breno Leitao wrote:
> devm_alloc_workqueue() builds a va_list from its own variadic
> arguments and then passes that va_list 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);
> 
> alloc_workqueue() expands to alloc_workqueue_noprof(), which
> performs its own va_start() over its ... parameters. The inner
> vsnprintf(wq->name, sizeof(wq->name), fmt, args) inside
> __alloc_workqueue() therefore receives the outer va_list object
> as the first variadic slot, not the caller's actual format
> arguments. On x86-64 SysV a "%s" conversion dereferences the
> pointer inside that va_list as a char * and copies arbitrary
> stack bytes from devm_alloc_workqueue()'s register-save area
> into wq->name. The write is bounded by WQ_NAME_LEN but the
> read is not, so this is an out-of-bounds stack read whose result
> also surfaces via show_all_workqueues(), print_worker_info(),
> the WQ_NAME_LEN pr_warn_once(), and lockdep lock names.
> 
> C does not allow forwarding a va_list through a ... parameter;
> a v-style entry point is required. __alloc_workqueue() is
> exactly that and is already used by alloc_workqueue_noprof().
> Split devm_alloc_workqueue() into devm_alloc_workqueue_noprof()
> plus an alloc_hooks() macro wrapper, mirroring the
> alloc_workqueue / alloc_workqueue_noprof split. The _noprof
> function calls __alloc_workqueue() directly with the va_list we
> already hold, and runs wq_init_lockdep(wq) afterwards to mirror
> alloc_workqueue_noprof() (otherwise wq->lockdep_map stays NULL
> and __flush_workqueue()'s on-stack
> COMPLETION_INITIALIZER_ONSTACK_MAP would NULL-deref it).
> 
> Keeping the alloc_hooks() wrapper preserves memory allocation
> profiling attribution under CONFIG_MEM_ALLOC_PROFILING=y: the
> kzalloc_noprof() and alloc_workqueue_attrs_noprof() inside
> __alloc_workqueue() are charged to the driver call site rather
> than to a single line in kernel/workqueue.c.
> 
> No caller changes are required. Drivers continue to write
> devm_alloc_workqueue() and devm_alloc_ordered_workqueue() as
> before; the new macro forwards through alloc_hooks() into the
> renamed _noprof function transparently. Out-of-tree modules
> need only be recompiled against the updated header to pick up
> the renamed export, mirroring the precedent set by the existing
> alloc_workqueue / alloc_workqueue_noprof split.
> 
> devm_alloc_ordered_workqueue() in include/linux/workqueue.h is
> a macro that forwards to devm_alloc_workqueue(), so it inherits
> this fix automatically. 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]

> 

There shouldn't be blank line(s) in the tag block.

> Fixes: 1dfc9d60a69e ("workqueue: devres: Add device-managed allocate workqueue")
> Signed-off-by: Breno Leitao <leitao@debian.org>

Otherwise second Tejun's "nice catch"!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] workqueue: fix devm_alloc_workqueue() va_list misuse

[Breno Leitao]

On Mon, Apr 27, 2026 at 08:51:22PM +0300, Andy Shevchenko wrote:
> On Mon, Apr 27, 2026 at 09:47:44AM -0700, Breno Leitao wrote:
> > devm_alloc_workqueue() builds a va_list from its own variadic
> > arguments and then passes that va_list 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);
> > 
> > alloc_workqueue() expands to alloc_workqueue_noprof(), which
> > performs its own va_start() over its ... parameters. The inner
> > vsnprintf(wq->name, sizeof(wq->name), fmt, args) inside
> > __alloc_workqueue() therefore receives the outer va_list object
> > as the first variadic slot, not the caller's actual format
> > arguments. On x86-64 SysV a "%s" conversion dereferences the
> > pointer inside that va_list as a char * and copies arbitrary
> > stack bytes from devm_alloc_workqueue()'s register-save area
> > into wq->name. The write is bounded by WQ_NAME_LEN but the
> > read is not, so this is an out-of-bounds stack read whose result
> > also surfaces via show_all_workqueues(), print_worker_info(),
> > the WQ_NAME_LEN pr_warn_once(), and lockdep lock names.
> > 
> > C does not allow forwarding a va_list through a ... parameter;
> > a v-style entry point is required. __alloc_workqueue() is
> > exactly that and is already used by alloc_workqueue_noprof().
> > Split devm_alloc_workqueue() into devm_alloc_workqueue_noprof()
> > plus an alloc_hooks() macro wrapper, mirroring the
> > alloc_workqueue / alloc_workqueue_noprof split. The _noprof
> > function calls __alloc_workqueue() directly with the va_list we
> > already hold, and runs wq_init_lockdep(wq) afterwards to mirror
> > alloc_workqueue_noprof() (otherwise wq->lockdep_map stays NULL
> > and __flush_workqueue()'s on-stack
> > COMPLETION_INITIALIZER_ONSTACK_MAP would NULL-deref it).
> > 
> > Keeping the alloc_hooks() wrapper preserves memory allocation
> > profiling attribution under CONFIG_MEM_ALLOC_PROFILING=y: the
> > kzalloc_noprof() and alloc_workqueue_attrs_noprof() inside
> > __alloc_workqueue() are charged to the driver call site rather
> > than to a single line in kernel/workqueue.c.
> > 
> > No caller changes are required. Drivers continue to write
> > devm_alloc_workqueue() and devm_alloc_ordered_workqueue() as
> > before; the new macro forwards through alloc_hooks() into the
> > renamed _noprof function transparently. Out-of-tree modules
> > need only be recompiled against the updated header to pick up
> > the renamed export, mirroring the precedent set by the existing
> > alloc_workqueue / alloc_workqueue_noprof split.
> > 
> > devm_alloc_ordered_workqueue() in include/linux/workqueue.h is
> > a macro that forwards to devm_alloc_workqueue(), so it inherits
> > this fix automatically. 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]
> 
> > 
> 
> There shouldn't be blank line(s) in the tag block.

Ack, I will remove it in the respin.

> 
> > Fixes: 1dfc9d60a69e ("workqueue: devres: Add device-managed allocate workqueue")
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> 
> Otherwise second Tejun's "nice catch"!

Re: [PATCH] workqueue: fix devm_alloc_workqueue() va_list misuse

[Breno Leitao]

On Mon, Apr 27, 2026 at 07:29:31AM -1000, Tejun Heo wrote:
> Nice catch. Could you fold both noprof entries onto a shared helper?
> 
> 	static struct workqueue_struct *
> 	alloc_workqueue_va(const char *fmt, unsigned int flags,
> 			   int max_active, va_list args)
> 	{
> 		struct workqueue_struct *wq;
> 
> 		wq = __alloc_workqueue(fmt, flags, max_active, args);
> 		if (wq)
> 			wq_init_lockdep(wq);
> 		return wq;
> 	}
> 
> alloc_workqueue_lockdep_map() can stay on bare __alloc_workqueue() since
> it supplies its own lockdep_map.

Ack, I will update and respin.

Thanks for the review,
--breno