The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/1592
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === Signed-off-by: Christian Brauner <[email protected]>
From 5e80f6695d278412343ffab0a29ba3fc8027fc55 Mon Sep 17 00:00:00 2001 From: Christian Brauner <[email protected]> Date: Sat, 27 May 2017 08:16:01 +0200 Subject: [PATCH 1/2] conf: improve write_id_mapping() Signed-off-by: Christian Brauner <[email protected]> --- 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 <[email protected]> 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 <[email protected]> --- 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@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; } /*
_______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
