[lxc-devel] [lxc/master] idmap: gather output of new{g,u}idmap

brauner on Github lxc-bot at linuxcontainers.org
Sun May 28 14:49:30 UTC 2017


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170528/ab35413e/attachment.bin>
-------------- next part --------------
From 5e80f6695d278412343ffab0a29ba3fc8027fc55 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sat, 27 May 2017 08:16:01 +0200
Subject: [PATCH 1/2] conf: improve write_id_mapping()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/conf.c | 43 ++++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 85805f975..c05a8f1ea 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -3384,27 +3384,32 @@ int lxc_assign_network(const char *lxcpath, char *lxcname,
 static int write_id_mapping(enum idtype idtype, pid_t pid, const char *buf,
 			    size_t buf_size)
 {
-	char path[PATH_MAX];
-	int ret, closeret;
-	FILE *f;
+	char path[MAXPATHLEN];
+	int fd, ret;
 
-	ret = snprintf(path, PATH_MAX, "/proc/%d/%cid_map", pid, idtype == ID_TYPE_UID ? 'u' : 'g');
-	if (ret < 0 || ret >= PATH_MAX) {
-		fprintf(stderr, "%s: path name too long\n", __func__);
+	ret = snprintf(path, MAXPATHLEN, "/proc/%d/%cid_map", pid,
+		       idtype == ID_TYPE_UID ? 'u' : 'g');
+	if (ret < 0 || ret >= MAXPATHLEN) {
+		ERROR("failed to create path \"%s\"", path);
 		return -E2BIG;
 	}
-	f = fopen(path, "w");
-	if (!f) {
-		perror("open");
-		return -EINVAL;
+
+	fd = open(path, O_WRONLY);
+	if (fd < 0) {
+		SYSERROR("failed to open \"%s\"", path);
+		return -1;
 	}
-	ret = fwrite(buf, buf_size, 1, f);
-	if (ret < 0)
-		SYSERROR("writing id mapping");
-	closeret = fclose(f);
-	if (closeret)
-		SYSERROR("writing id mapping");
-	return ret < 0 ? ret : closeret;
+
+	ret = write(fd, buf, buf_size);
+	if (ret != buf_size)
+		SYSERROR("failed to write %cid mapping to \"%s\"",
+			 idtype == ID_TYPE_UID ? 'u' : 'g', path);
+	else
+		ret = 0;
+
+	close(fd);
+
+	return ret;
 }
 
 /* Check whether a binary exist and has either CAP_SETUID, CAP_SETGID or both. */
@@ -4671,6 +4676,8 @@ static struct lxc_list *idmap_add_id(struct lxc_conf *conf,
 		entry->hostid = (unsigned long) uid;
 		entry->range = 1;
 		lxc_list_add_tail(new, tmp);
+		DEBUG("adding uid mapping: nsid %lu hostid %lu range %lu",
+		      entry->nsid, entry->hostid, entry->range);
 	}
 	if (hostgid_mapped < 0) {
 		hostgid_mapped = find_unmapped_nsuid(conf, ID_TYPE_GID);
@@ -4690,6 +4697,8 @@ static struct lxc_list *idmap_add_id(struct lxc_conf *conf,
 		entry->hostid = (unsigned long) gid;
 		entry->range = 1;
 		lxc_list_add_tail(new, tmp);
+		DEBUG("adding gid mapping: nsid %lu hostid %lu range %lu",
+		      entry->nsid, entry->hostid, entry->range);
 	}
 	lxc_list_for_each_safe(it, &conf->id_map, next) {
 		tmp = malloc(sizeof(*tmp));

From b29dfe70c640c30f287d0eefa0b9e82bdf38e88f Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sun, 28 May 2017 15:10:35 +0200
Subject: [PATCH 2/2] conf: rework lxc_map_ids()

Especially, in case the new{g,u}idmap is used we should try to gather
std{err,out} on error to better debug what is going on.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/conf.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 102 insertions(+), 23 deletions(-)

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index c05a8f1ea..51c2a8642 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -3477,11 +3477,28 @@ int lxc_map_ids(struct lxc_list *idmap, pid_t pid)
 	struct id_map *map;
 	struct lxc_list *iterator;
 	enum idtype type;
+	char u_or_g;
 	char *pos;
-	int euid;
-	int ret = 0, use_shadow = 0;
-	int uidmap = 0, gidmap = 0;
-	char *buf = NULL;
+	int euid, fill, left;
+	pid_t child;
+	int pipefd[2];
+	ssize_t bytes;
+	char output[MAXPATHLEN];
+	char pidstr[LXC_NUMSTRLEN64];
+	/* strlen("new at idmap") = 9
+	 * +
+	 * strlen(" ") = 1
+	 * +
+	 * LXC_NUMSTRLEN64
+	 * +
+	 * strlen(" ") = 1
+	 *
+	 * We add some additional space to make sure that we really have
+	 * LXC_IDMAPLEN bytes available for our the {g,u]id mapping.
+	 */
+	char mapbuf[9 + 1 + LXC_NUMSTRLEN64 + 1 + LXC_IDMAPLEN] = {0};
+	int fret = 0, ret = 0, use_shadow = 0, uidmap = 0, gidmap = 0;
+	bool had_entry = false;
 
 	euid = geteuid();
 
@@ -3506,17 +3523,12 @@ int lxc_map_ids(struct lxc_list *idmap, pid_t pid)
 		return -1;
 	}
 
-	for (type = ID_TYPE_UID; type <= ID_TYPE_GID; type++) {
-		int left, fill;
-		bool had_entry = false;
-		if (!buf) {
-			buf = pos = malloc(LXC_IDMAPLEN);
-			if (!buf)
-				return -ENOMEM;
-		}
-		pos = buf;
+	for (type = ID_TYPE_UID, u_or_g = 'u'; type <= ID_TYPE_GID;
+	     type++, type == ID_TYPE_GID ? (u_or_g = 'g') : (u_or_g = '@')) {
+		pos = mapbuf;
+
 		if (use_shadow)
-			pos += sprintf(buf, "new%cidmap %d", type == ID_TYPE_UID ? 'u' : 'g', pid);
+			pos += sprintf(mapbuf, "new%cidmap %d", u_or_g, pid);
 
 		lxc_list_for_each(iterator, idmap) {
 			/* The kernel only takes <= 4k for writes to
@@ -3528,7 +3540,7 @@ int lxc_map_ids(struct lxc_list *idmap, pid_t pid)
 
 			had_entry = true;
 
-			left = LXC_IDMAPLEN - (pos - buf);
+			left = LXC_IDMAPLEN - (pos - mapbuf);
 			fill = snprintf(pos, left, "%s%lu %lu %lu%s",
 					use_shadow ? " " : "", map->nsid,
 					map->hostid, map->range,
@@ -3541,22 +3553,89 @@ int lxc_map_ids(struct lxc_list *idmap, pid_t pid)
 		if (!had_entry)
 			continue;
 
-		if (!use_shadow) {
-			ret = write_id_mapping(type, pid, buf, pos - buf);
+		/* Try to catch the ouput of new{g,u}idmap to make debugging
+		 * easier.
+		 */
+		if (use_shadow) {
+			if (pipe(pipefd) < 0) {
+				SYSERROR("failed to create pipe");
+				return -1;
+			}
+
+			child = fork();
+			if (child < 0) {
+				close(pipefd[0]);
+				close(pipefd[1]);
+				SYSERROR("failed to create new process");
+				return -1;
+			}
+
+			if (child == 0) {
+				/* Close the read-end of the pipe. */
+				close(pipefd[0]);
+
+				/* Redirect std{err,out} to write-end of the
+				 * pipe. */
+				ret = dup2(pipefd[1], STDOUT_FILENO);
+				if (ret >= 0)
+					ret = dup2(pipefd[1], STDERR_FILENO);
+
+				/* Close the write-end of the pipe. */
+				close(pipefd[1]);
+
+				if (ret < 0) {
+					SYSERROR("failed to duplicate std{err,out} "
+						 "file descriptor");
+					exit(EXIT_FAILURE);
+				}
+
+				ret = snprintf(pidstr, sizeof(pidstr), "%d", pid);
+				if (ret < 0 || ret >= sizeof(pidstr)) {
+					ERROR("failed to create string");
+					exit(EXIT_FAILURE);
+				}
+
+				/*
+				 * new{g,u}idmap checks argc and argv. Both will
+				 * be "wrong" from its perspective if we pass
+				 * the requested id maps as a single string. So
+				 * let's go through "/bin/sh -c".
+				 */
+				execl("/bin/sh", "sh", "-c", mapbuf, (char *)NULL);
+				SYSERROR("failed to exec \"/bin/sh -c %s\"", mapbuf);
+				exit(EXIT_FAILURE);
+			}
+
+			/* close the write-end of the pipe */
+			close(pipefd[1]);
+
+			bytes = read(pipefd[0], &output, MAXPATHLEN);
+			if (bytes > 0)
+				output[bytes - 1] = '\0';
+
+			if (wait_for_pid(child) != 0) {
+				close(pipefd[0]);
+				ERROR("new%cidmap failed to write mapping: %s",
+				      u_or_g, bytes > 0 ? output : "(null)");
+				return -1;
+			}
+
+			/* close the read-end of the pipe */
+			close(pipefd[0]);
 		} else {
-			left = LXC_IDMAPLEN - (pos - buf);
+			/* Add final newline. */
+			left = LXC_IDMAPLEN - (pos - mapbuf);
 			fill = snprintf(pos, left, "\n");
 			if (fill <= 0 || fill >= left)
 				SYSERROR("Too many {g,u}id mappings defined.");
 			pos += fill;
-			ret = system(buf);
+			fret = write_id_mapping(type, pid, mapbuf, pos - mapbuf);
 		}
-		if (ret)
-			break;
+
+		memset(mapbuf, 0, sizeof(mapbuf));
 	}
 
-	free(buf);
-	return ret;
+	return fret;
 }
 
 /*


More information about the lxc-devel mailing list