
On 06/04/2013 09:35 PM, Daniel P. Berrange wrote:
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@cn.fujitsu.com> --- 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 ?
lxcContainerSetID is only meaningful after we set idmap for user namespace, Actually I just move lxcContainerSetID to the top of lxcContainerChild. It's better to call setuid/setgid as early as we can.
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.
sure, I will wait for your related patches being merged and then update this patchset. Thanks Gao
+ 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