On 2026/1/30 9:35, Waiman Long wrote:
> 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.
>
I suggest that putting isolcpus_update_mutex into cpuset_full_lock, since this
function means that all the locks needed have been acquired.
void cpuset_full_lock(void)
{
mutex_lock(&isolcpus_update_mutex);
cpus_read_lock();
mutex_lock(&cpuset_mutex);
}
void cpuset_full_unlock(void)
{
mutex_unlock(&cpuset_mutex);
cpus_read_unlock();
mutex_unlock(&isolcpus_update_mutex);
}
In the __update_isolation_cpumasks function, we can pair:
```
...
mutex_unlock(&cpuset_mutex);
cpus_read_unlock();
... Actions
cpus_read_lock();
mutex_lock(&cpuset_mutex);
...
```
--
Best regards,
Ridong
