[Devel] [PATCH VZ10] ve/cgroup: Grab missing cgroup_lock in cgroup_leave_vz_slice

[Vladimir Riabchun]

On 7/3/26 10:59, Pavel Tikhomirov wrote:
> 
> 
> On 7/2/26 14:50, Vladimir Riabchun wrote:
>> When creating a container with a lockdep enabled, there is a warning:
>> [  177.055606] WARNING: CPU: 2 PID: 6013 at kernel/cgroup/cgroup.c:3050 cgroup_migrate_add_src+0xe4/0x4d0
>> [  177.057199] Modules linked in: ...
>> [  177.069734] CPU: 2 UID: 0 PID: 6013 Comm: vzctl ve: 123 Kdump: loaded Not tainted 6.12.0-211.16.1.12.10.ovz10-v2 #2 12.10.ovz10 PREEMPT(voluntary)
>> [  177.071838] Hardware name: Acronis OpenStack Compute/Virtuozzo, BIOS 1.16.1-1.vz9.2 04/01/2014
>> [  177.073190] RIP: 0010:cgroup_migrate_add_src+0xe4/0x4d0
>> [  177.074016] Code: 00 00 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f e9 11 ba 61 02 be ff ff ff ff 48 c7 c7 10 c5 47 9e e8 f0 23 5f 02 85 c0 75 02 <0f> 0b 48 b8 00 00 00 00 00 fc ff df 48 89 ea 83 e5 07 48 c1 ea 03
>> [  177.076731] RSP: 0018:ffffc900054777f8 EFLAGS: 00010046
>> [  177.077560] RAX: 0000000000000000 RBX: ffff88813ad28800 RCX: 0000000000000001
>> [  177.078736] RDX: 0000000000000046 RSI: ffffffff9d4f3310 RDI: ffffffff9cbd1440
>> [  177.079831] RBP: ffffffff9ed162b4 R08: 0000000000000000 R09: 0000000000000006
>> [  177.080969] R10: 0000000000000000 R11: 0000000000000001 R12: ffffc900054778b0
>> [  177.082029] R13: ffff88819b2e6000 R14: ffff88819b2e6000 R15: ffff88858f118000
>> [  177.083078] FS:  00007f0172bdc600(0000) GS:ffff88875e400000(0000) knlGS:0000000000000000
>> [  177.084316] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [  177.085204] CR2: 00007ffe60f65cc8 CR3: 00000001b52ae006 CR4: 00000000003726f0
>> [  177.086271] Call Trace:
>> [  177.086735]  <TASK>
>> [  177.096467]  ? cgroup_migrate_add_src+0xe0/0x4d0
>> [  177.097217]  cgroup_attach_task+0x1a0/0x560
>> [  177.100947]  ve_start_container+0x7f8/0xbd0
>> [  177.101731]  ve_state_write+0x5b/0x80
>> [  177.102369]  cgroup_file_write+0x2c6/0x740
>> [  177.106910]  kernfs_fop_write_iter+0x3df/0x5f0
>> [  177.107638]  vfs_write+0x5fd/0xe90
>> [  177.112378]  ksys_write+0xfb/0x1d0
>> [  177.114393]  do_syscall_64+0x92/0x180
>> [  177.123132]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>> [  177.123955] RIP: 0033:0x7f01737d7954
>> [  177.124604] Code: c7 00 16 00 00 00 b8 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 80 3d 75 d7 0d 00 00 74 13 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 48 83 ec 28 48 89 54 24 18 48
>> [  177.127297] RSP: 002b:00007ffe60f67da8 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
>> [  177.128471] RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00007f01737d7954
>> [  177.129527] RDX: 0000000000000005 RSI: 00007f0173916081 RDI: 000000000000000b
>> [  177.130579] RBP: 000000000000000b R08: 0000000000000073 R09: 00000000ffffffff
>> [  177.131636] R10: 0000000000000000 R11: 0000000000000202 R12: 00007f0173916081
>> [  177.132756] R13: 00007ffe60f67e10 R14: 00007ffe60f68ea0 R15: 0000556be8eeb1b0
>> [  177.133851]  </TASK>
>> [  177.134301] irq event stamp: 2964
>> [  177.134903] hardirqs last  enabled at (2963): [<ffffffff9c4dd717>] _raw_spin_unlock_irqrestore+0x57/0x80
>> [  177.136366] hardirqs last disabled at (2964): [<ffffffff9c4dd310>] _raw_spin_lock_irq+0x60/0x90
>> [  177.137686] softirqs last  enabled at (2916): [<ffffffff99b16a07>] handle_softirqs+0x397/0x540
>> [  177.138987] softirqs last disabled at (2907): [<ffffffff99b16d3d>] __irq_exit_rcu+0xfd/0x250
>> [  177.140263] ---[ end trace 0000000000000000 ]---
>>
>> This is caused by cgroup_leave_vz_slice calling cgroup_attach_task
>> without cgroup_lock. cgroup_join_vz_slice doesn't have this issue
>> because cgroup_kn_lock_live implicitly takes cgroup_lock.
>> Protect cgroup_attach_task call in cgroup_leave_vz_slice with the
>> same mutex.
>>
>> https://virtuozzo.atlassian.net/browse/VSTOR-136835
>>
>> Feature: debug kernel
>> Fixes: 20a5affcf263 ("ve/cgroup: add vz.slice cgroup to put kernel threads to")
>> Signed-off-by: Vladimir Riabchun <vladimir.riabchun at virtuozzo.com>
>> ---
>>   kernel/cgroup/cgroup.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
>> index 5585545304a3..131ba2d8ae3e 100644
>> --- a/kernel/cgroup/cgroup.c
>> +++ b/kernel/cgroup/cgroup.c
>> @@ -2286,6 +2286,7 @@ int cgroup_leave_vz_slice(struct ve_struct *ve)
>>   {
>>   	struct css_set *cset;
>>   	struct cgroup *cgrp;
>> +	int ret;
>>   
>>   	cset = rcu_dereference_protected(ve->ve_nsproxy, 1)->cgroup_ns->root_cset;
>>   	cgrp = __cset_cgroup_from_root(cset, &cgrp_dfl_root);
>> @@ -2294,7 +2295,10 @@ int cgroup_leave_vz_slice(struct ve_struct *ve)
>>   	    !test_bit(CGRP_VE_ROOT, &cgrp->flags))
>>   		return 0;
>>   
>> -	return cgroup_attach_task(cgrp, current, true);
>> +	cgroup_lock();
>> +	ret = cgroup_attach_task(cgrp, current, true);
> 
> Should we also do cgroup_attach_(un)lock as per cgroup_attach_task's comment?
> (same in cgroup_join_vz_slice()) (this is likely a requirement for cpuset_attach
> paths, and we have cpuset controller on contianer cgroups)
> 

True, submitted v2 version with this lock held as well.

>> +	cgroup_unlock();
>> +	return ret;
>>   }
>>   
>>   struct cgroup_subsys_state *css_ve_root1(struct cgroup_subsys_state *css)
> 

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo