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

Submitted by Cyrill Gorcunov on April 5, 2019, 9:47 a.m.

Details

Message ID 20190405094748.GU1421@uranus.lan
State Accepted
Series "sfd: Move into separate file and raise limits early"
Headers show

Commit Message

Cyrill Gorcunov April 5, 2019, 9:47 a.m.
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 v2: use sfd map where appropriate

 criu/include/servicefd.h | 19 +++++++++++++++++
 criu/servicefd.c         | 44 +++++++++++++++++++++++++++++++++++-----
 2 files changed, 58 insertions(+), 5 deletions(-)

Patch hide | download patch | download mbox

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..d7e81836b072 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,14 @@  int install_service_fd(enum sfd_type type, int fd)
 		return fd;
 	}
 
+	if (!test_bit(type, sfd_map)) {
+		if (sfd_verify_target(type, fd, sfd))
+			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 +196,14 @@  static void move_service_fd(struct pstree_item *me, int type, int new_id, int ne
 	if (old < 0)
 		return;
 
+	if (!test_bit(type, sfd_map))
+		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);
 }