mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] cpu: Fix online housekeeping CPU check in _cpu_down()
@ 2026-11-08 18:08 Waiman Long
  2026-11-08 18:19 ` Waiman Long
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Waiman Long @ 2026-11-08 18:08 UTC (permalink / raw)
  To: Thomas Gleixner, Peter Zijlstra, Frederic Weisbecker
  Cc: linux-kernel, Waiman Long

There is currently a check in _cpu_down() to make sure that there is
at least one online housekeeping (HK) CPU left after the shutdown of
the given @cpu parameter. However the cpu_online_mask used for the
check may include the current @cpu to be shut down. So the fact that
there is at least one online housekeeping CPU at the moment does not
guarantee that there will be any online housekeeping CPU left for the
sched domain after the shutdown.

Fix that by making sure that there are at least 2 online housekeeping
CPUs available if @cpu is currently a housekeeping CPU.

Fixes: de715325cc47 ("cpu: Revert "cpu/hotplug: Prevent self deadlock on CPU hot-unplug"")
Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/cpu.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index b3c8553d7bd6..ed641ee6b37c 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1415,11 +1415,14 @@ static int __ref _cpu_down(unsigned int cpu, int tasks_frozen,
 	cpus_write_lock();
 
 	/*
-	 * Keep at least one housekeeping cpu onlined to avoid generating
-	 * an empty sched_domain span.
+	 * Keep at least one housekeeping cpu online to avoid generating an
+	 * empty sched_domain span. As cpu_online_mask includes the @cpu to be
+	 * shut down, there should be at least 2 online HK CPUs at the moment
+	 * if @cpu is currently a HK CPU.
 	 */
-	if (cpumask_any_and(cpu_online_mask,
-			    housekeeping_cpumask(HK_TYPE_DOMAIN)) >= nr_cpu_ids) {
+	if (cpumask_test_cpu(cpu, housekeeping_cpumask(HK_TYPE_DOMAIN)) &&
+	   (cpumask_weight_and(cpu_online_mask,
+			       housekeeping_cpumask(HK_TYPE_DOMAIN)) < 2)) {
 		ret = -EBUSY;
 		goto out;
 	}
-- 
2.56.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpu: Fix online housekeeping CPU check in _cpu_down()
  2026-11-08 18:08 [PATCH] cpu: Fix online housekeeping CPU check in _cpu_down() Waiman Long
@ 2026-11-08 18:19 ` Waiman Long
  2026-11-08 19:18 ` Bradley Morgan
  2026-11-09  2:37 ` Mathieu Desnoyers
  2 siblings, 0 replies; 5+ messages in thread
From: Waiman Long @ 2026-11-08 18:19 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: linux-kernel, Thomas Gleixner, Frederic Weisbecker, Peter Zijlstra

Mathieu,

Sorry, I forgot to cc you on this patch.

Cheers,
Longman

On 11/8/26 1:08 PM, Waiman Long wrote:
> There is currently a check in _cpu_down() to make sure that there is
> at least one online housekeeping (HK) CPU left after the shutdown of
> the given @cpu parameter. However the cpu_online_mask used for the
> check may include the current @cpu to be shut down. So the fact that
> there is at least one online housekeeping CPU at the moment does not
> guarantee that there will be any online housekeeping CPU left for the
> sched domain after the shutdown.
>
> Fix that by making sure that there are at least 2 online housekeeping
> CPUs available if @cpu is currently a housekeeping CPU.
>
> Fixes: de715325cc47 ("cpu: Revert "cpu/hotplug: Prevent self deadlock on CPU hot-unplug"")
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>   kernel/cpu.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index b3c8553d7bd6..ed641ee6b37c 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -1415,11 +1415,14 @@ static int __ref _cpu_down(unsigned int cpu, int tasks_frozen,
>   	cpus_write_lock();
>   
>   	/*
> -	 * Keep at least one housekeeping cpu onlined to avoid generating
> -	 * an empty sched_domain span.
> +	 * Keep at least one housekeeping cpu online to avoid generating an
> +	 * empty sched_domain span. As cpu_online_mask includes the @cpu to be
> +	 * shut down, there should be at least 2 online HK CPUs at the moment
> +	 * if @cpu is currently a HK CPU.
>   	 */
> -	if (cpumask_any_and(cpu_online_mask,
> -			    housekeeping_cpumask(HK_TYPE_DOMAIN)) >= nr_cpu_ids) {
> +	if (cpumask_test_cpu(cpu, housekeeping_cpumask(HK_TYPE_DOMAIN)) &&
> +	   (cpumask_weight_and(cpu_online_mask,
> +			       housekeeping_cpumask(HK_TYPE_DOMAIN)) < 2)) {
>   		ret = -EBUSY;
>   		goto out;
>   	}


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpu: Fix online housekeeping CPU check in _cpu_down()
  2026-11-08 18:08 [PATCH] cpu: Fix online housekeeping CPU check in _cpu_down() Waiman Long
  2026-11-08 18:19 ` Waiman Long
@ 2026-11-08 19:18 ` Bradley Morgan
  2026-11-09  2:37 ` Mathieu Desnoyers
  2 siblings, 0 replies; 5+ messages in thread
From: Bradley Morgan @ 2026-11-08 19:18 UTC (permalink / raw)
  To: longman; +Cc: frederic, linux-kernel, peterz, tglx

On 8 November 2026 18:08:49 GMT, Waiman Long <longman@redhat.com> wrote:
>There is currently a check in _cpu_down() to make sure that there is
>at least one online housekeeping (HK) CPU left after the shutdown of
>the given @cpu parameter. However the cpu_online_mask used for the
>check may include the current @cpu to be shut down. So the fact that
>there is at least one online housekeeping CPU at the moment does not
>guarantee that there will be any online housekeeping CPU left for the
>sched domain after the shutdown.
>
>Fix that by making sure that there are at least 2 online housekeeping
>CPUs available if @cpu is currently a housekeeping CPU.
>
>Fixes: de715325cc47 ("cpu: Revert "cpu/hotplug: Prevent self deadlock on CPU hot-unplug"")
>Signed-off-by: Waiman Long <longman@redhat.com>
>---
> kernel/cpu.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
>diff --git a/kernel/cpu.c b/kernel/cpu.c
>index b3c8553d7bd6..ed641ee6b37c 100644
>--- a/kernel/cpu.c
>+++ b/kernel/cpu.c
>@@ -1415,11 +1415,14 @@ static int __ref _cpu_down(unsigned int cpu, int tasks_frozen,
> 	cpus_write_lock();
> 
> 	/*
>-	 * Keep at least one housekeeping cpu onlined to avoid generating
>-	 * an empty sched_domain span.
>+	 * Keep at least one housekeeping cpu online to avoid generating an
>+	 * empty sched_domain span. As cpu_online_mask includes the @cpu to be
>+	 * shut down, there should be at least 2 online HK CPUs at the moment
>+	 * if @cpu is currently a HK CPU.
> 	 */
>-	if (cpumask_any_and(cpu_online_mask,
>-			    housekeeping_cpumask(HK_TYPE_DOMAIN)) >= nr_cpu_ids) {
>+	if (cpumask_test_cpu(cpu, housekeeping_cpumask(HK_TYPE_DOMAIN)) &&
>+	   (cpumask_weight_and(cpu_online_mask,
>+			       housekeeping_cpumask(HK_TYPE_DOMAIN)) < 2)) {

Well, this patch is good, reasonable enough to me, I may be odd sometimes,
but this patch is sane enough.

Reviewed-by: Bradley Morgan <brads@mainlining.org>


> 		ret = -EBUSY;
> 		goto out;
> 	}
>

--- Thanks!

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpu: Fix online housekeeping CPU check in _cpu_down()
  2026-11-08 18:08 [PATCH] cpu: Fix online housekeeping CPU check in _cpu_down() Waiman Long
  2026-11-08 18:19 ` Waiman Long
  2026-11-08 19:18 ` Bradley Morgan
@ 2026-11-09  2:37 ` Mathieu Desnoyers
  2026-11-09  4:27   ` Waiman Long
  2 siblings, 1 reply; 5+ messages in thread
From: Mathieu Desnoyers @ 2026-11-09  2:37 UTC (permalink / raw)
  To: Waiman Long
  Cc: Thomas Gleixner, Peter Zijlstra, Frederic Weisbecker, Tejun Heo,
	Michal Koutný,
	Ridong Chen, Guopeng Zhang, cgroups, linux-kernel

[ Resent with the lists in Cc. The first copy went to Waiman only. ]

On 2026-11-08 13:08, Waiman Long wrote:
[...]

This is not about the patch itself, but about the mask it reads. The
Sashiko AI reviews of this patch and of my sched/isolation patch [1]
both flag a race on the HK_TYPE_DOMAIN cpumask. I checked it against
the code. It is real in some configurations, and older than this patch.

Since commit a84097e625f2 ("cgroup/cpuset: Call housekeeping_update()
without holding cpus_read_lock"), housekeeping_update() replaces the
mask and frees the old one with only cpuset_top_mutex held. Its message
says that CPU hotplug can run in parallel. But
housekeeping_dereference_check() still accepts cpus_write_lock, or
cpuset_mutex, as enough to read the mask without RCU. _cpu_down() reads
it that way, with or without this patch:

  CPU 0                                  CPU 1

  write to cpuset.cpus.partition
    cpuset_update_sd_hk_unlock()
      mutex_unlock(&cpuset_mutex)
      cpus_read_unlock()
                                         echo 0 > .../cpuN/online
                                           _cpu_down()
                                             cpus_write_lock()
                                             m = housekeeping_cpumask(
                                                   HK_TYPE_DOMAIN)
                                             <preempted>
      housekeeping_update()
        rcu_assign_pointer(...)
        synchronize_rcu()
          CPU 1 is not an RCU reader
        flushes workqueues
        tmigr_isolated_exclude_cpumask()
          the !CONFIG_NO_HZ_COMMON stub
        kfree(old)
                                             cpumask_any_and(..., m)
                                               reads freed memory

With CONFIG_NO_HZ_COMMON, tmigr_isolated_exclude_cpumask() takes
cpus_read_lock() before kfree(old). That waits for CPU 1. So most
configurations are safe, but by accident. Without CONFIG_NO_HZ_COMMON,
nothing makes kfree(old) wait. The same holds when one of its two
allocations fails: it returns -ENOMEM before it takes the lock.

This comes from reading the code. I did not try to reproduce it.

A fix could make housekeeping_update() take and release
cpus_read_lock() before kfree(old). That waits for any hotplug reader
of the old mask, whatever the timer code does. It holds only
cpuset_top_mutex there, which comes first in the cpuset lock order. I
did not find a reader that holds cpuset_mutex without the hotplug
lock, but I did not check every caller. I can send a patch, if you
agree with that direction.

[1] https://lore.kernel.org/all/20261108185129.1549419-1-mathieu.desnoyers@efficios.com/

Thanks,

Mathieu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cpu: Fix online housekeeping CPU check in _cpu_down()
  2026-11-09  2:37 ` Mathieu Desnoyers
@ 2026-11-09  4:27   ` Waiman Long
  0 siblings, 0 replies; 5+ messages in thread
From: Waiman Long @ 2026-11-09  4:27 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Thomas Gleixner, Peter Zijlstra, Frederic Weisbecker, Tejun Heo,
	Michal Koutný,
	Ridong Chen, Guopeng Zhang, cgroups, linux-kernel

On 11/8/26 9:37 PM, Mathieu Desnoyers wrote:
> [ Resent with the lists in Cc. The first copy went to Waiman only. ]
>
> On 2026-11-08 13:08, Waiman Long wrote:
> [...]
>
> This is not about the patch itself, but about the mask it reads. The
> Sashiko AI reviews of this patch and of my sched/isolation patch [1]
> both flag a race on the HK_TYPE_DOMAIN cpumask. I checked it against
> the code. It is real in some configurations, and older than this patch.
>
> Since commit a84097e625f2 ("cgroup/cpuset: Call housekeeping_update()
> without holding cpus_read_lock"), housekeeping_update() replaces the
> mask and frees the old one with only cpuset_top_mutex held. Its message
> says that CPU hotplug can run in parallel. But
> housekeeping_dereference_check() still accepts cpus_write_lock, or
> cpuset_mutex, as enough to read the mask without RCU. _cpu_down() reads
> it that way, with or without this patch:
>
>    CPU 0                                  CPU 1
>
>    write to cpuset.cpus.partition
>      cpuset_update_sd_hk_unlock()
>        mutex_unlock(&cpuset_mutex)
>        cpus_read_unlock()
>                                           echo 0 > .../cpuN/online
>                                             _cpu_down()
>                                               cpus_write_lock()
>                                               m = housekeeping_cpumask(
>                                                     HK_TYPE_DOMAIN)
>                                               <preempted>
>        housekeeping_update()
>          rcu_assign_pointer(...)
>          synchronize_rcu()
>            CPU 1 is not an RCU reader
>          flushes workqueues
>          tmigr_isolated_exclude_cpumask()
>            the !CONFIG_NO_HZ_COMMON stub
>          kfree(old)
>                                               cpumask_any_and(..., m)
>                                                 reads freed memory
>
> With CONFIG_NO_HZ_COMMON, tmigr_isolated_exclude_cpumask() takes
> cpus_read_lock() before kfree(old). That waits for CPU 1. So most
> configurations are safe, but by accident. Without CONFIG_NO_HZ_COMMON,
> nothing makes kfree(old) wait. The same holds when one of its two
> allocations fails: it returns -ENOMEM before it takes the lock.
>
> This comes from reading the code. I did not try to reproduce it.
>
> A fix could make housekeeping_update() take and release
> cpus_read_lock() before kfree(old). That waits for any hotplug reader
> of the old mask, whatever the timer code does. It holds only
> cpuset_top_mutex there, which comes first in the cpuset lock order. I
> did not find a reader that holds cpuset_mutex without the hotplug
> lock, but I did not check every caller. I can send a patch, if you
> agree with that direction.
>
> [1] https://lore.kernel.org/all/20261108185129.1549419-1-mathieu.desnoyers@efficios.com/

Yes, I am aware of the sashiko report of this UAF problem. Taking and 
releasing cpus_read_lock() before kfree() is a possible workaround. 
Another possible fix is to take out cpuset_mutex and cpus_write_lock 
from housekeeping_dereference_check() and add rcu_read_lock/_unlock 
around the housekeeping cpumask access. The latter approach is what I am 
planning to do.

Cheers,
Longman


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-09  4:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-08 18:08 [PATCH] cpu: Fix online housekeeping CPU check in _cpu_down() Waiman Long
2026-11-08 18:19 ` Waiman Long
2026-11-08 19:18 ` Bradley Morgan
2026-11-09  2:37 ` Mathieu Desnoyers
2026-11-09  4:27   ` Waiman Long

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®