[CRIU] [PATCH 2/3] sfd: Make sure we're not overwriting existing files

[Andrey Vagin]

On Wed, Apr 03, 2019 at 04:22:01PM +0300, Cyrill Gorcunov wrote:
> Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
> ---
>  criu/include/servicefd.h | 19 +++++++++++++++++++
>  criu/servicefd.c         | 40 +++++++++++++++++++++++++++++++++++-----
>  2 files changed, 54 insertions(+), 5 deletions(-)
> 
> diff --git a/criu/include/servicefd.h b/criu/include/servicefd.h
> index f8c2814092b7..7be472cf43e0 100644
> --- a/criu/include/servicefd.h
> +++ b/criu/include/servicefd.h
> @@ -1,7 +1,13 @@
>  #ifndef __CR_SERVICE_FD_H__
>  #define __CR_SERVICE_FD_H__
>  
> +#include <stdio.h>
>  #include <stdbool.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +
> +#include "criu-log.h"
>  
>  enum sfd_type {
>  	SERVICE_FD_MIN,
> @@ -28,6 +34,19 @@ enum sfd_type {
>  struct pstree_item;
>  extern bool sfds_protected;
>  
> +
> +#define sfd_verify_target(_type, _old_fd, _new_fd)			\
> +	({								\
> +		int __ret = 0;						\
> +		if (fcntl(_new_fd, F_GETFD) != -1 && errno != EBADF) {	\
> +			pr_err("%s busy target %d -> %d\n",		\
> +			       sfd_type_name(_type), _old_fd, _new_fd);	\
> +			__ret = -1;					\
> +		}							\
> +		__ret;							\
> +	})
> +
> +extern const char *sfd_type_name(enum sfd_type type);
>  extern int init_service_fd(void);
>  extern int get_service_fd(enum sfd_type type);
>  extern bool is_any_service_fd(int fd);
> diff --git a/criu/servicefd.c b/criu/servicefd.c
> index a9909735af44..d918313a7181 100644
> --- a/criu/servicefd.c
> +++ b/criu/servicefd.c
> @@ -10,8 +10,6 @@
>  #include "common/compiler.h"
>  #include "common/list.h"
>  
> -#include "criu-log.h"
> -
>  #include "util.h"
>  #include "bitops.h"
>  #include "pstree.h"
> @@ -40,6 +38,31 @@ static int sfd_arr[SERVICE_FD_MAX];
>   */
>  bool sfds_protected = false;
>  
> +const char *sfd_type_name(enum sfd_type type)
> +{
> +	static const char *names[] = {
> +		[SERVICE_FD_MIN]	= __stringify_1(SERVICE_FD_MIN),
> +		[LOG_FD_OFF]		= __stringify_1(LOG_FD_OFF),
> +		[IMG_FD_OFF]		= __stringify_1(IMG_FD_OFF),
> +		[PROC_FD_OFF]		= __stringify_1(PROC_FD_OFF),
> +		[PROC_PID_FD_OFF]	= __stringify_1(PROC_PID_FD_OFF),
> +		[CR_PROC_FD_OFF]	= __stringify_1(CR_PROC_FD_OFF),
> +		[ROOT_FD_OFF]		= __stringify_1(ROOT_FD_OFF),
> +		[CGROUP_YARD]		= __stringify_1(CGROUP_YARD),
> +		[USERNSD_SK]		= __stringify_1(USERNSD_SK),
> +		[NS_FD_OFF]		= __stringify_1(NS_FD_OFF),
> +		[TRANSPORT_FD_OFF]	= __stringify_1(TRANSPORT_FD_OFF),
> +		[RPC_SK_OFF]		= __stringify_1(RPC_SK_OFF),
> +		[FDSTORE_SK_OFF]	= __stringify_1(FDSTORE_SK_OFF),
> +		[SERVICE_FD_MAX]	= __stringify_1(SERVICE_FD_MAX),
> +	};
> +
> +	if (type < ARRAY_SIZE(names))
> +		return names[type];
> +
> +	return "UNKNOWN";
> +}
> +
>  int init_service_fd(void)
>  {
>  	struct rlimit64 rlimit;
> @@ -107,7 +130,8 @@ int service_fd_min_fd(struct pstree_item *item)
>  
>  static void sfds_protection_bug(enum sfd_type type)
>  {
> -	pr_err("Service fd %u is being modified in protected context\n", type);
> +	pr_err("Service fd %s is being modified in protected context\n",
> +	       sfd_type_name(type));
>  	print_stack_trace(current ? vpid(current) : 0);
>  	BUG();
>  }
> @@ -128,8 +152,12 @@ int install_service_fd(enum sfd_type type, int fd)
>  		return fd;
>  	}
>  
> +	if (sfd_verify_target(type, fd, sfd))

sfd_verify_target() has to be called only if we install a service fd
first time.

> +		return -1;
> +
>  	if (dup3(fd, sfd, O_CLOEXEC) != sfd) {
> -		pr_perror("Dup %d -> %d failed", fd, sfd);
> +		pr_perror("%s dup %d -> %d failed",
> +			  sfd_type_name(type), fd, sfd);
>  		close(fd);
>  		return -1;
>  	}
> @@ -166,10 +194,12 @@ static void move_service_fd(struct pstree_item *me, int type, int new_id, int ne
>  	if (old < 0)
>  		return;
>  
> +	sfd_verify_target(type, old, new);
>  	ret = dup2(old, new);
>  	if (ret == -1) {
>  		if (errno != EBADF)
> -			pr_perror("Unable to clone %d->%d", old, new);
> +			pr_perror("%s unable to clone %d->%d",
> +				  sfd_type_name(type), old, new);
>  	} else if (!(rsti(me)->clone_flags & CLONE_FILES))
>  		close(old);
>  }
> -- 
> 2.20.1
>