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

[Andrew Vagin]

On Wed, Oct 01, 2014 at 08:57:28PM +0000, Tycho Andersen wrote:
> We're using NULL as a sentinel here to indicate that we shouldn't restore any
> cgroup properties. We should make sure that we don't leak this information and
> instead check the n_properties field, which we should also set correctly.

Tycho, I beg you to run tests before sending patches.

It's like brushing teeth before bedtime.

[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 =================================

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