Re: [PATCH 21/23] cgroup/cpuset: Limit the side effect of using CPU hotplug on isolated partition

[sashiko-bot@kernel.org]

Sashiko AI review found 3 potential issue(s):
- [High] The fallback CPU mask is overwritten and ignored in `cpuset_update_tasks_cpumask`, causing task migration to fail silently.
- [High] The `cpuhp_offline_cb_mode` hack bypasses partition invalidation globally, but its fallback logic only protects `PRS_ISOLATED`, breaking standard `PRS_ROOT` partitions.
- [High] Unsynchronized access to the global `cpuhp_offline_cb_mode` flag creates a data race and TOCTOU vulnerability in partition validation.
--

commit 273c35c4b20a0b62c4d54c5410e48d5450b71df4
Author: Waiman Long <longman@redhat.com>

cgroup/cpuset: Limit the side effect of using CPU hotplug on isolated partition

CPU hotplug is used to facilitate the modification of the
HK_TYPE_KERNEL_NOISE and HK_TYPE_MANAGED_IRQ cpumasks. However, tearing
down and bringing up CPUs can impact the cpuset partition states
as well. This patch attempts to work around partition invalidation by
pretending partitions are empty during cpuhp_offline_cb_mode, and
temporarily mapping tasks in isolated partitions to their parent's
effective CPUs.

> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index a927b9cd4f718..5f6b4e67748f2 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -434,6 +434,13 @@ static inline bool partition_is_populated(struct cpuset *cs,
>  	struct cpuset *cp;
>  	struct cgroup_subsys_state *pos_css;
>  
> +	/*
> +	 * Hack: In cpuhp_offline_cb_mode, pretend all partitions are empty
> +	 * to prevent unnecessary partition invalidation.
> +	 */
> +	if (cpuhp_offline_cb_mode)
> +		return false;
> +

Is it safe to read cpuhp_offline_cb_mode here without synchronization?

The hotplug thread sets this flag while holding lock_device_hotplug(). A
concurrent userspace write to cpuset.cpus.partition triggers
validate_partition(), which holds cpuset_mutex and cpus_read_lock(), but not
lock_device_hotplug().

Could validate_partition() read cpuhp_offline_cb_mode as true before the
hotplug thread acquires cpus_write_lock(), causing the safety checks to
falsely conclude the partition is empty and bypassing cgroup validation?

[ ... ]

> @@ -3881,6 +3888,17 @@ hotplug_update_tasks(struct cpuset *cs,
>  	cs->effective_mems = *new_mems;
>  	spin_unlock_irq(&callback_lock);
>  
> +	/*
> +	 * When cpuhp_offline_cb_mode is active, valid isolated partition
> +	 * with tasks may have no online CPUs available for a short while.
> +	 * In that case, we fall back to parent's effective CPUs temporarily
> +	 * which will be reset back to their rightful value once the affected
> +	 * CPUs are online again.
> +	 */
> +	if (cpuhp_offline_cb_mode && cpumask_empty(new_cpus) &&
> +	   (cs->partition_root_state == PRS_ISOLATED))
> +		cpumask_copy(new_cpus, parent_cs(cs)->effective_cpus);
> +

By bypassing partition_is_populated() globally during cpuhp_offline_cb_mode,
all partitions skip normal invalidation. However, this fallback logic strictly
checks for PRS_ISOLATED.

If a standard PRS_ROOT partition loses its last online CPU, it gets bypassed
for invalidation but receives no fallback mask. Won't this leave the PRS_ROOT
partition with a completely empty effective_cpus mask, breaking its isolation
and causing subsequent cgroup_attach() operations to fail with -ENOSPC?

>  	if (cpus_updated)
>  		cpuset_update_tasks_cpumask(cs, new_cpus);

Does the temporary fallback mask actually get applied to the tasks?

Earlier in hotplug_update_tasks(), cs->effective_cpus is overwritten with the
empty incoming new_cpus mask. Then, inside cpuset_update_tasks_cpumask(),
new_cpus is overwritten again by intersecting it with cs->effective_cpus:

kernel/cgroup/cpuset.c:cpuset_update_tasks_cpumask() {
    ...
		} else {
			cpumask_and(new_cpus, possible_mask, cs->effective_cpus);
		}
		set_cpus_allowed_ptr(task, new_cpus);
    ...
}

Since cs->effective_cpus is already empty, the resulting new_cpus mask passed
to set_cpus_allowed_ptr() will also be empty. Doesn't set_cpus_allowed_ptr()
fail with -EINVAL when given an empty mask, leaving the task's affinity
unchanged and forcing the scheduler to migrate the tasks to arbitrary fallback
CPUs system-wide?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260421030351.281436-1-longman@redhat.com?part=21