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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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