util: move open_proc_self_fd to service_fd

We need this to avoid conflicts with file descriptors which has to be
restored. Currently open_proc PROC_SELF is not used during restoring
file descriptors, but we are going to use it for memfd restore.

Note: in get_proc_fd let's not close service fd if we detect service fd
is not ours, it will be replaced in open_pid_proc anyway, and e.g. in
protected sfd context we can't close or open sfd, but can replace it
without any problems.

While on it also add FIXME because the check in get_proc_fd is error
prone in some rare cases (nested pidns is not supported yet).

We need to populate this new SELF service fd in populate_pid_proc, so
that it is later available in protected context. Also don't close
/proc/self service fd in prep_unix_sk_cwd as it can be called from
protected context and there is no much point of closing it anyway.

Close proc self servicefd in close_old_fds, because first we don't wan't
to reuse it from some ancestor, second there can be some junk fd as we
are yet only in the beginning of close_old_fds. This junk fd can come
from service fds of other tasks from parent's shared fdtable, and this
fd would not allow us to do opendir_proc(PROC_SELF).

Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

criu/cr-restore.c

@@ -418,6 +418,10 @@ static int populate_pid_proc(void)
 		pr_err("Can't open PROC_SELF\n");
 		return -1;
 	}
+	if (open_pid_proc(PROC_SELF) < 0) {
+		pr_err("Can't open PROC_SELF\n");
+		return -1;
+	}
 	return 0;
 }
 

criu/files.c

@@ -1256,6 +1256,14 @@ int close_old_fds(void)
 	struct dirent *de;
 	int fd, ret;
 
+	/**
+	 * Close previous /proc/self/ service fd, as we don't wan't to reuse it
+	 * from a different task. Also there can be some junk fd in it's place
+	 * after we've moved our service fds (e.g. from other task of parents
+	 * shared fdtable), we need to close it before opendir_proc() below.
+	 */
+	__close_service_fd(PROC_SELF_FD_OFF);
+
 	dir = opendir_proc(PROC_SELF, "fd");
 	if (dir == NULL)
 		return -1;
@@ -1297,7 +1305,6 @@ int prepare_fds(struct pstree_item *me)
 	sfds_protected = false;
 	close_service_fd(CGROUP_YARD);
 	sfds_protected = true;
-	set_proc_self_fd(-1); /* flush any proc cached fds we may have */
 
 	if (rsti(me)->fdt) {
 		struct fdt *fdt = rsti(me)->fdt;

criu/include/servicefd.h

@@ -17,6 +17,7 @@ enum sfd_type {
 	IMG_STREAMER_FD_OFF,
 	PROC_FD_OFF,		/* fd with /proc for all proc_ calls */
 	PROC_PID_FD_OFF,
+	PROC_SELF_FD_OFF,
 	CR_PROC_FD_OFF,		/* some other's proc fd:
 				 *  - For dump -- target ns' proc
 				 *  - For restore -- CRIU ns' proc
@@ -44,6 +45,7 @@ extern bool is_service_fd(int fd, enum sfd_type type);
 extern int service_fd_min_fd(struct pstree_item *item);
 extern int install_service_fd(enum sfd_type type, int fd);
 extern int close_service_fd(enum sfd_type type);
+extern void __close_service_fd(enum sfd_type type);
 extern int clone_service_fd(struct pstree_item *me);
 
 #endif /* __CR_SERVICE_FD_H__ */

criu/include/util.h

@@ -180,7 +180,7 @@ extern int cr_daemon(int nochdir, int noclose, int close_fd);
 extern int status_ready(void);
 extern int is_root_user(void);
 
-extern void set_proc_self_fd(int fd);
+extern int set_proc_self_fd(int fd);
 
 static inline bool dir_dots(const struct dirent *de)
 {

criu/servicefd.c

@@ -46,6 +46,7 @@ const char *sfd_type_name(enum sfd_type type)
 		[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),
+		[PROC_SELF_FD_OFF]	= __stringify_1(PROC_SELF_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),
@@ -206,6 +207,15 @@ int close_service_fd(enum sfd_type type)
 	return 0;
 }
 
+void __close_service_fd(enum sfd_type type)
+{
+	int fd;
+
+	fd = __get_service_fd(type, service_fd_id);
+	close(fd);
+	clear_bit(type, sfd_map);
+}
+
 static int move_service_fd(struct pstree_item *me, int type, int new_id, int new_base)
 {
 	int old = get_service_fd(type);

criu/sk-unix.c

@@ -1211,7 +1211,6 @@ static int prep_unix_sk_cwd(struct unix_sk_info *ui, int *prev_cwd_fd,
 		if (switch_ns_by_fd(ns_fd, &mnt_ns_desc, prev_mntns_fd))
 			return -1;
 
-		set_proc_self_fd(-1);
 		close(ns_fd);
 	}
 

criu/util.c

@@ -281,15 +281,18 @@ int move_fd_from(int *img_fd, int want_fd)
 
 static pid_t open_proc_pid = PROC_NONE;
 static pid_t open_proc_self_pid;
-static int open_proc_self_fd = -1;
 
-void set_proc_self_fd(int fd)
+int set_proc_self_fd(int fd)
 {
-	if (open_proc_self_fd >= 0)
-		close(open_proc_self_fd);
+	int ret;
+
+	if (fd < 0)
+		return close_service_fd(PROC_SELF_FD_OFF);
 
-	open_proc_self_fd = fd;
 	open_proc_self_pid = getpid();
+	ret = install_service_fd(PROC_SELF_FD_OFF, fd);
+
+	return ret;
 }
 
 static inline int set_proc_pid_fd(int pid, int fd)
@@ -308,10 +311,18 @@ static inline int set_proc_pid_fd(int pid, int fd)
 static inline int get_proc_fd(int pid)
 {
 	if (pid == PROC_SELF) {
-		if (open_proc_self_fd != -1 && open_proc_self_pid != getpid()) {
-			close(open_proc_self_fd);
+		int open_proc_self_fd;
+
+		open_proc_self_fd = get_service_fd(PROC_SELF_FD_OFF);
+		/**
+		 * FIXME in case two processes from different pidnses have the
+		 * same pid from getpid() and one inherited service fds from
+		 * another or they share them by shared fdt - this check will
+		 * not detect that one of them reuses /proc/self of another.
+		 * Everything proc related may break in this case.
+		 */
+		if (open_proc_self_fd >= 0 && open_proc_self_pid != getpid())
 			open_proc_self_fd = -1;
-		}
 		return open_proc_self_fd;
 	} else if (pid == open_proc_pid)
 		return get_service_fd(PROC_PID_FD_OFF);
@@ -402,7 +413,7 @@ inline int open_pid_proc(pid_t pid)
 	}
 
 	if (pid == PROC_SELF)
-		set_proc_self_fd(fd);
+		fd = set_proc_self_fd(fd);
 	else
 		fd = set_proc_pid_fd(pid, fd);