Re: [PATCH/for-next 2/2] cgroup/cpuset: Introduce a new top level isolcpus_update_mutex

[Waiman Long <llong@redhat.com>]

On 1/29/26 7:56 PM, Chen Ridong wrote:
>
> On 2026/1/30 5:16, Waiman Long wrote:
>> On 1/29/26 3:01 AM, Chen Ridong wrote:
>>> On 2026/1/28 12:42, Waiman Long wrote:
>>>> The current cpuset partition code is able to dynamically update
>>>> the sched domains of a running system and the corresponding
>>>> HK_TYPE_DOMAIN housekeeping cpumask to perform what is essentally the
>>>> "isolcpus=domain,..." boot command line feature at run time.
>>>>
>>>> The housekeeping cpumask update requires flushing a number of different
>>>> workqueues which may not be safe with cpus_read_lock() held as the
>>>> workqueue flushing code may acquire cpus_read_lock() or acquiring locks
>>>> which have locking dependency with cpus_read_lock() down the chain. Below
>>>> is an example of such circular locking problem.
>>>>
>>>>     ======================================================
>>>>     WARNING: possible circular locking dependency detected
>>>>     6.18.0-test+ #2 Tainted: G S
>>>>     ------------------------------------------------------
>>>>     test_cpuset_prs/10971 is trying to acquire lock:
>>>>     ffff888112ba4958 ((wq_completion)sync_wq){+.+.}-{0:0}, at:
>>>> touch_wq_lockdep_map+0x7a/0x180
>>>>
>>>>     but task is already holding lock:
>>>>     ffffffffae47f450 (cpuset_mutex){+.+.}-{4:4}, at:
>>>> cpuset_partition_write+0x85/0x130
>>>>
>>>>     which lock already depends on the new lock.
>>>>
>>>>     the existing dependency chain (in reverse order) is:
>>>>     -> #4 (cpuset_mutex){+.+.}-{4:4}:
>>>>     -> #3 (cpu_hotplug_lock){++++}-{0:0}:
>>>>     -> #2 (rtnl_mutex){+.+.}-{4:4}:
>>>>     -> #1 ((work_completion)(&arg.work)){+.+.}-{0:0}:
>>>>     -> #0 ((wq_completion)sync_wq){+.+.}-{0:0}:
>>>>
>>>>     Chain exists of:
>>>>       (wq_completion)sync_wq --> cpu_hotplug_lock --> cpuset_mutex
>>>>
>>>>     5 locks held by test_cpuset_prs/10971:
>>>>      #0: ffff88816810e440 (sb_writers#7){.+.+}-{0:0}, at: ksys_write+0xf9/0x1d0
>>>>      #1: ffff8891ab620890 (&of->mutex#2){+.+.}-{4:4}, at:
>>>> kernfs_fop_write_iter+0x260/0x5f0
>>>>      #2: ffff8890a78b83e8 (kn->active#187){.+.+}-{0:0}, at:
>>>> kernfs_fop_write_iter+0x2b6/0x5f0
>>>>      #3: ffffffffadf32900 (cpu_hotplug_lock){++++}-{0:0}, at:
>>>> cpuset_partition_write+0x77/0x130
>>>>      #4: ffffffffae47f450 (cpuset_mutex){+.+.}-{4:4}, at:
>>>> cpuset_partition_write+0x85/0x130
>>>>
>>>>     Call Trace:
>>>>      <TASK>
>>>>        :
>>>>      touch_wq_lockdep_map+0x93/0x180
>>>>      __flush_workqueue+0x111/0x10b0
>>>>      housekeeping_update+0x12d/0x2d0
>>>>      update_parent_effective_cpumask+0x595/0x2440
>>>>      update_prstate+0x89d/0xce0
>>>>      cpuset_partition_write+0xc5/0x130
>>>>      cgroup_file_write+0x1a5/0x680
>>>>      kernfs_fop_write_iter+0x3df/0x5f0
>>>>      vfs_write+0x525/0xfd0
>>>>      ksys_write+0xf9/0x1d0
>>>>      do_syscall_64+0x95/0x520
>>>>      entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>>>
>>>> To avoid such a circular locking dependency problem, we have to
>>>> call housekeeping_update() without holding the cpus_read_lock()
>>>> and cpuset_mutex. One way to do that is to introduce a new top level
>>>> isolcpus_update_mutex which will be acquired first if the set of isolated
>>>> CPUs may have to be updated. This new isolcpus_update_mutex will provide
>>>> the need mutual exclusion without the need to hold cpus_read_lock().
>>>>
>>>> As cpus_read_lock() is now no longer held when
>>>> tmigr_isolated_exclude_cpumask() is called, it needs to acquire it
>>>> directly.
>>>>
>>>> The lockdep_is_cpuset_held() is also updated to check the new
>>>> isolcpus_update_mutex.
>>>>
>>> I worry about the issue:
>>>
>>> CPU1                CPU2
>>> rmdir
>>> css->ss->css_killed(css);
>>> cpuset_css_killed
>>>                  __update_isolation_cpumasks
>>>                  cpuset_full_unlock
>>> css->flags |= CSS_DYING;
>>> css_clear_dir(css);
>>> ...
>>> // offline and free do not
>>> // get isolcpus_update_mutex
>>> cpuset_css_offline
>>> cpuset_css_free
>>>                  cpuset_full_lock
>>>                  ...
>>>                  // UAF?
>>>
> Hi, Longman,
>
> In this patch, I noticed that cpuset_css_offline and cpuset_css_free do not
> acquire the isolcpus_update_mutex. This could potentially lead to a UAF issue.
>
>> That is the reason why I add a new top-level isolcpus_update_mutex.
>> cpuset_css_killed() and the update_isolation_cpumasks()'s unlock/lock sequence
>> will have to acquire this isolcpus_update_mutex first.
>>
> However, simply adding isolcpus_update_mutex to cpuset_css_killed and
> update_isolation_cpumasks may not be sufficient.
>
> As I mentioned, the path that calls __update_isolation_cpumasks may first
> acquire isolcpus_update_mutex and cpuset_full_lock, but once cpuset_css_killed
> is completed, it will release the “full” lock and then attempt to reacquire it
> later. During this intermediate period, the cpuset may have already been freed,
> because cpuset_css_offline and cpuset_css_free do not currently acquire the
> isolcpus_update_mutex.

You are right that acquisition of the new isolcpus_update_mutex should 
be in all the places where cpuset_full_lock() is acquired. Will update 
the patch to do that. That should eliminate the risk.

Cheers,
Longman