[CRIU] [PATCH v2] criu: Add --keep-on-exec option

[Andrey Vagin]

On Tue, Jun 04, 2019 at 10:41:21AM +0300, Cyrill Gorcunov wrote:
> When we run action scripts we communicate with third-party
> tools via file descriptors inherited from a criu execution
> caller.
> 
> Since commit 6c8ea60491305482eb60d9b17a8635ab9248a5ab this
> no longer possible, moreover it breaks API.
> 
> Thus lets provide --keep-on-exec option where we could
> tell explicitly which exactly fds should be kept opened.
> 
> v2:
>  - provide qsort_cmp_int_array to avoid code duplication

We need a test for this functionality...

> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
> ---
>  Documentation/criu.txt    |  7 +++++++
>  criu/config.c             | 23 +++++++++++++++++++++++
>  criu/include/cr_options.h |  2 ++
>  criu/include/util.h       |  2 ++
>  criu/util.c               | 23 +++++++++++++++++++++++
>  5 files changed, 57 insertions(+)
> 
> diff --git a/Documentation/criu.txt b/Documentation/criu.txt
> index 6111c3baf0e1..5ffdd7cb1b28 100644
> --- a/Documentation/criu.txt
> +++ b/Documentation/criu.txt
> @@ -138,6 +138,13 @@ Common options are applicable to any 'command'.
>              notification message contains a file descriptor for
>              the master pty
>  
> +*--keep-on-exec* 'fd'::
> +    Keep 'fd' file descriptor when executing service programs and scripts
> +    during certain stages. When *criu* is executed it inherits opened file
> +    descriptors or a caller (if they are present). Sometimes it is needed
> +    to pass such descriptors to the *--action-script* scripts, otherwise
> +    they get closed for security reason.
> +
>  *-V*, *--version*::
>      Print program version and exit.
>  
> diff --git a/criu/config.c b/criu/config.c
> index 24f0b33858ad..a53f0c0ab6ce 100644
> --- a/criu/config.c
> +++ b/criu/config.c
> @@ -416,6 +416,22 @@ static int parse_join_ns(const char *ptr)
>  	return 0;
>  }
>  
> +static int parse_keep_on_exec(char *optarg)
> +{
> +	size_t size = sizeof(opts.keep_on_exec[0]) * (opts.nr_keep_on_exec + 1);
> +	int fd = atoi(optarg);

atoi() does not detect errors. You can use strtol or sscanf.

> +
> +	if (xrealloc_safe(&opts.keep_on_exec, size))
> +		return -ENOMEM;
> +
> +	opts.keep_on_exec[opts.nr_keep_on_exec++] = fd;
> +	qsort(opts.keep_on_exec, opts.nr_keep_on_exec,
> +	      sizeof(opts.keep_on_exec[0]),
> +	      qsort_cmp_int_array);
> +
> +	return 0;
> +}
> +
>  /*
>   * parse_options() is the point where the getopt parsing happens. The CLI
>   * parsing as well as the configuration file parsing happens here.
> @@ -511,6 +527,7 @@ int parse_options(int argc, char **argv, bool *usage_error,
>  		BOOL_OPT("remote", &opts.remote),
>  		{ "config",			required_argument,	0, 1089},
>  		{ "no-default-config",		no_argument,		0, 1090},
> +		{ "keep-on-exec",		required_argument,	0, 1092},
>  		{ },
>  	};
>  
> @@ -797,6 +814,12 @@ int parse_options(int argc, char **argv, bool *usage_error,
>  		case 1091:
>  			opts.ps_socket = atoi(optarg);
>  			break;
> +		case 1092:
> +			if (parse_keep_on_exec(optarg)) {
> +				pr_err("Unable to parse value for --keep-on-exec\n");
> +				return 1;
> +			}
> +			break;
>  		case 'V':
>  			pr_msg("Version: %s\n", CRIU_VERSION);
>  			if (strcmp(CRIU_GITID, "0"))
> diff --git a/criu/include/cr_options.h b/criu/include/cr_options.h
> index 8ddbf2341a48..2022ba12f8ec 100644
> --- a/criu/include/cr_options.h
> +++ b/criu/include/cr_options.h
> @@ -90,6 +90,8 @@ struct cr_options {
>  	struct list_head	inherit_fds;
>  	struct list_head	external;
>  	struct list_head	join_ns;
> +	int			*keep_on_exec;
> +	size_t			nr_keep_on_exec;
>  	char			*libdir;
>  	int			use_page_server;
>  	unsigned short		port;
> diff --git a/criu/include/util.h b/criu/include/util.h
> index a14be72293be..f4d2171bca35 100644
> --- a/criu/include/util.h
> +++ b/criu/include/util.h
> @@ -378,4 +378,6 @@ static inline void print_stack_trace(pid_t pid) {}
>  		___ret;									\
>  	})
>  
> +extern int qsort_cmp_int_array(const void *__a, const void *__b);
> +
>  #endif /* __CR_UTIL_H__ */
> diff --git a/criu/util.c b/criu/util.c
> index 31cdee1ff60f..3cfe7e02cce2 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -515,6 +515,20 @@ int cr_system(int in, int out, int err, char *cmd, char *const argv[], unsigned
>  	return cr_system_userns(in, out, err, cmd, argv, flags, -1);
>  }
>  
> +static bool should_keep_on_exec(int fd)
> +{
> +	if (!opts.nr_keep_on_exec)
> +		return false;
> +
> +	if (bsearch(&fd, opts.keep_on_exec,
> +		    opts.nr_keep_on_exec,
> +		    sizeof(opts.keep_on_exec[0]),
> +		    qsort_cmp_int_array))
> +	    return true;
> +
> +	return false;
> +}
> +
>  static int close_fds(int minfd)
>  {
>  	DIR *dir;
> @@ -541,6 +555,8 @@ static int close_fds(int minfd)
>  			continue;
>  		if (fd < minfd)
>  			continue;
> +		if (should_keep_on_exec(fd))
> +			continue;
>  		close(fd);
>  	}
>  	closedir(dir);
> @@ -1341,6 +1357,13 @@ void rlimit_unlimit_nofile(void)
>  	service_fd_rlim_cur = kdat.sysctl_nr_open;
>  }
>  
> +int qsort_cmp_int_array(const void *__a, const void *__b)
> +{
> +	int a = ((int *)__a)[0];
> +	int b = ((int *)__b)[0];
> +	return a == b ? 0 : (a < b ? -1 : 1);
> +}
> +
>  
>  #ifdef __GLIBC__
>  #include <execinfo.h>
> -- 
> 2.20.1
>