[CRIU] [PATCH] cg: don't leak properties when cg dir exists

[Tycho Andersen]

Hi Andrew,

On Thu, Oct 02, 2014 at 08:38:37PM +0400, Andrew Vagin wrote:
>
> Could you try out the attached patch?

Yes, this seems to fix it for me. I seem to recall that there were a
few other files I had to correct permissions on, but I can't remember
what they were right now. I can post again (or better, post a patch)
next time it happens. Thanks!

Tycho

> > 
> > > 
> > > Tycho
> > > 
> > > > Thanks,
> > > > Andrey.
> > > > 
> > > > > 
> > > > > > [root at avagin-fc19-cr criu]# bash test/zdtm.sh static/cgroup00
> > > > > > ================================= CRIU CHECK =================================
> > > > > > Looks good.
> > > > > > Execute zdtm/live/static/cgroup00
> > > > > > ./cgroup00 --pidfile=cgroup00.pid --outfile=cgroup00.out --dirname=cgroup00.test
> > > > > > Dump 1457
> > > > > > Executing pre-restore hook
> > > > > > Cleaning cgclean.OkAnTX
> > > > > > Left there is:
> > > > > > cgroup.clone_children  cgroup.procs  cgroup.sane_behavior  notify_on_release  release_agent  tasks
> > > > > > Restore
> > > > > > Test: zdtm/live/static/cgroup00, Result: FAIL
> > > > > > ==================================== ERROR ====================================
> > > > > > Test: zdtm/live/static/cgroup00, Namespace: 
> > > > > > Dump log   : /root/git/criu/test/dump/static/cgroup00/1457/1/dump.log
> > > > > > --------------------------------- grep Error ---------------------------------
> > > > > > ------------------------------------- END -------------------------------------
> > > > > > Restore log: /root/git/criu/test/dump/static/cgroup00/1457/1/restore.log
> > > > > > --------------------------------- grep Error ---------------------------------
> > > > > > (00.032275)   1457: Error (cgroup.c:943): cg: copying property cpuset.cpus to cpuset////cpuset.cpus failed
> > > > > > (00.032319)   1457: Error (cgroup.c:968): cg: copying prop cpuset.cpus failed
> > > > > > (00.032336)   1457: Error (cgroup.c:1005): cg: Couldn't copy special cgroup properties
> > > > > > (00.032373) Error (cr-restore.c:1820): Restoring FAILED.
> > > > > > (00.039606) Error (cr-restore.c:1177): 1457 exited, status=1
> > > > > > ------------------------------------- END -------------------------------------
> > > > > > ================================= ERROR OVER =================================
> > > > > 
> > > > > For me this test passes, can you paste the entire log file? (really
> > > > > just the next line, there was a \n in my perror message)
> > > > > 
> > > > > Tycho
> > > > > 
> > > > > > Thanks,
> > > > > > Andrey
> > > > > > 
> > > > > > > 
> > > > > > > Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> > > > > > > ---
> > > > > > >  cgroup.c | 15 +++++++--------
> > > > > > >  1 file changed, 7 insertions(+), 8 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/cgroup.c b/cgroup.c
> > > > > > > index 26b0a17..ce49d04 100644
> > > > > > > --- a/cgroup.c
> > > > > > > +++ b/cgroup.c
> > > > > > > @@ -1114,12 +1114,7 @@ static int prepare_cgroup_dir_properties(char *path, int off, CgroupDirEntry **e
> > > > > > >  		size_t off2 = off;
> > > > > > >  
> > > > > > >  		off2 += sprintf(path + off, "/%s", e->dir_name);
> > > > > > > -		/*
> > > > > > > -		 * Check to see if we made e->properties NULL during restore
> > > > > > > -		 * because directory already existed and as such we don't want to
> > > > > > > -		 * change its properties
> > > > > > > -		 */
> > > > > > > -		if (e->properties) {
> > > > > > > +		if (e->n_properties > 0) {
> > > > > > >  			for (j = 0; j < e->n_properties; ++j) {
> > > > > > >  				if (restore_cgroup_prop(e->properties[j], path, off2) < 0)
> > > > > > >  					return -1;
> > > > > > > @@ -1168,7 +1163,7 @@ static int prepare_cgroup_dirs(char *paux, size_t off, CgroupDirEntry **ents, si
> > > > > > >  		/*
> > > > > > >  		 * Checking to see if file already exists. If not, create it. If
> > > > > > >  		 * it does exist, prevent us from overwriting the properties
> > > > > > > -		 * later by setting the CgroupDirEntry's properties pointer to NULL
> > > > > > > +		 * later by removing the CgroupDirEntry's properties.
> > > > > > >  		 */
> > > > > > >  		if (access(paux, F_OK) < 0) {
> > > > > > >  			if (errno != ENOENT) {
> > > > > > > @@ -1181,7 +1176,11 @@ static int prepare_cgroup_dirs(char *paux, size_t off, CgroupDirEntry **ents, si
> > > > > > >  			}
> > > > > > >  			pr_info("Created dir %s\n", paux);
> > > > > > >  		} else {
> > > > > > > -			e->properties = NULL;
> > > > > > > +			if (e->n_properties > 0) {
> > > > > > > +				xfree(e->properties);
> > > > > > > +				e->properties = NULL;
> > > > > > > +				e->n_properties = 0;
> > > > > > > +			}
> > > > > > >  			pr_info("Determined dir %s already existed\n", paux);
> > > > > > >  		}
> > > > > > >  
> > > > > > > -- 
> > > > > > > 1.9.1
> > > > > > > 
> > > > > > > _______________________________________________
> > > > > > > CRIU mailing list
> > > > > > > CRIU at openvz.org
> > > > > > > https://lists.openvz.org/mailman/listinfo/criu
> > _______________________________________________
> > CRIU mailing list
> > CRIU at openvz.org
> > https://lists.openvz.org/mailman/listinfo/criu

> From 3dcda4d899ee861d163bd764a0261714b051d54c Mon Sep 17 00:00:00 2001
> From: Andrey Vagin <avagin at openvz.org>
> Date: Thu, 2 Oct 2014 20:33:46 +0400
> Subject: [PATCH] zdtm.sh: don't change owner of a test directory
> 
> Signed-off-by: Andrey Vagin <avagin at openvz.org>
> ---
>  test/zdtm.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/zdtm.sh b/test/zdtm.sh
> index b5133c3..9a2c188 100755
> --- a/test/zdtm.sh
> +++ b/test/zdtm.sh
> @@ -407,7 +407,7 @@ start_test()
>  	if ! echo $TEST_SUID_LIST | grep -q $tname; then
>  		export ZDTM_UID=18943
>  		export ZDTM_GID=58467
> -		chown $ZDTM_UID:$ZDTM_GID $tdir
> +		chmod a+w $tdir
>  	fi
>  
>  	if [ -z "$PIDNS" ]; then
> -- 
> 1.9.3
>