Re: [libvirt] [PATCH v3 04/12] LXC: introduce virLXCControllerSetupUserns and lxcContainerSetID

On Thu, May 23, 2013 at 12:06:48PM +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. > > lxcContainerSetID 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(a)cn.fujitsu.com&gt; > --- > src/lxc/lxc_container.c | 63 ++++++++++++++++++++++++++++++++-------------- > src/lxc/lxc_controller.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 110 insertions(+), 18 deletions(-) > > diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c > index 618252c..52fcf39 100644 > --- a/src/lxc/lxc_container.c > +++ b/src/lxc/lxc_container.c > @@ -335,6 +335,30 @@ int lxcContainerWaitForContinue(int control) > > > /** > + * lxcContainerSetID: > + * > + * 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 lxcContainerSetID(virDomainDefPtr def) > +{ > + /* Only call virSetUIDGID when user namespace is enabled > + * for this container. And user namespace is only enabled > + * when nuidmap&ngidmap is not zero */ > + > + if (def->idmap.nuidmap && virSetUIDGID(0, 0) < 0) { > + virReportSystemError(errno, "%s", > + _("setuid or setgid failed")); > + return -1; > + } > + > + return 0; > +} > + > + > +/** > * lxcContainerRenameAndEnableInterfaces: > * @nveths: number of interfaces > * @veths: interface names > @@ -1937,6 +1961,27 @@ static int lxcContainerChild(void *data) > cmd = lxcContainerBuildInitCmd(vmDef); > virCommandWriteArgLog(cmd, 1); > > + if (lxcContainerResolveSymlinks(vmDef) < 0) > + goto cleanup; > + > + if (!virFileExists(vmDef->os.init)) { > + virReportSystemError(errno, > + _("cannot find init path '%s' relative to container root"), > + vmDef->os.init); > + goto cleanup; > + } > + > + /* Wait for interface devices to show up */ > + if (lxcContainerWaitForContinue(argv->monitor) < 0) { > + virReportSystemError(errno, "%s", > + _("Failed to read the container continue message")); > + goto cleanup; > + } > + VIR_DEBUG("Received container continue message"); > + > + if (lxcContainerSetID(vmDef) < 0) > + goto cleanup; > + > root = virDomainGetRootFilesystem(vmDef); > > if (argv->nttyPaths) { > @@ -1962,29 +2007,11 @@ static int lxcContainerChild(void *data) > goto cleanup; > } > > - if (lxcContainerResolveSymlinks(vmDef) < 0) > - goto cleanup; > - > if (lxcContainerSetupPivotRoot(vmDef, root, > argv->ttyPaths, argv->nttyPaths, > argv->securityDriver) < 0) > goto cleanup; > > - if (!virFileExists(vmDef->os.init)) { > - virReportSystemError(errno, > - _("cannot find init path '%s' relative to container root"), > - vmDef->os.init); > - goto cleanup; > - } > - > - /* Wait for interface devices to show up */ > - if (lxcContainerWaitForContinue(argv->monitor) < 0) { > - virReportSystemError(errno, "%s", > - _("Failed to read the container continue message")); > - goto cleanup; > - } > - VIR_DEBUG("Received container continue message"); Why did you need to move these functions before the lxcContainerSetID call ?

> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c > index dfe686b..0a2e3ac 100644 > --- a/src/lxc/lxc_controller.c > +++ b/src/lxc/lxc_controller.c > @@ -1122,6 +1122,68 @@ cleanup2: > } > > > +static int > +virLXCControllerSetupUsernsMap(virDomainIdMapEntryPtr map, > + char *path, > + int num) > +{ > + virBuffer map_value = VIR_BUFFER_INITIALIZER; > + int i, ret = -1; > + > + for (i = 0; i < num; i++) > + virBufferAsprintf(&map_value, "%u %u %u\n", > + map[i].start, map[i].target, map[i].count); > + You need to check virBufferError() to see if OOM has occured here.

> + if (virFileWriteStr(path, virBufferCurrentContent(&map_value), 0) < 0) { > + virReportSystemError(errno, _("unable write to %s"), path); > + goto cleanup; > + } > + ret = 0; > + > +cleanup: > + virBufferFreeAndReset(&map_value); > + return ret; > +} Daniel