[Devel] [PATCH vz9 1/2] ve/cgroups: fix use after free in ve_exit_ns

[Alexander Atanasov]

On 1.02.23 15:06, Pavel Tikhomirov wrote:
> Nacked-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
> 
> This is not last reference to ve. Last reference is removed when we 
> remove cgroup directory with rmdir. One cant remove cgroup until there 
> is a process in it. ve_exit_ns is called from task in this ve cgroup.

Then should we fix vz7 do the same?

if it is really not the last reference - why does it need to hold the 
mutex do drop the reference ?
And i haven't checked how they are destroyed on reboot.

> 
> On 30.01.2023 13:00, Alexander Atanasov wrote:
>> Release the lock before dropping the last reference to ve in
>> ve_exit_ns which can lead to a call to ve_destroy which in turns
>> does free the ve. In general it is always a bug to drop a reference
>> of an object with locks held inside of it.
>>
>> https://jira.sw.ru/browse/PSBM-144580
>> Signed-off-by: Alexander Atanasov <alexander.atanasov at virtuozzo.com>
>> ---
>>   kernel/ve/ve.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> i've checked vz7 and it does not have this issue.
>>
>> diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
>> index 407d7de6e071..80865161670e 100644
>> --- a/kernel/ve/ve.c
>> +++ b/kernel/ve/ve.c
>> @@ -857,9 +857,11 @@ void ve_exit_ns(struct pid_namespace *pid_ns)
>>       ve_hook_iterate_fini(VE_SS_CHAIN, ve);
>>       ve_list_del(ve);
>>       ve_drop_context(ve);
>> +    up_write(&ve->op_sem);
>> +
>>       printk(KERN_INFO "CT: %s: stopped\n", ve_name(ve));
>> +
>>       put_ve(ve); /* from ve_start_container() */
>> -    up_write(&ve->op_sem);
>>   }
>>   u64 ve_get_monotonic(struct ve_struct *ve)
> 

-- 
Regards,
Alexander Atanasov