On Fri, May 10, 2013 at 05:58:13PM +0800, Gao feng wrote:
This patch introduces new helper function virLXCControllerSetupUserns, in this function, we set the files uid_map and gid_map of the init task of container.
lxcContainerSetUserns is used for creating cred for tasks running in container. Since after setuid/setgid, we may be a new user. This patch calls lxcContainerSetUserns at first to make sure the new created files belong to right user.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 59 +++++++++++++++++++++++++++------------- src/lxc/lxc_controller.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 18 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 094f205..eabaa34 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -331,6 +331,26 @@ int lxcContainerWaitForContinue(int control)
/** + * lxcContainerSetUserns: + * + * This function calls setuid and setgid to create proper + * cred for tasks running in container. + * + * Returns 0 on success or -1 in case of error + */ +static int lxcContainerSetUserns(virDomainDefPtr def)
I think it'd be better named as s/lxcContainerSetUserns/lxcContainerSetID/
+{ + if (def->idmap.nuidmap && def->idmap.ngidmap && virSetUIDGID(0, 0) < 0) {
This doesn't do the right thing if "nuidmap > 0 && ngidmap == 0" or vica-verca. Better to write it as uid_t uid = (uid_t)-1; gid_t gid = (gid_t)-1; if (def->idmap.nuidmap) uid = 0; if (def->idmap.ngidmap) gid = 0; if ((def->idmap.nuidmap || def->idmap.ngidmap) && virSetUIDGID(uid, gid) < 0) { ...error... } (virSetUIDGID accepts -1 as meaning don't change that id)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 8c358c3..e9b90bf 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1036,6 +1036,73 @@ cleanup2: }
+static int virLXCControllerSetupUsernsMap(struct idmap *map, + char *path, + int num) +{ + char *map_value = NULL; + int i, ret = -1; + + for (i = 0; i < num; i++) { + if (virAsprintf(&map_value, "%s %u %u %u\n", + map_value ? map_value : "", + map[i].start, map[i].target, map[i].count) < 0) + goto cleanup; + }
You're leaking memory on every iteration of this loop by ovewriting the map_value pointer each time. You should use the virBuffer APIs to build up a string incrementally.
+ + if (virFileWriteStr(path, map_value, 0) < 0) { + if (errno == -ENOENT) + virReportSystemError(errno, + _("%s doesn't exist, please disable userns"), + path);
This special errno check shouldn't be required if your earlier check for existance of user ns is correctly called.
+ virReportSystemError(errno, _("unable write to %s"), path); + goto cleanup; + } + ret = 0; + +cleanup: + VIR_FREE(map_value); + return ret; +} + +/** + * virLXCControllerSetupUserns + * + * Set proc files for user namespace + * + * Returns 0 on success or -1 in case of error + */ +static int virLXCControllerSetupUserns(virLXCControllerPtr ctrl) +{ + char *uid_map = NULL; + char *gid_map = NULL; + int ret = -1; + + if (ctrl->def->idmap.nuidmap == 0 || ctrl->def->idmap.ngidmap == 0) + return 0;
This implies that the user must supply both a UID map and a GID map or neither. If it is an error to provide one and not the other, then this is something you should enforce during parsing.
+ + if (virAsprintf(&uid_map, "/proc/%d/uid_map", ctrl->initpid) < 0) + goto cleanup; + + if (virLXCControllerSetupUsernsMap(ctrl->def->idmap.uidmap, uid_map, + ctrl->def->idmap.nuidmap) < 0) + goto cleanup; + + if (virAsprintf(&gid_map, "/proc/%d/gid_map", ctrl->initpid) < 0) + goto cleanup; + + if (virLXCControllerSetupUsernsMap(ctrl->def->idmap.gidmap, gid_map, + ctrl->def->idmap.ngidmap) < 0) + goto cleanup; + + ret = 0; +cleanup: + VIR_FREE(uid_map); + VIR_FREE(gid_map); + return ret; +}
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|