Merge pull request #686 from hqhq/hq_refactor_nsexec

Refactor nsexec.c and add some comments

libcontainer/nsenter/nsexec.c

@@ -39,6 +39,7 @@ struct nsenter_config {
 	char     *gidmap;
 	int      gidmap_len;
 	uint8_t  is_setgroup;
+	int      consolefd;
 };
 
 // list of known message types we want to send to bootstrap program
@@ -86,6 +87,8 @@ static int clone_parent(jmp_buf *env, int flags)
 	ca.env = env;
 	child  = clone(child_func, ca.stack_ptr, CLONE_PARENT | SIGCHLD | flags,
 		      &ca);
+	// On old kernels, CLONE_PARENT cannot work with CLONE_NEWPID,
+	// unshare before clone to workaround this.
 	if (child == -1 && errno == EINVAL) {
 		if (unshare(flags)) {
 			pr_perror("Unable to unshare namespaces");
@@ -167,10 +170,12 @@ static void update_process_idmap(char *pathfmt, int pid, char *map, int map_len)
 	len = write(fd, map, map_len);
 	if (len == -1) {
 		pr_perror("failed to write to %s", buf);
+		close(fd);
 		exit(1);
 	} else if (len != map_len) {
-		fprintf(stderr, "Failed to write data to %s (%d/%d)",
+		pr_perror("Failed to write data to %s (%d/%d)",
 			buf, len, map_len);
+		close(fd);
 		exit(1);
 	}
 
@@ -214,6 +219,7 @@ static void update_process_gidmap(int pid, uint8_t is_setgroup, char *map, int m
 			// ENOENT; this is OK.
 			if (errno != ENOENT) {
 				pr_perror("failed to write allow to %s", buf);
+				close(fd);
 				exit(1);
 			}
 		}
@@ -244,7 +250,6 @@ static void start_child(int pipenum, jmp_buf *env, int syncpipe[2],
 	// update uid_map and gid_map for the child process if they
 	// were provided
 	update_process_uidmap(childpid, config->uidmap, config->uidmap_len);
-
 	update_process_gidmap(childpid, config->is_setgroup, config->gidmap, config->gidmap_len);
 
 	// Send the sync signal to the child
@@ -266,16 +271,14 @@ static void start_child(int pipenum, jmp_buf *env, int syncpipe[2],
 	exit(0);
 }
 
-static void process_nl_attributes(int pipenum, char *data, int data_size)
+static struct nsenter_config process_nl_attributes(int pipenum, char *data, int data_size)
 {
-	jmp_buf			env;
 	struct nsenter_config	config	    = {0};
 	struct nlattr		*nlattr;
 	int			payload_len;
 	int			start       = 0;
-	int			consolefd   = -1;
-	int			syncpipe[2] = {-1, -1};
 
+	config.consolefd = -1;
 	while (start < data_size) {
 		nlattr = (struct nlattr *)(data + start);
 		start += NLA_HDRLEN;
@@ -286,8 +289,8 @@ static void process_nl_attributes(int pipenum, char *data, int data_size)
 		} else if (nlattr->nla_type == CONSOLE_PATH_ATTR) {
 			// get the console path before setns because it may
 			// change mnt namespace
-			consolefd = open(data + start, O_RDWR);
+			config.consolefd = open(data + start, O_RDWR);
-			if (consolefd < 0) {
+			if (config.consolefd < 0) {
 				pr_perror("Failed to open console %s",
 					  data + start);
 				exit(1);
@@ -295,7 +298,7 @@ static void process_nl_attributes(int pipenum, char *data, int data_size)
 		} else if (nlattr->nla_type == NS_PATHS_ATTR) {
 			// if custom namespaces are required, open all
 			// descriptors and perform setns on them
-			int	i;
+			int	i, j;
 			int	nslen = num_namespaces(data + start);
 			int	fds[nslen];
 			char	*nslist[nslen];
@@ -314,6 +317,9 @@ static void process_nl_attributes(int pipenum, char *data, int data_size)
 				}
 				fds[i] = open(ns, O_RDONLY);
 				if (fds[i] == -1) {
+					for (j = 0; j < i; j++) {
+						close(fds[j]);
+					}
 					pr_perror("Failed to open %s", ns);
 					exit(1);
 				}
@@ -344,14 +350,61 @@ static void process_nl_attributes(int pipenum, char *data, int data_size)
 		start += NLA_ALIGN(payload_len);
 	}
 
+	return config;
+}
+
+void nsexec(void)
+{
+	int pipenum;
+
+	// If we don't have init pipe, then just return to the go routine,
+	// we'll only have init pipe for start or exec
+	pipenum = get_init_pipe();
+	if (pipenum == -1) {
+		return;
+	}
+
+	// Retrieve the netlink header
+	struct nlmsghdr nl_msg_hdr;
+	int		len;
+
+	if ((len = read(pipenum, &nl_msg_hdr, NLMSG_HDRLEN)) != NLMSG_HDRLEN) {
+		pr_perror("Invalid netlink header length %d", len);
+		exit(1);
+	}
+
+	if (nl_msg_hdr.nlmsg_type == NLMSG_ERROR) {
+		pr_perror("Failed to read netlink message");
+		exit(1);
+	}
+
+	if (nl_msg_hdr.nlmsg_type != INIT_MSG) {
+		pr_perror("Unexpected msg type %d", nl_msg_hdr.nlmsg_type);
+		exit(1);
+	}
+
+	// Retrieve data
+	int  nl_total_size = NLMSG_PAYLOAD(&nl_msg_hdr, 0);
+	char data[nl_total_size];
+
+	if ((len = read(pipenum, data, nl_total_size)) != nl_total_size) {
+		pr_perror("Failed to read netlink payload, %d != %d", len,
+			  nl_total_size);
+		exit(1);
+	}
+
+	jmp_buf	env;
+	int	syncpipe[2] = {-1, -1};
+	struct	nsenter_config config = process_nl_attributes(pipenum,
+						data, nl_total_size);
+
 	// required clone_flags to be passed
 	if (config.cloneflags == -1) {
 		pr_perror("Missing clone_flags");
 		exit(1);
 	}
 	// prepare sync pipe between parent and child. We need this to let the
-	// child
+	// child know that the parent has finished setting up
-	// know that the parent has finished setting up
 	if (pipe(syncpipe) != 0) {
 		pr_perror("Failed to setup sync pipe between parent and child");
 		exit(1);
@@ -360,6 +413,7 @@ static void process_nl_attributes(int pipenum, char *data, int data_size)
 	if (setjmp(env) == 1) {
 		// Child
 		uint8_t s = 0;
+		int	consolefd = config.consolefd;
 
 		// close the writing side of pipe
 		close(syncpipe[1]);
@@ -416,45 +470,3 @@ static void process_nl_attributes(int pipenum, char *data, int data_size)
 	// Parent
 	start_child(pipenum, &env, syncpipe, &config);
 }
-
-void nsexec(void)
-{
-	int pipenum;
-
-	// if we dont have init pipe, then just return to the parent
-	pipenum = get_init_pipe();
-	if (pipenum == -1) {
-		return;
-	}
-
-	// Retrieve the netlink header
-	struct nlmsghdr nl_msg_hdr;
-	int		len;
-
-	if ((len = read(pipenum, &nl_msg_hdr, NLMSG_HDRLEN)) != NLMSG_HDRLEN) {
-		pr_perror("Invalid netlink header length %d", len);
-		exit(1);
-	}
-
-	if (nl_msg_hdr.nlmsg_type == NLMSG_ERROR) {
-		pr_perror("Failed to read netlink message");
-		exit(1);
-	}
-
-	if (nl_msg_hdr.nlmsg_type != INIT_MSG) {
-		pr_perror("Unexpected msg type %d", nl_msg_hdr.nlmsg_type);
-		exit(1);
-	}
-
-	// Retrieve data
-	int  nl_total_size = NLMSG_PAYLOAD(&nl_msg_hdr, 0);
-	char data[nl_total_size];
-
-	if ((len = read(pipenum, data, nl_total_size)) != nl_total_size) {
-		pr_perror("Failed to read netlink payload, %d != %d", len,
-			  nl_total_size);
-		exit(1);
-	}
-
-	process_nl_attributes(pipenum, data, nl_total_size);
-}