[Devel] [PATCH VZ8 v3 14/14] cgroup/ve: do not run release_agent on non-running ve

[Kirill Tkhai]

On 16.02.2021 11:44, Valeriy Vdovin wrote:
> cgroup1_release_agent is a function that runs within a private
> ve workqueue. When exectured, it runs an executable in a userspace
> by a call to call_usermodehelper_ve. There is conflict that when
> ve is getting shutdown and some of last cgroups get's deleted at the
> same time, the workqueue might still be running, but ve_stop_ns has
> already been called. ve_stop_ns will stop usermode helper threads,
> needed for call_usermodehelper_ve. Because of that a call to
> call_usermodehelper_ve will never return, causing a hang.
> To defeat that hang VZ7 code of call_usermodehelper_ve included the
> check that ve is still running before running the userspace executable.
> It also checked for ve->init_task->flags & PF_EXITING condition.
> But in VZ8 the whole usermodehelper infrastructure is much more different.
> Also VZ8 does not have ve->init_task in it's fields.
> That's why it seems more relevant right now to do ve->is_running
> check before the call to call_usermodehelper_ve.
> 
> Signed-off-by: Valeriy Vdovin <valeriy.vdovin at virtuozzo.com>

Reviewed-by: Kirill Tkhai <ktkhai at virtuozzo.com>

> ---
>  kernel/cgroup/cgroup-v1.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
> index 993ac38b895f..cd1a0df6c528 100644
> --- a/kernel/cgroup/cgroup-v1.c
> +++ b/kernel/cgroup/cgroup-v1.c
> @@ -934,9 +934,12 @@ void cgroup1_release_agent(struct work_struct *work)
>  		envp[i++] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin";
>  		envp[i] = NULL;
>  
> -
>  		mutex_unlock(&cgroup_mutex);
>  
> +		down_write(&ve->op_sem);
> +		if (!ve->is_running)
> +			goto continue_with_mutex;
> +
>  		err = call_usermodehelper_ve(ve, argv[0], argv,
>  			envp, UMH_WAIT_EXEC);
>  
> @@ -944,6 +947,7 @@ void cgroup1_release_agent(struct work_struct *work)
>  			pr_warn_ratelimited("cgroup1_release_agent "
>  					    "%s %s failed: %d\n",
>  					    agentbuf, pathbuf, err);
> +continue_with_mutex:
>  		up_write(&ve->op_sem);
>  		mutex_lock(&cgroup_mutex);
>  continue_free:
>