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

[Andrey Vagin]

On Fri, Mar 29, 2019 at 08:55:23PM +0300, Cyrill Gorcunov wrote:
> Just a warn for now since we need to investigate
> every case without breaking ci.

We don't have these cases, so it should be an error.

Here is one inline comment

> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
> ---
>  criu/include/servicefd.h | 15 +++++++++++++++
>  criu/servicefd.c         |  4 ++--
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/criu/include/servicefd.h b/criu/include/servicefd.h
> index f8c2814092b7..1c4349e4eeb0 100644
> --- a/criu/include/servicefd.h
> +++ b/criu/include/servicefd.h
> @@ -1,7 +1,11 @@
>  #ifndef __CR_SERVICE_FD_H__
>  #define __CR_SERVICE_FD_H__
>  
> +#include <stdio.h>
>  #include <stdbool.h>
> +#include <unistd.h>
> +
> +#include "criu-log.h"
>  
>  enum sfd_type {
>  	SERVICE_FD_MIN,
> @@ -28,6 +32,17 @@ enum sfd_type {
>  struct pstree_item;
>  extern bool sfds_protected;
>  
> +#define sfd_verify_targtet(_type, _old_fd, _new_fd)					\
> +	do {										\
> +		char self_path[64];							\
> +											\
> +		snprintf(self_path, sizeof(self_path), "/proc/self/fd/%d", _new_fd);	\
> +		if (access(self_path, F_OK)) {						\
> +			pr_warn("type %d: busy target %d -> %d\n",			\
> +				_type, _old_fd, _new_fd);				\
> +		}									\
> +	} while (0)
> +
>  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..bb8c26ca466c 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"
> @@ -128,6 +126,7 @@ int install_service_fd(enum sfd_type type, int fd)
>  		return fd;
>  	}
>

You have to check that this service_fd has not been installed yet:
	if (!test_bit(type, sfd_map))

> +	sfd_verify_targtet(type, fd, sfd);
>  	if (dup3(fd, sfd, O_CLOEXEC) != sfd) {
>  		pr_perror("Dup %d -> %d failed", fd, sfd);
>  		close(fd);
> @@ -166,6 +165,7 @@ static void move_service_fd(struct pstree_item *me, int type, int new_id, int ne
>  	if (old < 0)
>  		return;
>  
> +	sfd_verify_targtet(type, old, new);
>  	ret = dup2(old, new);
>  	if (ret == -1) {
>  		if (errno != EBADF)
> -- 
> 2.20.1
>