* 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