linux-kernel - Re: [Question] set_cpus_allowed_ptr() call failed at cpuset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8bda2a8d-7faf-621d-c3c0-6351a49219ea@redhat.com>
Date:   Fri, 14 Jan 2022 15:33:34 -0500
From:   Waiman Long <longman@...hat.com>
To:     Tejun Heo <tj@...nel.org>, Zhang Qiao <zhangqiao22@...wei.com>
Cc:     lizefan.x@...edance.com, hannes@...xchg.org,
        cgroups@...r.kernel.org, linux-kernel@...r.kernel.org,
        Michal Koutný <mkoutny@...e.com>
Subject: Re: [Question] set_cpus_allowed_ptr() call failed at cpuset_attach()

On 1/14/22 11:20, Tejun Heo wrote:
> (cc'ing Waiman and Michal and quoting whole body)
>
> Seems sane to me but let's hear what Waiman and Michal think.
>
> On Fri, Jan 14, 2022 at 09:15:06AM +0800, Zhang Qiao wrote:
>> Hello everyone
>>
>> 	I found the following warning log on qemu. I migrated a task from one cpuset cgroup to
>> another, while I also performed the cpu hotplug operation, and got following calltrace.
>>
>> 	This may lead to a inconsistency between the affinity of the task and cpuset.cpus of the
>> dest cpuset, but this task can be successfully migrated to the dest cpuset cgroup.
>>
>> 	Can we use cpus_read_lock()/cpus_read_unlock() to guarantee that set_cpus_allowed_ptr()
>> doesn't fail, as follows:
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index d0e163a02099..2535d23d2c51 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -2265,6 +2265,7 @@ static void cpuset_attach(struct cgroup_taskset *tset)
>>          guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
>>
>>          cgroup_taskset_for_each(task, css, tset) {
>> +               cpus_read_lock();
>>                  if (cs != &top_cpuset)
>>                          guarantee_online_cpus(task, cpus_attach);
>>                  else
>> @@ -2274,6 +2275,7 @@ static void cpuset_attach(struct cgroup_taskset *tset)
>>                   * fail.  TODO: have a better way to handle failure here
>>                   */
>>                  WARN_ON_ONCE(set_cpus_allowed_ptr(task, cpus_attach));
>> +               cpus_read_unlock();
>>
>>
>> 	Is there a better solution?
>>
>> 	Thanks

The change looks OK to me. However, we may need to run the full set of 
regression test to make sure that lockdep won't complain about potential 
deadlock.

Cheers,
Longman