Re: [PATCH] cpu: Fix online housekeeping CPU check in _cpu_down()

[Waiman Long]

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@xxxxxxxxxxxx/

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