Re: [Devel] [PATCH VZ10 v2 3/3] ve_namespace: Allow entering non-running VE namespace

[Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>]

On 9/9/26 16:33, Vasileios Almpanis wrote:
>> This doesn't bring a lot of value and makes tests more complex.
>> To enter VE namespace we need to actually start VE.
>>
>> In vzctl we have locking to prevent parallel creations.
>> If someone decides to skip locks and start one CT
>> in parallel - big mistake, but not ours.
>>
>> https://virtuozzo.atlassian.net/browse/VSTOR-144504
>>
>> Feature: ve selftests
>> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
>>
>> diff --git a/kernel/ve/ve_namespace.c b/kernel/ve/ve_namespace.c
>> index 3dbe0dde2645..879a7863e33f 100644
>> --- a/kernel/ve/ve_namespace.c
>> +++ b/kernel/ve/ve_namespace.c
>> @@ -227,7 +227,8 @@ static int ve_ns_install(struct nsset *nsset, struct ns_common *new)
>>   		return -EPERM;
>>   
>>   	if (!VE_IS_RUNNING(ve_ns->ve))
>> -		return -EINVAL;
>> +		pr_warn_once("Switching to a non-running ve %s.\n",
> 
> Maybe better print ve->veid instead of ve_name(). ve_offline() sets
> ve->ve_name to NULL when the ve cgroup is removed, so we will see
> "ve (null)" in that case.

We don't initialize this value in tests, it is 0.
Actually, can we have this situation? Reaching ve_ns_install means
we grabbed a ve_ns reference in validate_nsset. How can we do ->css_offline
at the same time?

> 
>> +			ve_name(ve_ns->ve));
> 
> This change means that now a host task can enter the CT before the CT
> has been started. A task can enter into the VE by two ways either
> copy_ve_ns or by switch_ve_namespace both of which call ve_setup_task.
> This function sets whether the task can be ptraced, core dumped etc
> which will be skipped because ve_setup_task checks if ve is running
> which it will not be. So here we break the invariant that tasks that
> enter into the ve from host cannot be ptraced etc. I suggest to add
> a flag to ve_setup_task entered which will be 1 for switch_ve_namespace
> and 0 for copy_ve_ns> 

Good point. I'm not sure how real it is though. It should be someone
really nasty on the host trying to poison CT, but if this nasty person
is switching VE ns - it has CAP_SYS_ADMIN already.

It can protect us from bugs in vzctl though, so I think it is not completely
useless.
What if we drop VE_IS_RUNNING(ve) in ve_setup_task?

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

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel