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

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

[Breno Leitao]

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")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202604022343.GQtkF2vO-lkp@intel.com/
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 kernel/workqueue.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 083d8fe301f46..3cb4376a248b0 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,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];
 		}
 	}
 

---
base-commit: 3fa7d958829eb9bc3b469ed07f11de3d2804ef71
change-id: 20260410-workqueue_fix_nios-e6763904aee9

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

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

[Tejun Heo]

Hello,

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? This is mostly to shut up the reported
compiler warning? If so, can you please note that in a ocmment and the
description?

Thanks.

-- 
tejun

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

[Breno Leitao]

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];
                }
        }

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

[Tejun Heo]

Hello,

On Fri, Apr 10, 2026 at 03:42:00AM -0700, Breno Leitao wrote:
> @@ -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.
> +                        */

Can you add more details on the warning and update the patch desc
accordingly? This only triggers on UP configs, right?

Thanks.

-- 
tejun