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.
Signed-off-by: Waiman Long <longman@redhat.com>
---
kernel/cgroup/cpuset.c | 79 ++++++++++++++++++++++++-----------
kernel/sched/isolation.c | 4 +-
kernel/time/timer_migration.c | 3 +-
3 files changed, 57 insertions(+), 29 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 98c7cb732206..96390ceb5122 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -78,7 +78,7 @@ static cpumask_var_t subpartitions_cpus;
static cpumask_var_t isolated_cpus;
/*
- * isolated_cpus updating flag (protected by cpuset_mutex)
+ * isolated_cpus updating flag (protected by isolcpus_update_mutex)
* Set if isolated_cpus is going to be updated in the current
* cpuset_mutex crtical section.
*/
@@ -223,29 +223,46 @@ struct cpuset top_cpuset = {
};
/*
- * There are two global locks guarding cpuset structures - cpuset_mutex and
- * callback_lock. The cpuset code uses only cpuset_mutex. Other kernel
- * subsystems can use cpuset_lock()/cpuset_unlock() to prevent change to cpuset
- * structures. Note that cpuset_mutex needs to be a mutex as it is used in
- * paths that rely on priority inheritance (e.g. scheduler - on RT) for
- * correctness.
+ * CPUSET Locking Convention
+ * -------------------------
*
- * A task must hold both locks to modify cpusets. If a task holds
- * cpuset_mutex, it blocks others, ensuring that it is the only task able to
- * also acquire callback_lock and be able to modify cpusets. It can perform
- * various checks on the cpuset structure first, knowing nothing will change.
- * It can also allocate memory while just holding cpuset_mutex. While it is
- * performing these checks, various callback routines can briefly acquire
- * callback_lock to query cpusets. Once it is ready to make the changes, it
- * takes callback_lock, blocking everyone else.
+ * Below are the three global locks guarding cpuset structures in lock
+ * acquisition order:
+ * - isolcpus_update_mutex (optional)
+ * - cpu_hotplug_lock (cpus_read_lock/cpus_write_lock)
+ * - cpuset_mutex
+ * - callback_lock (raw spinlock)
*
- * Calls to the kernel memory allocator can not be made while holding
- * callback_lock, as that would risk double tripping on callback_lock
- * from one of the callbacks into the cpuset code from within
- * __alloc_pages().
+ * The first isolcpus_update_mutex should only be held if the existing set of
+ * isolated CPUs (in isolated partition) or any of the partition states may be
+ * changed when some cpuset control files are being written into. Otherwise,
+ * it can be skipped. Holding isolcpus_update_mutex/cpus_read_lock or
+ * cpus_write_lock will ensure mutual exclusion of isolated_cpus update.
*
- * If a task is only holding callback_lock, then it has read-only
- * access to cpusets.
+ * As cpuset will now indirectly flush a number of different workqueues in
+ * housekeeping_update() when the set of isolated CPUs is going to be changed,
+ * it may not be safe from the circular locking perspective to hold the
+ * cpus_read_lock. So cpuset_full_lock() will be released before calling
+ * housekeeping_update() and re-acquired afterward.
+ *
+ * A task must hold all the remaining three locks to modify externally visible
+ * or used fields of cpusets, though some of the internally used cpuset fields
+ * can be modified by holding cpu_hotplug_lock and cpuset_mutex only. If only
+ * reliable read access of the externally used fields are needed, a task can
+ * hold either cpuset_mutex or callback_lock.
+ *
+ * If a task holds cpu_hotplug_lock and cpuset_mutex, it blocks others,
+ * ensuring that it is the only task able to also acquire callback_lock and
+ * be able to modify cpusets. It can perform various checks on the cpuset
+ * structure first, knowing nothing will change. It can also allocate memory
+ * without holding callback_lock. While it is performing these checks, various
+ * callback routines can briefly acquire callback_lock to query cpusets. Once
+ * it is ready to make the changes, it takes callback_lock, blocking everyone
+ * else.
+ *
+ * Calls to the kernel memory allocator cannot be made while holding
+ * callback_lock which is a spinlock, as the memory allocator may sleep or
+ * call back into cpuset code and acquire callback_lock.
*
* Now, the task_struct fields mems_allowed and mempolicy may be changed
* by other task, we use alloc_lock in the task_struct fields to protect
@@ -256,6 +273,7 @@ struct cpuset top_cpuset = {
* cpumasks and nodemasks.
*/
+static DEFINE_MUTEX(isolcpus_update_mutex);
static DEFINE_MUTEX(cpuset_mutex);
/**
@@ -302,7 +320,7 @@ void cpuset_full_unlock(void)
#ifdef CONFIG_LOCKDEP
bool lockdep_is_cpuset_held(void)
{
- return lockdep_is_held(&cpuset_mutex);
+ return lockdep_is_held(&isolcpus_update_mutex);
}
#endif
@@ -1294,9 +1312,8 @@ static bool prstate_housekeeping_conflict(int prstate, struct cpumask *new_cpus)
static void __update_isolation_cpumasks(bool twork);
static void isolation_task_work_fn(struct callback_head *cb)
{
- cpuset_full_lock();
+ guard(mutex)(&isolcpus_update_mutex);
__update_isolation_cpumasks(true);
- cpuset_full_lock();
}
/*
@@ -1338,8 +1355,18 @@ static void __update_isolation_cpumasks(bool twork)
return;
}
+ lockdep_assert_held(&isolcpus_update_mutex);
+ /*
+ * Release cpus_read_lock & cpuset_mutex before calling
+ * housekeeping_update() and re-acquiring them afterward if not
+ * calling from task_work.
+ */
+ if (!twork)
+ cpuset_full_unlock();
ret = housekeeping_update(isolated_cpus);
WARN_ON_ONCE(ret < 0);
+ if (!twork)
+ cpuset_full_lock();
isolated_cpus_updating = false;
}
@@ -3196,6 +3223,7 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
return -EACCES;
buf = strstrip(buf);
+ mutex_lock(&isolcpus_update_mutex);
cpuset_full_lock();
if (!is_cpuset_online(cs))
goto out_unlock;
@@ -3226,6 +3254,7 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
rebuild_sched_domains_locked();
out_unlock:
cpuset_full_unlock();
+ mutex_unlock(&isolcpus_update_mutex);
if (of_cft(of)->private == FILE_MEMLIST)
schedule_flush_migrate_mm();
return retval ?: nbytes;
@@ -3329,6 +3358,7 @@ static ssize_t cpuset_partition_write(struct kernfs_open_file *of, char *buf,
else
return -EINVAL;
+ guard(mutex)(&isolcpus_update_mutex);
cpuset_full_lock();
if (is_cpuset_online(cs))
retval = update_prstate(cs, val);
@@ -3502,6 +3532,7 @@ static void cpuset_css_killed(struct cgroup_subsys_state *css)
{
struct cpuset *cs = css_cs(css);
+ guard(mutex)(&isolcpus_update_mutex);
cpuset_full_lock();
/* Reset valid partition back to member */
if (is_partition_valid(cs))
diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
index 3b725d39c06e..ef152d401fe2 100644
--- a/kernel/sched/isolation.c
+++ b/kernel/sched/isolation.c
@@ -123,8 +123,6 @@ int housekeeping_update(struct cpumask *isol_mask)
struct cpumask *trial, *old = NULL;
int err;
- lockdep_assert_cpus_held();
-
trial = kmalloc(cpumask_size(), GFP_KERNEL);
if (!trial)
return -ENOMEM;
@@ -136,7 +134,7 @@ int housekeeping_update(struct cpumask *isol_mask)
}
if (!housekeeping.flags)
- static_branch_enable_cpuslocked(&housekeeping_overridden);
+ static_branch_enable(&housekeeping_overridden);
if (housekeeping.flags & HK_FLAG_DOMAIN)
old = housekeeping_cpumask_dereference(HK_TYPE_DOMAIN);
diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c
index 6da9cd562b20..244a8d025e78 100644
--- a/kernel/time/timer_migration.c
+++ b/kernel/time/timer_migration.c
@@ -1559,8 +1559,6 @@ int tmigr_isolated_exclude_cpumask(struct cpumask *exclude_cpumask)
cpumask_var_t cpumask __free(free_cpumask_var) = CPUMASK_VAR_NULL;
int cpu;
- lockdep_assert_cpus_held();
-
if (!works)
return -ENOMEM;
if (!alloc_cpumask_var(&cpumask, GFP_KERNEL))
@@ -1570,6 +1568,7 @@ int tmigr_isolated_exclude_cpumask(struct cpumask *exclude_cpumask)
* First set previously isolated CPUs as available (unisolate).
* This cpumask contains only CPUs that switched to available now.
*/
+ guard(cpus_read_lock)();
cpumask_andnot(cpumask, cpu_online_mask, exclude_cpumask);
cpumask_andnot(cpumask, cpumask, tmigr_available_cpumask);
--
2.52.0
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?
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
> kernel/cgroup/cpuset.c | 79 ++++++++++++++++++++++++-----------
> kernel/sched/isolation.c | 4 +-
> kernel/time/timer_migration.c | 3 +-
> 3 files changed, 57 insertions(+), 29 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 98c7cb732206..96390ceb5122 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -78,7 +78,7 @@ static cpumask_var_t subpartitions_cpus;
> static cpumask_var_t isolated_cpus;
>
> /*
> - * isolated_cpus updating flag (protected by cpuset_mutex)
> + * isolated_cpus updating flag (protected by isolcpus_update_mutex)
> * Set if isolated_cpus is going to be updated in the current
> * cpuset_mutex crtical section.
> */
> @@ -223,29 +223,46 @@ struct cpuset top_cpuset = {
> };
>
> /*
> - * There are two global locks guarding cpuset structures - cpuset_mutex and
> - * callback_lock. The cpuset code uses only cpuset_mutex. Other kernel
> - * subsystems can use cpuset_lock()/cpuset_unlock() to prevent change to cpuset
> - * structures. Note that cpuset_mutex needs to be a mutex as it is used in
> - * paths that rely on priority inheritance (e.g. scheduler - on RT) for
> - * correctness.
> + * CPUSET Locking Convention
> + * -------------------------
> *
> - * A task must hold both locks to modify cpusets. If a task holds
> - * cpuset_mutex, it blocks others, ensuring that it is the only task able to
> - * also acquire callback_lock and be able to modify cpusets. It can perform
> - * various checks on the cpuset structure first, knowing nothing will change.
> - * It can also allocate memory while just holding cpuset_mutex. While it is
> - * performing these checks, various callback routines can briefly acquire
> - * callback_lock to query cpusets. Once it is ready to make the changes, it
> - * takes callback_lock, blocking everyone else.
> + * Below are the three global locks guarding cpuset structures in lock
> + * acquisition order:
> + * - isolcpus_update_mutex (optional)
> + * - cpu_hotplug_lock (cpus_read_lock/cpus_write_lock)
> + * - cpuset_mutex
> + * - callback_lock (raw spinlock)
> *
> - * Calls to the kernel memory allocator can not be made while holding
> - * callback_lock, as that would risk double tripping on callback_lock
> - * from one of the callbacks into the cpuset code from within
> - * __alloc_pages().
> + * The first isolcpus_update_mutex should only be held if the existing set of
> + * isolated CPUs (in isolated partition) or any of the partition states may be
> + * changed when some cpuset control files are being written into. Otherwise,
> + * it can be skipped. Holding isolcpus_update_mutex/cpus_read_lock or
> + * cpus_write_lock will ensure mutual exclusion of isolated_cpus update.
> *
> - * If a task is only holding callback_lock, then it has read-only
> - * access to cpusets.
> + * As cpuset will now indirectly flush a number of different workqueues in
> + * housekeeping_update() when the set of isolated CPUs is going to be changed,
> + * it may not be safe from the circular locking perspective to hold the
> + * cpus_read_lock. So cpuset_full_lock() will be released before calling
> + * housekeeping_update() and re-acquired afterward.
> + *
> + * A task must hold all the remaining three locks to modify externally visible
> + * or used fields of cpusets, though some of the internally used cpuset fields
> + * can be modified by holding cpu_hotplug_lock and cpuset_mutex only. If only
> + * reliable read access of the externally used fields are needed, a task can
> + * hold either cpuset_mutex or callback_lock.
> + *
> + * If a task holds cpu_hotplug_lock and cpuset_mutex, it blocks others,
> + * ensuring that it is the only task able to also acquire callback_lock and
> + * be able to modify cpusets. It can perform various checks on the cpuset
> + * structure first, knowing nothing will change. It can also allocate memory
> + * without holding callback_lock. While it is performing these checks, various
> + * callback routines can briefly acquire callback_lock to query cpusets. Once
> + * it is ready to make the changes, it takes callback_lock, blocking everyone
> + * else.
> + *
> + * Calls to the kernel memory allocator cannot be made while holding
> + * callback_lock which is a spinlock, as the memory allocator may sleep or
> + * call back into cpuset code and acquire callback_lock.
> *
> * Now, the task_struct fields mems_allowed and mempolicy may be changed
> * by other task, we use alloc_lock in the task_struct fields to protect
> @@ -256,6 +273,7 @@ struct cpuset top_cpuset = {
> * cpumasks and nodemasks.
> */
>
> +static DEFINE_MUTEX(isolcpus_update_mutex);
> static DEFINE_MUTEX(cpuset_mutex);
>
> /**
> @@ -302,7 +320,7 @@ void cpuset_full_unlock(void)
> #ifdef CONFIG_LOCKDEP
> bool lockdep_is_cpuset_held(void)
> {
> - return lockdep_is_held(&cpuset_mutex);
> + return lockdep_is_held(&isolcpus_update_mutex);
> }
> #endif
>
> @@ -1294,9 +1312,8 @@ static bool prstate_housekeeping_conflict(int prstate, struct cpumask *new_cpus)
> static void __update_isolation_cpumasks(bool twork);
> static void isolation_task_work_fn(struct callback_head *cb)
> {
> - cpuset_full_lock();
> + guard(mutex)(&isolcpus_update_mutex);
> __update_isolation_cpumasks(true);
> - cpuset_full_lock();
> }
>
> /*
> @@ -1338,8 +1355,18 @@ static void __update_isolation_cpumasks(bool twork)
> return;
> }
>
> + lockdep_assert_held(&isolcpus_update_mutex);
> + /*
> + * Release cpus_read_lock & cpuset_mutex before calling
> + * housekeeping_update() and re-acquiring them afterward if not
> + * calling from task_work.
> + */
> + if (!twork)
> + cpuset_full_unlock();
> ret = housekeeping_update(isolated_cpus);
> WARN_ON_ONCE(ret < 0);
> + if (!twork)
> + cpuset_full_lock();
>
> isolated_cpus_updating = false;
> }
> @@ -3196,6 +3223,7 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
> return -EACCES;
>
> buf = strstrip(buf);
> + mutex_lock(&isolcpus_update_mutex);
> cpuset_full_lock();
> if (!is_cpuset_online(cs))
> goto out_unlock;
> @@ -3226,6 +3254,7 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
> rebuild_sched_domains_locked();
> out_unlock:
> cpuset_full_unlock();
> + mutex_unlock(&isolcpus_update_mutex);
> if (of_cft(of)->private == FILE_MEMLIST)
> schedule_flush_migrate_mm();
> return retval ?: nbytes;
> @@ -3329,6 +3358,7 @@ static ssize_t cpuset_partition_write(struct kernfs_open_file *of, char *buf,
> else
> return -EINVAL;
>
> + guard(mutex)(&isolcpus_update_mutex);
> cpuset_full_lock();
> if (is_cpuset_online(cs))
> retval = update_prstate(cs, val);
> @@ -3502,6 +3532,7 @@ static void cpuset_css_killed(struct cgroup_subsys_state *css)
> {
> struct cpuset *cs = css_cs(css);
>
> + guard(mutex)(&isolcpus_update_mutex);
> cpuset_full_lock();
> /* Reset valid partition back to member */
> if (is_partition_valid(cs))
> diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
> index 3b725d39c06e..ef152d401fe2 100644
> --- a/kernel/sched/isolation.c
> +++ b/kernel/sched/isolation.c
> @@ -123,8 +123,6 @@ int housekeeping_update(struct cpumask *isol_mask)
> struct cpumask *trial, *old = NULL;
> int err;
>
> - lockdep_assert_cpus_held();
> -
> trial = kmalloc(cpumask_size(), GFP_KERNEL);
> if (!trial)
> return -ENOMEM;
> @@ -136,7 +134,7 @@ int housekeeping_update(struct cpumask *isol_mask)
> }
>
> if (!housekeeping.flags)
> - static_branch_enable_cpuslocked(&housekeeping_overridden);
> + static_branch_enable(&housekeeping_overridden);
>
> if (housekeeping.flags & HK_FLAG_DOMAIN)
> old = housekeeping_cpumask_dereference(HK_TYPE_DOMAIN);
> diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c
> index 6da9cd562b20..244a8d025e78 100644
> --- a/kernel/time/timer_migration.c
> +++ b/kernel/time/timer_migration.c
> @@ -1559,8 +1559,6 @@ int tmigr_isolated_exclude_cpumask(struct cpumask *exclude_cpumask)
> cpumask_var_t cpumask __free(free_cpumask_var) = CPUMASK_VAR_NULL;
> int cpu;
>
> - lockdep_assert_cpus_held();
> -
> if (!works)
> return -ENOMEM;
> if (!alloc_cpumask_var(&cpumask, GFP_KERNEL))
> @@ -1570,6 +1568,7 @@ int tmigr_isolated_exclude_cpumask(struct cpumask *exclude_cpumask)
> * First set previously isolated CPUs as available (unisolate).
> * This cpumask contains only CPUs that switched to available now.
> */
> + guard(cpus_read_lock)();
> cpumask_andnot(cpumask, cpu_online_mask, exclude_cpumask);
> cpumask_andnot(cpumask, cpumask, tmigr_available_cpumask);
>
--
Best regards,
Ridong
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?
>
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.
As long as all the possible paths (except CPU hotplug) that can call
into update_isolation_cpumasks() has acquired isolcpus_update_mutex, it
will block cpuset_css_killed() from completing. Note that I add a
"lockdep_assert_held(&isolcpus_update_mutex);" in
update_isolation_cpumasks().
Cheers,
Longman
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>> kernel/cgroup/cpuset.c | 79 ++++++++++++++++++++++++-----------
>> kernel/sched/isolation.c | 4 +-
>> kernel/time/timer_migration.c | 3 +-
>> 3 files changed, 57 insertions(+), 29 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 98c7cb732206..96390ceb5122 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -78,7 +78,7 @@ static cpumask_var_t subpartitions_cpus;
>> static cpumask_var_t isolated_cpus;
>>
>> /*
>> - * isolated_cpus updating flag (protected by cpuset_mutex)
>> + * isolated_cpus updating flag (protected by isolcpus_update_mutex)
>> * Set if isolated_cpus is going to be updated in the current
>> * cpuset_mutex crtical section.
>> */
>> @@ -223,29 +223,46 @@ struct cpuset top_cpuset = {
>> };
>>
>> /*
>> - * There are two global locks guarding cpuset structures - cpuset_mutex and
>> - * callback_lock. The cpuset code uses only cpuset_mutex. Other kernel
>> - * subsystems can use cpuset_lock()/cpuset_unlock() to prevent change to cpuset
>> - * structures. Note that cpuset_mutex needs to be a mutex as it is used in
>> - * paths that rely on priority inheritance (e.g. scheduler - on RT) for
>> - * correctness.
>> + * CPUSET Locking Convention
>> + * -------------------------
>> *
>> - * A task must hold both locks to modify cpusets. If a task holds
>> - * cpuset_mutex, it blocks others, ensuring that it is the only task able to
>> - * also acquire callback_lock and be able to modify cpusets. It can perform
>> - * various checks on the cpuset structure first, knowing nothing will change.
>> - * It can also allocate memory while just holding cpuset_mutex. While it is
>> - * performing these checks, various callback routines can briefly acquire
>> - * callback_lock to query cpusets. Once it is ready to make the changes, it
>> - * takes callback_lock, blocking everyone else.
>> + * Below are the three global locks guarding cpuset structures in lock
>> + * acquisition order:
>> + * - isolcpus_update_mutex (optional)
>> + * - cpu_hotplug_lock (cpus_read_lock/cpus_write_lock)
>> + * - cpuset_mutex
>> + * - callback_lock (raw spinlock)
>> *
>> - * Calls to the kernel memory allocator can not be made while holding
>> - * callback_lock, as that would risk double tripping on callback_lock
>> - * from one of the callbacks into the cpuset code from within
>> - * __alloc_pages().
>> + * The first isolcpus_update_mutex should only be held if the existing set of
>> + * isolated CPUs (in isolated partition) or any of the partition states may be
>> + * changed when some cpuset control files are being written into. Otherwise,
>> + * it can be skipped. Holding isolcpus_update_mutex/cpus_read_lock or
>> + * cpus_write_lock will ensure mutual exclusion of isolated_cpus update.
>> *
>> - * If a task is only holding callback_lock, then it has read-only
>> - * access to cpusets.
>> + * As cpuset will now indirectly flush a number of different workqueues in
>> + * housekeeping_update() when the set of isolated CPUs is going to be changed,
>> + * it may not be safe from the circular locking perspective to hold the
>> + * cpus_read_lock. So cpuset_full_lock() will be released before calling
>> + * housekeeping_update() and re-acquired afterward.
>> + *
>> + * A task must hold all the remaining three locks to modify externally visible
>> + * or used fields of cpusets, though some of the internally used cpuset fields
>> + * can be modified by holding cpu_hotplug_lock and cpuset_mutex only. If only
>> + * reliable read access of the externally used fields are needed, a task can
>> + * hold either cpuset_mutex or callback_lock.
>> + *
>> + * If a task holds cpu_hotplug_lock and cpuset_mutex, it blocks others,
>> + * ensuring that it is the only task able to also acquire callback_lock and
>> + * be able to modify cpusets. It can perform various checks on the cpuset
>> + * structure first, knowing nothing will change. It can also allocate memory
>> + * without holding callback_lock. While it is performing these checks, various
>> + * callback routines can briefly acquire callback_lock to query cpusets. Once
>> + * it is ready to make the changes, it takes callback_lock, blocking everyone
>> + * else.
>> + *
>> + * Calls to the kernel memory allocator cannot be made while holding
>> + * callback_lock which is a spinlock, as the memory allocator may sleep or
>> + * call back into cpuset code and acquire callback_lock.
>> *
>> * Now, the task_struct fields mems_allowed and mempolicy may be changed
>> * by other task, we use alloc_lock in the task_struct fields to protect
>> @@ -256,6 +273,7 @@ struct cpuset top_cpuset = {
>> * cpumasks and nodemasks.
>> */
>>
>> +static DEFINE_MUTEX(isolcpus_update_mutex);
>> static DEFINE_MUTEX(cpuset_mutex);
>>
>> /**
>> @@ -302,7 +320,7 @@ void cpuset_full_unlock(void)
>> #ifdef CONFIG_LOCKDEP
>> bool lockdep_is_cpuset_held(void)
>> {
>> - return lockdep_is_held(&cpuset_mutex);
>> + return lockdep_is_held(&isolcpus_update_mutex);
>> }
>> #endif
>>
>> @@ -1294,9 +1312,8 @@ static bool prstate_housekeeping_conflict(int prstate, struct cpumask *new_cpus)
>> static void __update_isolation_cpumasks(bool twork);
>> static void isolation_task_work_fn(struct callback_head *cb)
>> {
>> - cpuset_full_lock();
>> + guard(mutex)(&isolcpus_update_mutex);
>> __update_isolation_cpumasks(true);
>> - cpuset_full_lock();
>> }
>>
>> /*
>> @@ -1338,8 +1355,18 @@ static void __update_isolation_cpumasks(bool twork)
>> return;
>> }
>>
>> + lockdep_assert_held(&isolcpus_update_mutex);
>> + /*
>> + * Release cpus_read_lock & cpuset_mutex before calling
>> + * housekeeping_update() and re-acquiring them afterward if not
>> + * calling from task_work.
>> + */
>> + if (!twork)
>> + cpuset_full_unlock();
>> ret = housekeeping_update(isolated_cpus);
>> WARN_ON_ONCE(ret < 0);
>> + if (!twork)
>> + cpuset_full_lock();
>>
>> isolated_cpus_updating = false;
>> }
>> @@ -3196,6 +3223,7 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
>> return -EACCES;
>>
>> buf = strstrip(buf);
>> + mutex_lock(&isolcpus_update_mutex);
>> cpuset_full_lock();
>> if (!is_cpuset_online(cs))
>> goto out_unlock;
>> @@ -3226,6 +3254,7 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
>> rebuild_sched_domains_locked();
>> out_unlock:
>> cpuset_full_unlock();
>> + mutex_unlock(&isolcpus_update_mutex);
>> if (of_cft(of)->private == FILE_MEMLIST)
>> schedule_flush_migrate_mm();
>> return retval ?: nbytes;
>> @@ -3329,6 +3358,7 @@ static ssize_t cpuset_partition_write(struct kernfs_open_file *of, char *buf,
>> else
>> return -EINVAL;
>>
>> + guard(mutex)(&isolcpus_update_mutex);
>> cpuset_full_lock();
>> if (is_cpuset_online(cs))
>> retval = update_prstate(cs, val);
>> @@ -3502,6 +3532,7 @@ static void cpuset_css_killed(struct cgroup_subsys_state *css)
>> {
>> struct cpuset *cs = css_cs(css);
>>
>> + guard(mutex)(&isolcpus_update_mutex);
>> cpuset_full_lock();
>> /* Reset valid partition back to member */
>> if (is_partition_valid(cs))
>> diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
>> index 3b725d39c06e..ef152d401fe2 100644
>> --- a/kernel/sched/isolation.c
>> +++ b/kernel/sched/isolation.c
>> @@ -123,8 +123,6 @@ int housekeeping_update(struct cpumask *isol_mask)
>> struct cpumask *trial, *old = NULL;
>> int err;
>>
>> - lockdep_assert_cpus_held();
>> -
>> trial = kmalloc(cpumask_size(), GFP_KERNEL);
>> if (!trial)
>> return -ENOMEM;
>> @@ -136,7 +134,7 @@ int housekeeping_update(struct cpumask *isol_mask)
>> }
>>
>> if (!housekeeping.flags)
>> - static_branch_enable_cpuslocked(&housekeeping_overridden);
>> + static_branch_enable(&housekeeping_overridden);
>>
>> if (housekeeping.flags & HK_FLAG_DOMAIN)
>> old = housekeeping_cpumask_dereference(HK_TYPE_DOMAIN);
>> diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c
>> index 6da9cd562b20..244a8d025e78 100644
>> --- a/kernel/time/timer_migration.c
>> +++ b/kernel/time/timer_migration.c
>> @@ -1559,8 +1559,6 @@ int tmigr_isolated_exclude_cpumask(struct cpumask *exclude_cpumask)
>> cpumask_var_t cpumask __free(free_cpumask_var) = CPUMASK_VAR_NULL;
>> int cpu;
>>
>> - lockdep_assert_cpus_held();
>> -
>> if (!works)
>> return -ENOMEM;
>> if (!alloc_cpumask_var(&cpumask, GFP_KERNEL))
>> @@ -1570,6 +1568,7 @@ int tmigr_isolated_exclude_cpumask(struct cpumask *exclude_cpumask)
>> * First set previously isolated CPUs as available (unisolate).
>> * This cpumask contains only CPUs that switched to available now.
>> */
>> + guard(cpus_read_lock)();
>> cpumask_andnot(cpumask, cpu_online_mask, exclude_cpumask);
>> cpumask_andnot(cpumask, cpumask, tmigr_available_cpumask);
>>
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.
> As long as all the possible paths (except CPU hotplug) that can call into
> update_isolation_cpumasks() has acquired isolcpus_update_mutex, it will block
> cpuset_css_killed() from completing. Note that I add a
> "lockdep_assert_held(&isolcpus_update_mutex);" in update_isolation_cpumasks().
>
--
Best regards,
Ridong
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
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
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);
> ...
> ```
>
On 2026/1/30 11:53, Waiman Long wrote:
> 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.
>
Great.
--
Best regards,
Ridong
On 2026/1/29 16:01, 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().
>>
When I reviewed Frederic's patches, I concerned about this issue. However, I was
not certain whether any flush worker would need to acquire cpu_hotplug_lock or
cpuset_mutex.
Despite this warning, I do not understand how wq_completion would need to
acquire cpu_hotplug_lock and cpuset_mutex.
The reason I want to understand how wq_completion acquires cpu_hotplug_lock or
cpuset_mutex is to determine whether isolcpus_update_mutex is truly necessary.
As I mentioned in my previous email, I am concerned about a potential
use-after-free (UAF) issue, which might imply that isolcpus_update_mutex is
required in most places that currently acquire cpuset_mutex, with the possible
exception of the hotplug path?
>> 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?
>
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>> kernel/cgroup/cpuset.c | 79 ++++++++++++++++++++++++-----------
>> kernel/sched/isolation.c | 4 +-
>> kernel/time/timer_migration.c | 3 +-
>> 3 files changed, 57 insertions(+), 29 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 98c7cb732206..96390ceb5122 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -78,7 +78,7 @@ static cpumask_var_t subpartitions_cpus;
>> static cpumask_var_t isolated_cpus;
>>
>> /*
>> - * isolated_cpus updating flag (protected by cpuset_mutex)
>> + * isolated_cpus updating flag (protected by isolcpus_update_mutex)
>> * Set if isolated_cpus is going to be updated in the current
>> * cpuset_mutex crtical section.
>> */
>> @@ -223,29 +223,46 @@ struct cpuset top_cpuset = {
>> };
>>
>> /*
>> - * There are two global locks guarding cpuset structures - cpuset_mutex and
>> - * callback_lock. The cpuset code uses only cpuset_mutex. Other kernel
>> - * subsystems can use cpuset_lock()/cpuset_unlock() to prevent change to cpuset
>> - * structures. Note that cpuset_mutex needs to be a mutex as it is used in
>> - * paths that rely on priority inheritance (e.g. scheduler - on RT) for
>> - * correctness.
>> + * CPUSET Locking Convention
>> + * -------------------------
>> *
>> - * A task must hold both locks to modify cpusets. If a task holds
>> - * cpuset_mutex, it blocks others, ensuring that it is the only task able to
>> - * also acquire callback_lock and be able to modify cpusets. It can perform
>> - * various checks on the cpuset structure first, knowing nothing will change.
>> - * It can also allocate memory while just holding cpuset_mutex. While it is
>> - * performing these checks, various callback routines can briefly acquire
>> - * callback_lock to query cpusets. Once it is ready to make the changes, it
>> - * takes callback_lock, blocking everyone else.
>> + * Below are the three global locks guarding cpuset structures in lock
>> + * acquisition order:
>> + * - isolcpus_update_mutex (optional)
>> + * - cpu_hotplug_lock (cpus_read_lock/cpus_write_lock)
>> + * - cpuset_mutex
>> + * - callback_lock (raw spinlock)
>> *
>> - * Calls to the kernel memory allocator can not be made while holding
>> - * callback_lock, as that would risk double tripping on callback_lock
>> - * from one of the callbacks into the cpuset code from within
>> - * __alloc_pages().
>> + * The first isolcpus_update_mutex should only be held if the existing set of
>> + * isolated CPUs (in isolated partition) or any of the partition states may be
>> + * changed when some cpuset control files are being written into. Otherwise,
>> + * it can be skipped. Holding isolcpus_update_mutex/cpus_read_lock or
>> + * cpus_write_lock will ensure mutual exclusion of isolated_cpus update.
>> *
>> - * If a task is only holding callback_lock, then it has read-only
>> - * access to cpusets.
>> + * As cpuset will now indirectly flush a number of different workqueues in
>> + * housekeeping_update() when the set of isolated CPUs is going to be changed,
>> + * it may not be safe from the circular locking perspective to hold the
>> + * cpus_read_lock. So cpuset_full_lock() will be released before calling
>> + * housekeeping_update() and re-acquired afterward.
>> + *
>> + * A task must hold all the remaining three locks to modify externally visible
>> + * or used fields of cpusets, though some of the internally used cpuset fields
>> + * can be modified by holding cpu_hotplug_lock and cpuset_mutex only. If only
>> + * reliable read access of the externally used fields are needed, a task can
>> + * hold either cpuset_mutex or callback_lock.
>> + *
>> + * If a task holds cpu_hotplug_lock and cpuset_mutex, it blocks others,
>> + * ensuring that it is the only task able to also acquire callback_lock and
>> + * be able to modify cpusets. It can perform various checks on the cpuset
>> + * structure first, knowing nothing will change. It can also allocate memory
>> + * without holding callback_lock. While it is performing these checks, various
>> + * callback routines can briefly acquire callback_lock to query cpusets. Once
>> + * it is ready to make the changes, it takes callback_lock, blocking everyone
>> + * else.
>> + *
>> + * Calls to the kernel memory allocator cannot be made while holding
>> + * callback_lock which is a spinlock, as the memory allocator may sleep or
>> + * call back into cpuset code and acquire callback_lock.
>> *
>> * Now, the task_struct fields mems_allowed and mempolicy may be changed
>> * by other task, we use alloc_lock in the task_struct fields to protect
>> @@ -256,6 +273,7 @@ struct cpuset top_cpuset = {
>> * cpumasks and nodemasks.
>> */
>>
>> +static DEFINE_MUTEX(isolcpus_update_mutex);
>> static DEFINE_MUTEX(cpuset_mutex);
>>
>> /**
>> @@ -302,7 +320,7 @@ void cpuset_full_unlock(void)
>> #ifdef CONFIG_LOCKDEP
>> bool lockdep_is_cpuset_held(void)
>> {
>> - return lockdep_is_held(&cpuset_mutex);
>> + return lockdep_is_held(&isolcpus_update_mutex);
>> }
>> #endif
>>
>> @@ -1294,9 +1312,8 @@ static bool prstate_housekeeping_conflict(int prstate, struct cpumask *new_cpus)
>> static void __update_isolation_cpumasks(bool twork);
>> static void isolation_task_work_fn(struct callback_head *cb)
>> {
>> - cpuset_full_lock();
>> + guard(mutex)(&isolcpus_update_mutex);
>> __update_isolation_cpumasks(true);
>> - cpuset_full_lock();
>> }
>>
>> /*
>> @@ -1338,8 +1355,18 @@ static void __update_isolation_cpumasks(bool twork)
>> return;
>> }
>>
>> + lockdep_assert_held(&isolcpus_update_mutex);
>> + /*
>> + * Release cpus_read_lock & cpuset_mutex before calling
>> + * housekeeping_update() and re-acquiring them afterward if not
>> + * calling from task_work.
>> + */
>> + if (!twork)
>> + cpuset_full_unlock();
>> ret = housekeeping_update(isolated_cpus);
>> WARN_ON_ONCE(ret < 0);
>> + if (!twork)
>> + cpuset_full_lock();
>>
>> isolated_cpus_updating = false;
>> }
>> @@ -3196,6 +3223,7 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
>> return -EACCES;
>>
>> buf = strstrip(buf);
>> + mutex_lock(&isolcpus_update_mutex);
>> cpuset_full_lock();
>> if (!is_cpuset_online(cs))
>> goto out_unlock;
>> @@ -3226,6 +3254,7 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
>> rebuild_sched_domains_locked();
>> out_unlock:
>> cpuset_full_unlock();
>> + mutex_unlock(&isolcpus_update_mutex);
>> if (of_cft(of)->private == FILE_MEMLIST)
>> schedule_flush_migrate_mm();
>> return retval ?: nbytes;
>> @@ -3329,6 +3358,7 @@ static ssize_t cpuset_partition_write(struct kernfs_open_file *of, char *buf,
>> else
>> return -EINVAL;
>>
>> + guard(mutex)(&isolcpus_update_mutex);
>> cpuset_full_lock();
>> if (is_cpuset_online(cs))
>> retval = update_prstate(cs, val);
>> @@ -3502,6 +3532,7 @@ static void cpuset_css_killed(struct cgroup_subsys_state *css)
>> {
>> struct cpuset *cs = css_cs(css);
>>
>> + guard(mutex)(&isolcpus_update_mutex);
>> cpuset_full_lock();
>> /* Reset valid partition back to member */
>> if (is_partition_valid(cs))
>> diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
>> index 3b725d39c06e..ef152d401fe2 100644
>> --- a/kernel/sched/isolation.c
>> +++ b/kernel/sched/isolation.c
>> @@ -123,8 +123,6 @@ int housekeeping_update(struct cpumask *isol_mask)
>> struct cpumask *trial, *old = NULL;
>> int err;
>>
>> - lockdep_assert_cpus_held();
>> -
>> trial = kmalloc(cpumask_size(), GFP_KERNEL);
>> if (!trial)
>> return -ENOMEM;
>> @@ -136,7 +134,7 @@ int housekeeping_update(struct cpumask *isol_mask)
>> }
>>
>> if (!housekeeping.flags)
>> - static_branch_enable_cpuslocked(&housekeeping_overridden);
>> + static_branch_enable(&housekeeping_overridden);
>>
>> if (housekeeping.flags & HK_FLAG_DOMAIN)
>> old = housekeeping_cpumask_dereference(HK_TYPE_DOMAIN);
>> diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c
>> index 6da9cd562b20..244a8d025e78 100644
>> --- a/kernel/time/timer_migration.c
>> +++ b/kernel/time/timer_migration.c
>> @@ -1559,8 +1559,6 @@ int tmigr_isolated_exclude_cpumask(struct cpumask *exclude_cpumask)
>> cpumask_var_t cpumask __free(free_cpumask_var) = CPUMASK_VAR_NULL;
>> int cpu;
>>
>> - lockdep_assert_cpus_held();
>> -
>> if (!works)
>> return -ENOMEM;
>> if (!alloc_cpumask_var(&cpumask, GFP_KERNEL))
>> @@ -1570,6 +1568,7 @@ int tmigr_isolated_exclude_cpumask(struct cpumask *exclude_cpumask)
>> * First set previously isolated CPUs as available (unisolate).
>> * This cpumask contains only CPUs that switched to available now.
>> */
>> + guard(cpus_read_lock)();
>> cpumask_andnot(cpumask, cpu_online_mask, exclude_cpumask);
>> cpumask_andnot(cpumask, cpumask, tmigr_available_cpumask);
>>
>
--
Best regards,
Ridong
On 1/29/26 3:20 AM, Chen Ridong wrote:
>
> On 2026/1/29 16:01, 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().
>>>
> When I reviewed Frederic's patches, I concerned about this issue. However, I was
> not certain whether any flush worker would need to acquire cpu_hotplug_lock or
> cpuset_mutex.
>
> Despite this warning, I do not understand how wq_completion would need to
> acquire cpu_hotplug_lock and cpuset_mutex.
>
> The reason I want to understand how wq_completion acquires cpu_hotplug_lock or
> cpuset_mutex is to determine whether isolcpus_update_mutex is truly necessary.
> As I mentioned in my previous email, I am concerned about a potential
> use-after-free (UAF) issue, which might imply that isolcpus_update_mutex is
> required in most places that currently acquire cpuset_mutex, with the possible
> exception of the hotplug path?
A circular lock dependency can invoke more than 2 tasks/parties. In this
case, the task that hold wq_completion does not need to acquire
cpu_hotplug_lock. If a worker that flushes a work function required for
the completion to finish and it happens to acquire cpu_hotplug_lock with
another task trying to acquire cpus_write_lock in the interim, the
worker will wait there for the write lock to be released which will not
happen until the original task that calls flush_workqueue() release its
read lock. In essence, it is a deadlock.
Cheers,
Longman
On 2026/1/30 4:57, Waiman Long wrote:
> On 1/29/26 3:20 AM, Chen Ridong wrote:
>>
>> On 2026/1/29 16:01, 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().
>>>>
>> When I reviewed Frederic's patches, I concerned about this issue. However, I was
>> not certain whether any flush worker would need to acquire cpu_hotplug_lock or
>> cpuset_mutex.
>>
>> Despite this warning, I do not understand how wq_completion would need to
>> acquire cpu_hotplug_lock and cpuset_mutex.
>>
>> The reason I want to understand how wq_completion acquires cpu_hotplug_lock or
>> cpuset_mutex is to determine whether isolcpus_update_mutex is truly necessary.
>> As I mentioned in my previous email, I am concerned about a potential
>> use-after-free (UAF) issue, which might imply that isolcpus_update_mutex is
>> required in most places that currently acquire cpuset_mutex, with the possible
>> exception of the hotplug path?
>
> A circular lock dependency can invoke more than 2 tasks/parties. In this case,
> the task that hold wq_completion does not need to acquire cpu_hotplug_lock. If a
> worker that flushes a work function required for the completion to finish and it
> happens to acquire cpu_hotplug_lock with another task trying to acquire
> cpus_write_lock in the interim, the worker will wait there for the write lock to
> be released which will not happen until the original task that calls
> flush_workqueue() release its read lock. In essence, it is a deadlock.
>
Thanks, Longman,
I looked through the relevant workers:
pci_probe_flush_workqueue()
mem_cgroup_flush_workqueue()
vmstat_flush_workqueue()
However, I still haven’t found any worker function that actually acquires
cpu_hotplug_lock or cpuset_mutex. Perhaps I missed something.
--
Best regards,
Ridong
© 2016 - 2026 Red Hat, Inc.