[CRIU] [PATCH 06/10] cgroup: fix --cgroup-root and cgns interaction

[Andrew Vagin]

On Wed, Mar 02, 2016 at 04:17:04PM -0700, Tycho Andersen wrote:
> Basically, instead of --cgroup-root replacing the actual root, when a cgns
> is present, it just replaces the namespace prefix. See patch comments for
> details.
> 
> Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> ---
>  criu/cgroup.c | 65 ++++++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 47 insertions(+), 18 deletions(-)
> 
> diff --git a/criu/cgroup.c b/criu/cgroup.c
> index cf005d9..0ad70bf 100644
> --- a/criu/cgroup.c
> +++ b/criu/cgroup.c
> @@ -1502,26 +1502,63 @@ err:
>  }
>  
>  static int rewrite_cgsets(CgroupEntry *cge, char **controllers, int n_controllers,
> -			  char *from, char *to)
> +			  char **from, char *to)
>  {
>  	int i, j;
> +	bool set_from = false;
> +
>  	for (i = 0; i < cge->n_sets; i++) {
>  		CgSetEntry *set = cge->sets[i];
>  		for (j = 0; j < set->n_ctls; j++) {
>  			CgMemberEntry *cg = set->ctls[j];
> -			if (cgroup_contains(controllers, n_controllers, cg->name) &&
> -					/* +1 to get rid of leading / */
> -					strstartswith(cg->path + 1, from)) {
> +			char *tmp = cg->path, *tmp2 = NULL;
>  
> -				char *tmp = cg->path;
> +			if (!(cgroup_contains(controllers, n_controllers, cg->name) &&
> +					/* +1 to get rid of leading / */
> +					strstartswith(cg->path + 1, *from)))
> +				continue;
> +
> +			/* If this cgset has a cgns prefix, let's use
> +			 * that as the start of the root replacement.
> +			 */
> +			if (cg->has_cgns_prefix && cg->cgns_prefix) {
> +				/* Rewrite the group dir to match the
> +				 * prefix. We can do this exactly once
> +				 * since we know all the tasks are in
> +				 * the same cgroup ns (and thus have
> +				 * the same per-controller prefix path)
> +				 * since we don't support nesting.
> +				 */
> +				if (!set_from) {
> +					set_from = true;
> +					/* -2 because cgns_prefix includes leading and trailing /'s */
> +					*from = xsprintf("%s%s", to, (*from) + cg->cgns_prefix - 2);
> +				}
>  
> -				/* +1 to get rid of leading /, again */
>  				cg->path = xsprintf("%s%s", to, cg->path +
> -							strlen(from) + 1);
> -				if (!cg->path)
> +							cg->cgns_prefix - 1);
> +				cg->cgns_prefix = strlen(to);
> +			} else {
> +				/* otherwise, use the old rewriting strategy */
> +				cg->path = xsprintf("%s%s", to, cg->path +
> +							strlen(*from) + 1);
> +				if (!set_from) {
> +					set_from = true;
> +					*from = xstrdup(to);
> +				}
> +			}
> +
> +			if (tmp2) {

tmp2 is always NULL here

*** CID 158552:  Possible Control flow issues  (DEADCODE)
/criu/cgroup.c: 1625 in rewrite_cgsets()
1619                                            set_from = true;
1620                                            *from = xstrdup(to);
1621                                    }
1622                            }
1623
1624                            if (tmp2) {
>>>     CID 158552:  Possible Control flow issues  (DEADCODE)
>>>     Execution cannot reach this statement: "if (!*from)
  return -1;".
1625                                    if (!*from)
1626                                            return -1;
1627
1628                                    xfree(tmp2);
1629                            }
1630

** CID 87615:  Insecure data handling  (INTEGER_OVERFLOW)
/criu/cgroup.c: 1120 in move_in_cgroup()

> +				if (!*from)
>  					return -1;
> -				free(tmp);
> +
> +				xfree(tmp2);
>  			}
> +
> +			if (!cg->path)
> +				return -1;
> +
> +			free(tmp);
>  		}
>  
>  	}
> @@ -1549,18 +1586,10 @@ static int rewrite_cgroup_roots(CgroupEntry *cge)
>  		if (newroot) {
>  			for (j = 0; j < ctrl->n_dirs; j++) {
>  				CgroupDirEntry *cgde = ctrl->dirs[j];
> -				char *m;
>  
>  				pr_info("rewriting %s to %s\n", cgde->dir_name, newroot);
> -				if (rewrite_cgsets(cge, ctrl->cnames, ctrl->n_cnames, cgde->dir_name, newroot))
> -					return -1;
> -
> -				m = xstrdup(newroot);
> -				if (!m)
> +				if (rewrite_cgsets(cge, ctrl->cnames, ctrl->n_cnames, &cgde->dir_name, newroot))
>  					return -1;
> -
> -				free(cgde->dir_name);
> -				cgde->dir_name = m;
>  			}
>  		}
>  	}
> -- 
> 2.7.0
> 
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu