Re: [PATCH] cgroup/cpuset: Creating or adding CPUs to partition not allowed without privilege

On 4/28/26 12:03 AM, [email protected] wrote:
> Sashiko AI review found 2 potential issue(s):
> - [High] Unconditionally clearing `adding` and `deleting` flags on partition error permanently leaks CPUs from the parent.
> - [High] Unprivileged users can bypass the new CAP_SYS_ADMIN checks and steal CPUs from the parent cpuset by exploiting auto-promotion during hierarchical updates.
> --
>
> commit 1214158b7ede4552eb3f704b911775642a66d632
> Author: Waiman Long <[email protected]>
>
> cgroup/cpuset: Creating or adding CPUs to partition not allowed without privilege
>
> This commit restricts creating cpuset partitions or adding CPUs to an existing
> local partition to privileged users with CAP_SYS_ADMIN.
>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index e3a081a07c6d..5fc8555f2046 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
> [ ... ]
>
>> @@ -1833,12 +1835,18 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>   					       parent->effective_xcpus);
>>   		}
>>   
>> +		/*
>> +		 * Taking CPUs away from parent is not allowed without privilege
>> +		 */
>> +		if (deleting && !capable(CAP_SYS_ADMIN))
>> +			part_error = PERR_ACCESS;
>> +
> Is it possible for unprivileged users to bypass this check?
>
> If part_error is set to PERR_ACCESS during partcmd_update, the function
> transitions the partition to invalid but still returns 0 to the caller. This
> means the unauthorized CPU mask is saved to cs->cpus_allowed and
> cs->effective_xcpus.
>
> Later, if a privileged user modifies the parent's cpuset or a CPU hotplug event
> occurs, update_cpumasks_hier() is triggered and calls
> update_parent_effective_cpumask() with newmask == NULL.
>
> Since there is no capable(CAP_SYS_ADMIN) check in the newmask == NULL path,
> and the update runs under a privileged context, won't the system auto-promote
> the partition and officially steal the unauthorized CPUs from the parent?
It is a good point. Auto-promotion should be disabled if the current 
error code is PERR_ACCESS.
>
> [ ... ]
>
>> @@ -1919,8 +1927,10 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>   	}
>>   
>>   write_error:
>> -	if (part_error)
>> +	if (part_error) {
>>   		WRITE_ONCE(cs->prs_err, part_error);
>> +		adding = deleting = false;
>> +	}
> Does clearing the adding flag here leak CPUs from the parent?
>
> When a valid partition becomes invalid due to an error, such as PERR_NOCPUS or
> the new PERR_ACCESS, it must return its exclusively held CPUs to the parent's
> effective_cpus.
>
> The pre-patch code computed the rollback mask in tmp->addmask and left
> adding = true so that partition_xcpus_del() would be called later.
>
> By unconditionally clearing the adding flag here, the partition transitions
> to an invalid state, but the required partition_xcpus_del() call is skipped.
> Are these CPUs permanently leaked from the parent cpuset and the system's
> scheduling domains?

Right the clearing should be done in the same place where PERR_ACCESS 
error is set.

Regards,
LOngman

>