[CRIU] [PATCH 2/2] creds: Allow to restore more restricted tasks

[Andrew Vagin]

On Thu, Nov 26, 2015 at 03:24:40PM +0300, Cyrill Gorcunov wrote:
> Currently we require the caps of children task
> (of threads) to be exactly the same as their parent

We require that all threads of one process have the same set of
capabilities, because we don't save capabilities per-thread.

Children and parent can have different capabilities.


> is havin, which doesn't allow more restircted tasks
> (which granted with less caps) to be dumped.
> 
> Lets rename proc_status_creds_eq to proc_status_creds_dumpable
> and split it into two stages
> 
>  - compare non caps member to be the same
>  - allow to proceed dumping if children are
>    having less creds than a parent
> 
> Note that here might be a situation when parent creates
> children/threads but then drops off own caps, for this
> scenario we will refuse to dump yet. It should be addressed
> separately.
> 
> https://jira.sw.ru/PSBM-41416
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  include/proc_parse.h |  3 ++-
>  proc_parse.c         | 76 ++++++++++++++++++++++++++++++++++++++++++++++++----
>  ptrace.c             |  2 +-
>  3 files changed, 74 insertions(+), 7 deletions(-)
> 
> diff --git a/include/proc_parse.h b/include/proc_parse.h
> index 6dc5e57de600..952b697dc64d 100644
> --- a/include/proc_parse.h
> +++ b/include/proc_parse.h
> @@ -103,7 +103,8 @@ struct proc_status_creds {
>  	u32			cap_bnd[PROC_CAP_SIZE];
>  };
>  
> -bool proc_status_creds_eq(struct proc_status_creds *o1, struct proc_status_creds *o2);
> +bool proc_status_creds_dumpable(struct proc_status_creds *parent,
> +				struct proc_status_creds *child);
>  
>  struct fstype {
>  	char *name;
> diff --git a/proc_parse.c b/proc_parse.c
> index c479b0fa9b3c..19b8a2de0446 100644
> --- a/proc_parse.c
> +++ b/proc_parse.c
> @@ -2171,13 +2171,79 @@ int aufs_parse(struct mount_info *new)
>  	return ret;
>  }
>  
> -bool proc_status_creds_eq(struct proc_status_creds *o1, struct proc_status_creds *o2)
> +bool proc_status_creds_dumpable(struct proc_status_creds *parent,
> +				struct proc_status_creds *child)
>  {
> -	/* FIXME: this is a little too strict, we should do semantic comparison
> -	 * of seccomp filters instead of forcing them to be exactly identical.
> -	 * It's not unsafe, though, so let's be lazy for now.
> +	const size_t size = sizeof(struct proc_status_creds) -
> +			offsetof(struct proc_status_creds, cap_inh);
> +
> +	/*
> +	 * The comparision rules are the following
> +	 *
> +	 *  - CAPs can be relaxed in children
> +	 *  - seccomp filters should be passed via
> +	 *    semantic comparision (FIXME) but for
> +	 *    now we require them to be exactly
> +	 *    identical
> +	 *  - the rest of members must match
>  	 */
> -	return memcmp(o1, o2, sizeof(struct proc_status_creds)) == 0;
> +
> +	if (memcmp(parent, child, size)) {
> +		if (!pr_quelled(LOG_DEBUG)) {
> +			pr_debug("Creds undumpable (parent:child)\n"
> +				 "  uids:               %d:%d %d:%d %d:%d %d:%d\n"
> +				 "  gids:               %d:%d %d:%d %d:%d %d:%d\n"
> +				 "  state:              %d:%d"
> +				 "  ppid:               %d:%d\n"
> +				 "  sigpnd:             %llu:%llu\n"
> +				 "  shdpnd:             %llu:%llu\n"
> +				 "  seccomp_mode:       %d:%d\n"
> +				 "  last_filter:        %u:%u\n",
> +				 parent->uids[0], child->uids[0],
> +				 parent->uids[1], child->uids[1],
> +				 parent->uids[2], child->uids[2],
> +				 parent->uids[3], child->uids[3],
> +				 parent->gids[0], child->gids[0],
> +				 parent->gids[1], child->gids[1],
> +				 parent->gids[2], child->gids[2],
> +				 parent->gids[3], child->gids[3],
> +				 parent->state, child->state,
> +				 parent->ppid, child->ppid,
> +				 parent->sigpnd, child->sigpnd,
> +				 parent->shdpnd, child->shdpnd,
> +				 parent->seccomp_mode, child->seccomp_mode,
> +				 parent->last_filter, child->last_filter);
> +		}
> +		return false;
> +	}
> +
> +	if ((child->cap_inh[0] > parent->cap_inh[0]) ||
> +	    (child->cap_inh[1] > parent->cap_inh[1]) ||
> +	    (child->cap_prm[0] > parent->cap_prm[0]) ||
> +	    (child->cap_prm[1] > parent->cap_prm[1]) ||
> +	    (child->cap_eff[0] > parent->cap_eff[0]) ||
> +	    (child->cap_eff[1] > parent->cap_eff[1]) ||
> +	    (child->cap_bnd[0] > parent->cap_bnd[0]) ||
> +	    (child->cap_bnd[1] > parent->cap_bnd[1])) {
> +		if (!pr_quelled(LOG_DEBUG)) {
> +			pr_debug("Caps undumpable (parent:child)\n"
> +				 "  cap_inh:            %#x:%#x %#x:%#x\n"
> +				 "  cap_prm:            %#x:%#x %#x:%#x\n"
> +				 "  cap_eff:            %#x:%#x %#x:%#x\n"
> +				 "  cap_bnd:            %#x:%#x %#x:%#x\n",
> +				 parent->cap_inh[0], child->cap_inh[0],
> +				 parent->cap_inh[1], child->cap_inh[1],
> +				 parent->cap_prm[0], child->cap_prm[0],
> +				 parent->cap_prm[1], child->cap_prm[1],
> +				 parent->cap_eff[0], child->cap_eff[0],
> +				 parent->cap_eff[1], child->cap_eff[1],
> +				 parent->cap_bnd[0], child->cap_bnd[0],
> +				 parent->cap_bnd[1], child->cap_bnd[1]);
> +		}
> +		return false;
> +	}
> +
> +	return true;
>  }
>  
>  int parse_children(pid_t pid, pid_t **_c, int *_n)
> diff --git a/ptrace.c b/ptrace.c
> index 9c2b3cc80b7e..43b45bf70412 100644
> --- a/ptrace.c
> +++ b/ptrace.c
> @@ -235,7 +235,7 @@ try_again:
>  
>  		**creds = cr;
>  
> -	} else if (!proc_status_creds_eq(*creds, &cr)) {
> +	} else if (!proc_status_creds_dumpable(*creds, &cr)) {
>  		pr_err("creds don't match %d %d\n", pid, ppid);
>  		goto err;
>  	}
> -- 
> 2.5.0
>