Re: [PATCH] workqueue: validate cpumask_first() result in llc_populate_cpu_shard_id()

[Breno Leitao <leitao@debian.org>]

Hello Tejun,

On Thu, Apr 09, 2026 at 11:00:30PM -1000, Tejun Heo wrote:
> On Fri, Apr 10, 2026 at 01:49:50AM -0700, Breno Leitao wrote:
> > In llc_populate_cpu_shard_id(), cpumask_first(sibling_cpus) is used to
> > find the leader CPU, and the result is then used to index into
> > cpu_shard_id[]. Add a bounds check with WARN_ON_ONCE to guard against
> > unexpected values before using it as an array index.
> > 
> > Store the result in a local variable to make the code clearer, as also
> > to avoid calling cpumask_first() twice.
> > 
> > Fixes: 5920d046f7ae3 ("workqueue: add WQ_AFFN_CACHE_SHARD affinity scope")
> ...
> > @@ -8318,7 +8319,11 @@ static void __init llc_populate_cpu_shard_id(const struct cpumask *pod_cpus,
> >  			 * The siblings' shard MUST be the same as the leader.
> >  			 * never split threads in the same core.
> >  			 */
> > -			cpu_shard_id[c] = cpu_shard_id[cpumask_first(sibling_cpus)];
> > +			leader = cpumask_first(sibling_cpus);
> > +
> > +			if (WARN_ON_ONCE(leader >= nr_cpu_ids))
> > +				continue;
> > +			cpu_shard_id[c] = cpu_shard_id[leader];
> 
> sibling_cpus can't be empty, right?

Correct. sibling_cpus will have, at least, 'c' set.

> This is mostly to shut up the reported
> compiler warning? If so, can you please note that in a ocmment and the
> description?

Sure. Is something like the following acceptable?

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 083d8fe301f46..5dc304cdfa7f9 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -8300,6 +8300,7 @@ static void __init llc_populate_cpu_shard_id(const struct cpumask *pod_cpus,
        int cores_in_shard = 0;
        /* This is a cursor for the shards. Go from zero to nr_shards - 1*/
        int shard_id = 0;
+       int leader;
        int c;

        /* Iterate at every CPU for a given LLC pod, and assign it a shard */
@@ -8318,7 +8319,17 @@ static void __init llc_populate_cpu_shard_id(const struct cpumask *pod_cpus,
                         * The siblings' shard MUST be the same as the leader.
                         * never split threads in the same core.
                         */
-                       cpu_shard_id[c] = cpu_shard_id[cpumask_first(sibling_cpus)];
+                       leader = cpumask_first(sibling_cpus);
+
+                       /*
+                        * sibling_cpus cannot be empty here since 'c'
+                        * is always set in it. This check silences a
+                        * compiler warning about using the unchecked
+                        * cpumask_first() result as an array index.
+                        */
+                       if (WARN_ON_ONCE(leader >= nr_cpu_ids))
+                               continue;
+                       cpu_shard_id[c] = cpu_shard_id[leader];
                }
        }