[CRIU] [PATCH 2/2] service_fd: Close argument fd in install_service_fd()
Andrei Vagin
avagin at virtuozzo.com
Wed Mar 22 17:28:26 PDT 2017
On Wed, Mar 22, 2017 at 02:06:05PM +0300, Kirill Tkhai wrote:
> We use this function in many places in the way
>
> sfd = install_service_fd(XXX_OFF, fd);
> close(fd);
>
> But, in common case, it may happen, that fd is equal to sfd,
No, it can't be. If it will be equal for some reason, we will have a lot
of troubles. Service fds should not intersect with other descriptors.
> and we close sfd. So, lets be independent of such situations
> and make install_service_fd() close argument fd by itself.
>
> https://travis-ci.org/tkhai/criu/builds/213782750
>
> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
> ---
> criu/action-scripts.c | 2 +-
> criu/cgroup.c | 5 +++--
> criu/cr-dump.c | 6 +++---
> criu/cr-restore.c | 5 +++--
> criu/fdstore.c | 5 +++--
> criu/files.c | 1 -
> criu/image.c | 1 -
> criu/include/servicefd.h | 1 +
> criu/log.c | 5 +++--
> criu/mount.c | 3 ++-
> criu/namespaces.c | 1 -
> criu/net.c | 6 +++---
> criu/tty.c | 2 +-
> criu/uffd.c | 5 +++--
> criu/util.c | 24 +++++++++++++++++++-----
> 15 files changed, 45 insertions(+), 27 deletions(-)
>
> diff --git a/criu/action-scripts.c b/criu/action-scripts.c
> index 4fa8e2843..3093cff47 100644
> --- a/criu/action-scripts.c
> +++ b/criu/action-scripts.c
> @@ -161,7 +161,7 @@ int add_rpc_notify(int sk)
> BUG_ON(scripts_mode == SCRIPTS_SHELL);
> scripts_mode = SCRIPTS_RPC;
>
> - rpc_sk = install_service_fd(RPC_SK_OFF, sk);
> + rpc_sk = do_install_service_fd(RPC_SK_OFF, sk);
>
> return 0;
> }
> diff --git a/criu/cgroup.c b/criu/cgroup.c
> index a1b3eb23f..3a7573d3b 100644
> --- a/criu/cgroup.c
> +++ b/criu/cgroup.c
> @@ -1611,9 +1611,10 @@ static int prepare_cgroup_sfd(CgroupEntry *ce)
> }
>
> ret = install_service_fd(CGROUP_YARD, i);
> - close(i);
> - if (ret < 0)
> + if (ret < 0) {
> + close(i);
> goto err;
> + }
>
> paux[off++] = '/';
>
> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
> index 5f1f668bb..950314b0d 100644
> --- a/criu/cr-dump.c
> +++ b/criu/cr-dump.c
> @@ -1272,10 +1272,10 @@ static int dump_one_task(struct pstree_item *item)
> goto err_cure_imgset;
> }
>
> - if (install_service_fd(CR_PROC_FD_OFF, pfd) < 0)
> + if (install_service_fd(CR_PROC_FD_OFF, pfd) < 0) {
> + close(pfd);
> goto err_cure_imgset;
> -
> - close(pfd);
> + }
> }
>
> ret = parasite_fixup_vdso(parasite_ctl, pid, &vmas);
> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
> index d0a970bc5..61cd458e7 100644
> --- a/criu/cr-restore.c
> +++ b/criu/cr-restore.c
> @@ -1795,9 +1795,10 @@ static int restore_root_task(struct pstree_item *init)
> }
>
> ret = install_service_fd(CR_PROC_FD_OFF, fd);
> - close(fd);
> - if (ret < 0)
> + if (ret < 0) {
> + close(fd);
> return -1;
> + }
>
> /*
> * FIXME -- currently we assume that all the tasks live
> diff --git a/criu/fdstore.c b/criu/fdstore.c
> index d9bed4d5e..a729be303 100644
> --- a/criu/fdstore.c
> +++ b/criu/fdstore.c
> @@ -69,9 +69,10 @@ int fdstore_init(void)
> }
>
> ret = install_service_fd(FDSTORE_SK_OFF, sk);
> - close(sk);
> - if (ret < 0)
> + if (ret < 0) {
> + close(sk);
> return -1;
> + }
>
> return 0;
> }
> diff --git a/criu/files.c b/criu/files.c
> index 07cc9cc1d..a966fb931 100644
> --- a/criu/files.c
> +++ b/criu/files.c
> @@ -1659,7 +1659,6 @@ int open_transport_socket(void)
> close(sock);
> return -1;
> }
> - close(sock);
>
> return 0;
> }
> diff --git a/criu/image.c b/criu/image.c
> index 87715fdf9..d437ea7e1 100644
> --- a/criu/image.c
> +++ b/criu/image.c
> @@ -481,7 +481,6 @@ int open_image_dir(char *dir)
> close(fd);
> return -1;
> }
> - close(fd);
> fd = ret;
>
> if (opts.remote) {
> diff --git a/criu/include/servicefd.h b/criu/include/servicefd.h
> index b77922394..bfa9da95d 100644
> --- a/criu/include/servicefd.h
> +++ b/criu/include/servicefd.h
> @@ -32,6 +32,7 @@ extern int clone_service_fd(int id);
> extern int init_service_fd(void);
> extern int get_service_fd(enum sfd_type type);
> extern int reserve_service_fd(enum sfd_type type);
> +extern int do_install_service_fd(enum sfd_type type, int fd);
> extern int install_service_fd(enum sfd_type type, int fd);
> extern int close_service_fd(enum sfd_type type);
> extern bool is_service_fd(int fd, enum sfd_type type);
> diff --git a/criu/log.c b/criu/log.c
> index a2beabdb0..545ae7b97 100644
> --- a/criu/log.c
> +++ b/criu/log.c
> @@ -160,9 +160,10 @@ int log_init(const char *output)
> }
>
> fd = install_service_fd(LOG_FD_OFF, new_logfd);
> - close(new_logfd);
> - if (fd < 0)
> + if (fd < 0) {
> + close(new_logfd);
> goto err;
> + }
>
> return 0;
>
> diff --git a/criu/mount.c b/criu/mount.c
> index 6e5a4c6cf..3a06e8c7d 100644
> --- a/criu/mount.c
> +++ b/criu/mount.c
> @@ -3038,7 +3038,8 @@ static int mntns_set_root_fd(pid_t pid, int fd)
> ret = install_service_fd(ROOT_FD_OFF, fd);
> if (ret >= 0)
> mntns_root_pid = pid;
> - close(fd);
> + else
> + close(fd);
>
> return ret;
> }
> diff --git a/criu/namespaces.c b/criu/namespaces.c
> index 819c75dd5..87a75948d 100644
> --- a/criu/namespaces.c
> +++ b/criu/namespaces.c
> @@ -1691,7 +1691,6 @@ static int start_usernsd(void)
> return -1;
> }
>
> - close(sk[0]);
> return 0;
> }
>
> diff --git a/criu/net.c b/criu/net.c
> index 3afdd5a90..fc117ec58 100644
> --- a/criu/net.c
> +++ b/criu/net.c
> @@ -1872,11 +1872,11 @@ int netns_keep_nsfd(void)
> }
>
> ret = install_service_fd(NS_FD_OFF, ns_fd);
> - if (ret < 0)
> + if (ret < 0) {
> pr_err("Can't install ns net reference\n");
> - else
> + close(ns_fd);
> + } else
> pr_info("Saved netns fd for links restore\n");
> - close(ns_fd);
>
> return ret >= 0 ? 0 : -1;
> }
> diff --git a/criu/tty.c b/criu/tty.c
> index 0ff690c82..bed48c373 100644
> --- a/criu/tty.c
> +++ b/criu/tty.c
> @@ -2133,7 +2133,7 @@ int tty_prep_fds(void)
> else
> stdin_isatty = true;
>
> - if (install_service_fd(SELF_STDIN_OFF, STDIN_FILENO) < 0) {
> + if (do_install_service_fd(SELF_STDIN_OFF, STDIN_FILENO) < 0) {
> pr_err("Can't dup stdin to SELF_STDIN_OFF\n");
> return -1;
> }
> diff --git a/criu/uffd.c b/criu/uffd.c
> index e455b5345..6246f6301 100644
> --- a/criu/uffd.c
> +++ b/criu/uffd.c
> @@ -276,9 +276,10 @@ int prepare_lazy_pages_socket(void)
> return -1;
>
> new_fd = install_service_fd(LAZY_PAGES_SK_OFF, fd);
> - close(fd);
> - if (new_fd < 0)
> + if (new_fd < 0) {
> + close(fd);
> return -1;
> + }
>
> len = offsetof(struct sockaddr_un, sun_path) + strlen(sun.sun_path);
> if (connect(new_fd, (struct sockaddr *) &sun, len) < 0) {
> diff --git a/criu/util.c b/criu/util.c
> index bbe8f2008..ee2a1a8f4 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -267,7 +267,8 @@ static inline int set_proc_pid_fd(int pid, int fd)
>
> open_proc_pid = pid;
> ret = install_service_fd(PROC_PID_FD_OFF, fd);
> - close(fd);
> + if (ret < 0)
> + close(fd);
>
> return ret;
> }
> @@ -301,7 +302,7 @@ void close_proc()
>
> int set_proc_fd(int fd)
> {
> - if (install_service_fd(PROC_FD_OFF, fd) < 0)
> + if (do_install_service_fd(PROC_FD_OFF, fd) < 0)
> return -1;
> return 0;
> }
> @@ -318,9 +319,10 @@ static int open_proc_sfd(char *path)
> }
>
> ret = install_service_fd(PROC_FD_OFF, fd);
> - close(fd);
> - if (ret < 0)
> + if (ret < 0) {
> + close(fd);
> return -1;
> + }
>
> return 0;
> }
> @@ -432,7 +434,7 @@ int reserve_service_fd(enum sfd_type type)
> return sfd;
> }
>
> -int install_service_fd(enum sfd_type type, int fd)
> +int do_install_service_fd(enum sfd_type type, int fd)
> {
> int sfd = __get_service_fd(type, service_fd_id);
>
> @@ -447,6 +449,18 @@ int install_service_fd(enum sfd_type type, int fd)
> return sfd;
> }
>
> +int install_service_fd(enum sfd_type type, int fd)
> +{
> + int sfd;
> +
> + sfd = do_install_service_fd(type, fd);
> + if (sfd < 0)
> + return sfd;
> + if (sfd != fd)
> + close(fd);
> + return sfd;
> +}
> +
> int get_service_fd(enum sfd_type type)
> {
> BUG_ON((int)type <= SERVICE_FD_MIN || (int)type >= SERVICE_FD_MAX);
>
More information about the CRIU
mailing list