[CRIU] [PATCH v2] util: disable log service in cr_system_userns if error fd was -1

[Andrew Vagin]

On Fri, Apr 22, 2016 at 07:44:24PM +0300, Stanislav Kinsburskiy wrote:
> In case of error fd was negative, log_fd is _moved_ as STDERR for the child.
> After move, log_fd is closed, and thus log service is broken and any error
> prints will disappear.

The bug is that we move log_fd into STDERR. If out is negative,
we will move logfd into STDOUT and your patch doesn't handle this case.

I think we should not move logfd anywhere. Pls, look at my patch.

> This patch disables log service, thus redirecting error output to STDERR
> (which is log fd in this case).
> 
> Calling of log fini is:
> 1) to early: reopen_fd_as_nocheck will fail
> 2) to much: close_safe will print error message, because log fd was already
> closed
> 
> v2:
> log_disable helper introduced
> 
> Signed-off-by: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
> ---
>  criu/include/log.h |    1 +
>  criu/log.c         |    5 +++++
>  criu/util.c        |   17 ++++++++++++++---
>  3 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/criu/include/log.h b/criu/include/log.h
> index fe53a7c..7ecc3fe 100644
> --- a/criu/include/log.h
> +++ b/criu/include/log.h
> @@ -6,6 +6,7 @@
>  #include "criu-log.h"
>  
>  extern int log_init(const char *output);
> +extern void log_disable(void);
>  extern void log_fini(void);
>  extern int log_init_by_pid(void);
>  extern void log_closedir(void);
> diff --git a/criu/log.c b/criu/log.c
> index 6c5d807..1992e3a 100644
> --- a/criu/log.c
> +++ b/criu/log.c
> @@ -134,6 +134,11 @@ int log_init_by_pid(void)
>  	return log_init(path);
>  }
>  
> +void log_disable(void)
> +{
> +	disable_service_fd(LOG_FD_OFF);
> +}
> +
>  void log_fini(void)
>  {
>  	close_service_fd(LOG_FD_OFF);
> diff --git a/criu/util.c b/criu/util.c
> index dae6031..216efb6 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -451,6 +451,17 @@ int criu_get_image_dir(void)
>  	return get_service_fd(IMG_FD_OFF);
>  }
>  
> +int disable_service_fd(enum sfd_type type)
> +{
> +	int fd;
> +
> +	fd = get_service_fd(type);
> +	if (fd < 0)
> +		return 0;
> +
> +	clear_bit(type, sfd_map);
> +	return 0;
> +}
>  int close_service_fd(enum sfd_type type)
>  {
>  	int fd;
> @@ -641,9 +652,6 @@ int cr_system_userns(int in, int out, int err, char *cmd,
>  		    move_img_fd(&err, STDIN_FILENO))
>  			goto out_chld;
>  
> -		if (stop_log_fd)
> -			log_fini();
> -
>  		if (in < 0) {
>  			close(STDIN_FILENO);
>  		} else {
> @@ -660,6 +668,9 @@ int cr_system_userns(int in, int out, int err, char *cmd,
>  		if (reopen_fd_as_nocheck(STDERR_FILENO, err))
>  			goto out_chld;
>  
> +		if (stop_log_fd)
> +			log_disable();
> +
>  		execvp(cmd, argv);
>  
>  		pr_perror("exec failed");
> 
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu