On 05/14/2015 07:52 PM, Cyrill Gorcunov wrote:
> In vzctl/libvzctl bundle we restore container like
>
> - create ve/$ctid cgroup
> - move self into this cgroup
> - run criu from inside
>
> So that kernel code passes ve_can_attach test. In turn for
> our P.Haul project (which is managing live migration) the
> situation is different -- it opens ve/$ctid but moves
> criu service pid instead (so that the service will
> start restore procedure). Which leads to situation
> where ve_can_attach fails with -EINVAL.
>
> Reported-by: Nikita Spiridonov <nspirido...@odin.com>
> Signed-off-by: Cyrill Gorcunov <gorcu...@odin.com>
> CC: Vladimir Davydov <vdavy...@odin.com>
> CC: Konstantin Khorenko <khore...@odin.com>
> CC: Pavel Emelyanov <xe...@odin.com>
> CC: Andrey Vagin <ava...@odin.com>
> ---
>
> Guys, could you please take a look, especially from
> security POV, is it safe to remove all these checks?
>
> kernel/ve/ve.c | 31 +++++++++++++------------------
> 1 file changed, 13 insertions(+), 18 deletions(-)
>
> Index: linux-pcs7.git/kernel/ve/ve.c
> ===================================================================
> --- linux-pcs7.git.orig/kernel/ve/ve.c
> +++ linux-pcs7.git/kernel/ve/ve.c
> @@ -750,13 +750,6 @@ static void ve_destroy(struct cgroup *cg
> static int ve_can_attach(struct cgroup *cg, struct cgroup_taskset *tset)
> {
> struct ve_struct *ve = cgroup_ve(cg);
> - struct task_struct *task = current;
> -
> - if (cgroup_taskset_size(tset) != 1 ||
> - cgroup_taskset_first(tset) != task ||
> - !thread_group_leader(task) ||
> - !thread_group_empty(task))
> - return -EINVAL;
Is this true that without these checks a single thread of a multithread process
can enter CT?
If no - where is the check for this case?
If yes - let's prohibit this.
> if (ve->is_locked)
> return -EBUSY;
> @@ -775,20 +768,22 @@ static int ve_can_attach(struct cgroup *
> static void ve_attach(struct cgroup *cg, struct cgroup_taskset *tset)
> {
> struct ve_struct *ve = cgroup_ve(cg);
> - struct task_struct *tsk = current;
> -
> - /* this probihibts ptracing of task entered to VE from host system */
> - if (ve->is_running && tsk->mm)
> - tsk->mm->vps_dumpable = VD_VE_ENTER_TASK;
> + struct task_struct *tsk;
>
> - /* Drop OOM protection. */
> - tsk->signal->oom_score_adj = 0;
> - tsk->signal->oom_score_adj_min = 0;
> + cgroup_taskset_for_each(tsk, cg, tset) {
> + /* this probihibts ptracing of task entered to VE from host
> system */
> + if (ve->is_running && tsk->mm)
> + tsk->mm->vps_dumpable = VD_VE_ENTER_TASK;
> +
> + /* Drop OOM protection. */
> + tsk->signal->oom_score_adj = 0;
> + tsk->signal->oom_score_adj_min = 0;
>
> - /* Leave parent exec domain */
> - tsk->parent_exec_id--;
> + /* Leave parent exec domain */
> + tsk->parent_exec_id--;
>
> - tsk->task_ve = ve;
> + tsk->task_ve = ve;
> + }
> }
>
> static int ve_state_read(struct cgroup *cg, struct cftype *cft,
> _______________________________________________
> Devel mailing list
> Devel@openvz.org
> https://lists.openvz.org/mailman/listinfo/devel
>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
