| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f5adfdc9-4972-4437-8cc9-843b28e7414c@redhat.com>
Date: Thu, 29 Jan 2026 22:53:10 -0500
From: Waiman Long <llong@...hat.com>
To: Chen Ridong <chenridong@...weicloud.com>, Waiman Long <llong@...hat.com>,
Tejun Heo <tj@...nel.org>, Johannes Weiner <hannes@...xchg.org>,
Michal Koutný <mkoutny@...e.com>,
Ingo Molnar <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>,
Juri Lelli <juri.lelli@...hat.com>,
Vincent Guittot <vincent.guittot@...aro.org>,
Steven Rostedt <rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>,
Mel Gorman <mgorman@...e.de>, Valentin Schneider <vschneid@...hat.com>,
Anna-Maria Behnsen <anna-maria@...utronix.de>,
Frederic Weisbecker <frederic@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>, Shuah Khan <shuah@...nel.org>
Cc: cgroups@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org
Subject: Re: [PATCH/for-next 2/2] cgroup/cpuset: Introduce a new top level
isolcpus_update_mutex
On 1/29/26 8:42 PM, Chen Ridong wrote:
>
> 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);
> }
That is what I had done.
Cheers,
Longman
>
> In the __update_isolation_cpumasks function, we can pair:
>
> ```
> ...
> mutex_unlock(&cpuset_mutex);
> cpus_read_unlock();
> ... Actions
> cpus_read_lock();
> mutex_lock(&cpuset_mutex);
> ...
> ```
>
Powered by blists - more mailing lists