This patch introduces new element <idmap> for user namespace. for example <idmap> <uid start='0' target='1000' count='10'/> <gid start='0' target='1000' count='10'/> </idmap> this new element is used for setting proc files /proc/<pid>/{uid_map,gid_map}. This patch also supports multiple uid/gid elements setting in XML configuration. We don't support the semi configuation, user has to configure uid and gid both. Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com&gt; --- docs/formatdomain.html.in | 23 +++++++++++ docs/schemas/domaincommon.rng | 31 ++++++++++++++ src/conf/domain_conf.c | 95 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 22 ++++++++++ 4 files changed, 171 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3a200aa..cc0ae52 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -285,6 +285,29 @@ </pre> + <p> + If you want to enable user namespace,set the <code>idmap</code> element. + the <code>uid</code> and <code>gid</code> elements have three attributes: + </p> + + <dl> + <dt><code>start</code></dt> + <dd>First user id in container.</dd> + <dt><code>target</code></dt> + <dd>The first user id in container will be mapped to this target user + id in host.</dd> + <dt><code>count</code></dt> + <dd>How many users in container being allowed to map to host's user.</dd> + </dl> + + <pre> + &lt;idmap&gt; + &lt;uid start='0' target='1000' count='10'/&gt; + &lt;gid start='0' target='1000' count='10'/&gt; + &lt;/idmap&gt; + </pre> + + <h3><a name="elementsSysinfo">SMBIOS System Information</a></h3> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3cace35..ac07407 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -56,6 +56,9 @@ <ref name="pm"/> </optional> <optional> + <ref name="idmap"/> + </optional> + <optional> <ref name="devices"/> </optional> <zeroOrMore> @@ -465,6 +468,34 @@ </optional> </interleave> </define> + <define name="idmap"> + <zeroOrMore> + <element name="uid"> + <attribute name="start"> + <ref name="unsignedInt"/> + </attribute> + <attribute name="target"> + <ref name="unsignedInt"/> + </attribute> + <attribute name="count"> + <ref name="unsignedInt"/> + </attribute> + </element> + </zeroOrMore> + <zeroOrMore> + <element name="gid"> + <attribute name="start"> + <ref name="unsignedInt"/> + </attribute> + <attribute name="target"> + <ref name="unsignedInt"/> + </attribute> + <attribute name="count"> + <ref name="unsignedInt"/> + </attribute> + </element> + </zeroOrMore> + </define> <!-- Resources usage defines the amount of memory (maximum and possibly current usage) and number of virtual CPUs used by that domain. diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ad5550c..a3c5c84 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1939,6 +1939,9 @@ void virDomainDefFree(virDomainDefPtr def) virDomainTPMDefFree(def->tpm); + VIR_FREE(def->idmap.uidmap); + VIR_FREE(def->idmap.gidmap); + VIR_FREE(def->os.type); VIR_FREE(def->os.machine); VIR_FREE(def->os.init); @@ -10057,6 +10060,40 @@ cleanup: return ret; } + +/* Parse the XML definition for user namespace id map. + * + * idmap has the form of + * + * <uid start='0' target='1000' count='10'/> + * <gid start='0' target='1000' count='10'/> + */ +static virDomainIdMapEntryPtr +virDomainIdmapDefParseXML(const xmlNodePtr *node, + xmlXPathContextPtr ctxt, + ssize_t num) +{ + int i; + virDomainIdMapEntryPtr idmap = NULL; + xmlNodePtr save_ctxt = ctxt->node; + + if (VIR_ALLOC_N(idmap, num) < 0) { + virReportOOMError(); + goto error; + } + + for (i = 0; i < num; i++) { + ctxt->node = node[i]; + virXPathUInt("string(./@start)", ctxt, &idmap[i].start); + virXPathUInt("string(./@target)", ctxt, &idmap[i].target); + virXPathUInt("string(./@count)", ctxt, &idmap[i].count); + } + error: + ctxt->node = save_ctxt; + return idmap; +} + + /* Parse the XML definition for a vcpupin or emulatorpin. * * vcpupin has the form of @@ -11804,6 +11841,43 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(nodes); + /* analysis of the user namespace mapping */ + def->idmap.nuidmap = 0; + def->idmap.uidmap = NULL; + if ((n = virXPathNodeSet("./idmap/uid", ctxt, &nodes)) < 0) + goto error; + + if (n) { + def->idmap.uidmap = virDomainIdmapDefParseXML(nodes, ctxt, n); + if (!def->idmap.uidmap) + goto error; + + def->idmap.nuidmap = n; + } + VIR_FREE(nodes); + + def->idmap.ngidmap = 0; + def->idmap.gidmap = NULL; + + if ((n = virXPathNodeSet("./idmap/gid", ctxt, &nodes)) < 0) + goto error; + + if (n) { + def->idmap.gidmap = virDomainIdmapDefParseXML(nodes, ctxt, n); + if (!def->idmap.gidmap) + goto error; + + def->idmap.ngidmap = n; + } + VIR_FREE(nodes); + + if ((def->idmap.uidmap && !def->idmap.gidmap) || + (!def->idmap.uidmap && def->idmap.gidmap)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("uid and gid should be mapped both")); + goto error; + } + /* analysis of cpu handling */ if ((node = virXPathNode("./cpu[1]", ctxt)) != NULL) { xmlNodePtr oldnode = ctxt->node; @@ -16008,6 +16082,27 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, " </os>\n"); + + if (def->idmap.uidmap) { + virBufferAddLit(buf, " <idmap>\n"); + for (i = 0 ; i < def->idmap.nuidmap; i++) { + virBufferAsprintf(buf, + " <uid start='%u' target='%u' count='%u'/>\n", + def->idmap.uidmap[i].start, + def->idmap.uidmap[i].target, + def->idmap.uidmap[i].count); + } + for (i = 0 ; i < def->idmap.ngidmap; i++) { + virBufferAsprintf(buf, + " <gid start='%u' target='%u' count='%u'/>\n", + def->idmap.gidmap[i].start, + def->idmap.gidmap[i].target, + def->idmap.gidmap[i].count); + } + virBufferAddLit(buf, " </idmap>\n"); + } + + if (def->features) { virBufferAddLit(buf, " <features>\n"); for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3a71d6c..29a6d8a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -120,6 +120,12 @@ typedef virDomainSnapshotObjList *virDomainSnapshotObjListPtr; typedef struct _virDomainRNGDef virDomainRNGDef; typedef virDomainRNGDef *virDomainRNGDefPtr; +typedef struct _virDomainIdMapEntry virDomainIdMapEntry; +typedef virDomainIdMapEntry *virDomainIdMapEntryPtr; + +typedef struct _virDomainIdMapDef virDomainIdMapDef; +typedef virDomainIdMapDef *virDomainIdMapDefPtr; + /* Flags for the 'type' field in virDomainDeviceDef */ typedef enum { VIR_DOMAIN_DEVICE_NONE = 0, @@ -1837,6 +1843,21 @@ struct _virDomainRNGDef { virDomainDeviceInfo info; }; +struct _virDomainIdMapEntry { + unsigned int start; + unsigned int target; + unsigned int count; +}; + +struct _virDomainIdMapDef { + size_t nuidmap; + virDomainIdMapEntryPtr uidmap; + + size_t ngidmap; + virDomainIdMapEntryPtr gidmap; +}; + + void virBlkioDeviceWeightArrayClear(virBlkioDeviceWeightPtr deviceWeights, int ndevices); @@ -1899,6 +1920,7 @@ struct _virDomainDef { virNumaTuneDef numatune; virDomainResourceDefPtr resource; + virDomainIdMapDef idmap; /* These 3 are based on virDomainLifeCycleAction enum flags */ int onReboot; -- 1.8.1.4

User namespace will be enabled only when the idmap exist in configuration. If you want disable user namespace,just remove these elements from XML. If kernel doesn't support user namespace and idmap exist in configuration file, libvirt lxc will start failed and return "Kernel doesn't support user namespace" message. Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com&gt; --- src/lxc/lxc_container.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index c74e3ca..618252c 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2029,14 +2029,12 @@ cleanup: static int userns_supported(void) { -#if 1 - /* - * put off using userns until uid mapping is implemented - */ - return 0; -#else return lxcContainerAvailable(LXC_CONTAINER_FEATURE_USER) == 0; -#endif +} + +static int userns_required(virDomainDefPtr def) +{ + return def->idmap.uidmap && def->idmap.gidmap; } virArch lxcContainerGetAlt32bitArch(virArch arch) @@ -2116,9 +2114,15 @@ int lxcContainerStart(virDomainDefPtr def, cflags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC|SIGCHLD; - if (userns_supported()) { - VIR_DEBUG("Enable user namespaces"); - cflags |= CLONE_NEWUSER; + if (userns_required(def)) { + if (userns_supported()) { + VIR_DEBUG("Enable user namespace"); + cflags |= CLONE_NEWUSER; + } else { + virReportSystemError(VIR_ERR_NO_KERNEL, "%s", + _("Kernel doesn't support user namespace")); + return -1; + } } if (lxcNeedNetworkNamespace(def)) { -- 1.8.1.4

On 05/23/2013 12:06 AM, Gao feng wrote: Since this was pushed yesterday, my overnight Coverity run picked up a problem (resource leak because stack is not VIR_FREE()'d): 2118 /* allocate a stack for the container */ (1) Event alloc_arg: "virAllocN(void *, size_t, size_t)" allocates memory that is stored into "stack". [details] (2) Event cond_false: Condition "virAllocN(&stack, 1UL /* sizeof (*stack) */, stacksize) < 0", taking false branch Also see events: [var_assign][leaked_storage][leaked_storage] 2119 if (VIR_ALLOC_N(stack, stacksize) < 0) { 2120 virReportOOMError(); 2121 return -1; (3) Event if_end: End of if statement 2122 } (4) Event var_assign: Assigning: "stacktop" = "stack". Also see events: [alloc_arg][leaked_storage][leaked_storage] 2123 stacktop = stack + stacksize; 2124 2125 cflags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC|SIGCHLD; 2126 (5) Event cond_true: Condition "userns_required(def)", taking true branch 2127 if (userns_required(def)) { (6) Event cond_false: Condition "userns_supported()", taking false branch 2128 if (userns_supported()) { 2129 VIR_DEBUG("Enable user namespace"); 2130 cflags |= CLONE_NEWUSER; (7) Event else_branch: Reached else branch 2131 } else { 2132 virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", 2133 _("Kernel doesn't support user namespace")); (8) Event leaked_storage: Variable "stacktop" going out of scope leaks the storage it points to. (9) Event leaked_storage: Variable "stack" going out of scope leaks the storage it points to. Also see events: [alloc_arg][var_assign] 2134 return -1; John

We forgot to free the stack when Kernel doesn't support user namespace. Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com&gt; --- src/lxc/lxc_container.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 282c726..c8420db 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2131,6 +2131,7 @@ int lxcContainerStart(virDomainDefPtr def, } else { virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Kernel doesn't support user namespace")); + VIR_FREE(stack); return -1; } } -- 1.8.1.4

Make sure the mapping line contains the root user of container is the first element of idmap array. So we can get the real user id on host for the container easily. This patch also check the map information, User must map the root user of container to any user of host. Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com&gt; --- src/conf/domain_conf.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a3c5c84..e271f5a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10061,6 +10061,14 @@ cleanup: } +static int virDomainIdMapEntrySort(const void *a, const void *b) +{ + const virDomainIdMapEntryPtr entrya = (const virDomainIdMapEntryPtr) a; + const virDomainIdMapEntryPtr entryb = (const virDomainIdMapEntryPtr) b; + + return entrya->start > entryb->start; +} + /* Parse the XML definition for user namespace id map. * * idmap has the form of @@ -10088,6 +10096,18 @@ virDomainIdmapDefParseXML(const xmlNodePtr *node, virXPathUInt("string(./@target)", ctxt, &idmap[i].target); virXPathUInt("string(./@count)", ctxt, &idmap[i].count); } + + qsort(idmap, num, sizeof(idmap[0]), virDomainIdMapEntrySort); + + if (idmap[0].start != 0) { + /* Root user of container hasn't been mapped to any user of host, + * return error. */ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("You must map the root user of container")); + VIR_FREE(idmap); + idmap = NULL; + } + error: ctxt->node = save_ctxt; return idmap; -- 1.8.1.4

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"); - /* rename and enable interfaces */ if (lxcContainerRenameAndEnableInterfaces(!!(vmDef->features & (1 << VIR_DOMAIN_FEATURE_PRIVNET)), 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); + + 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; +} + +/** + * 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; + + /* User namespace is disabled for container */ + if (ctrl->def->idmap.nuidmap == 0) + return 0; + + 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; +} + + /** * virLXCControllerMoveInterfaces @@ -1544,6 +1606,9 @@ virLXCControllerRun(virLXCControllerPtr ctrl) VIR_FORCE_CLOSE(control[1]); VIR_FORCE_CLOSE(containerhandshake[1]); + if (virLXCControllerSetupUserns(ctrl) < 0) + goto cleanup; + if (virLXCControllerMoveInterfaces(ctrl) < 0) goto cleanup; -- 1.8.1.4

On 06/04/2013 09:35 PM, Daniel P. Berrange wrote: 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. sure, I will wait for your related patches being merged and then update this patchset. Thanks Gao

user namespace doesn't allow to create devices in uninit userns. We should create devices on host side. We first mount tmpfs on dev directroy under state dir of container. then create devices under this dev dir. Finally in container, mount the dev directroy created on host to the /dev/ directroy of container. Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com&gt; --- src/lxc/lxc_container.c | 96 +++++++++++++--------------------- src/lxc/lxc_controller.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+), 60 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 52fcf39..c647f8e 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -683,7 +683,7 @@ err: } -static int lxcContainerMountBasicFS(char *sec_mount_options) +static int lxcContainerMountBasicFS(void) { const struct { const char *src; @@ -711,8 +711,6 @@ static int lxcContainerMountBasicFS(char *sec_mount_options) int i, rc = -1; char *opts = NULL; - VIR_DEBUG("Mounting basic filesystems sec_mount_options=%s", sec_mount_options); - for (i = 0; i < ARRAY_CARDINALITY(mnts); i++) { const char *srcpath = NULL; @@ -750,26 +748,6 @@ static int lxcContainerMountBasicFS(char *sec_mount_options) } } - /* - * tmpfs is limited to 64kb, since we only have device nodes in there - * and don't want to DOS the entire OS RAM usage - */ - - if (virAsprintf(&opts, - "mode=755,size=65536%s", sec_mount_options) < 0) { - virReportOOMError(); - goto cleanup; - } - - VIR_DEBUG("Mount devfs on /dev type=tmpfs flags=%x, opts=%s", - MS_NOSUID, opts); - if (mount("devfs", "/dev", "tmpfs", MS_NOSUID, opts) < 0) { - virReportSystemError(errno, - _("Failed to mount %s on %s type %s (%s)"), - "devfs", "/dev", "tmpfs", opts); - goto cleanup; - } - rc = 0; cleanup: @@ -811,6 +789,33 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def ATTRIBUTE_UNUSED, } #endif +static int lxcContainerMountFSDev(virDomainDefPtr def, + const char *stateDir) +{ + int ret; + char *path = NULL; + + VIR_DEBUG("Mount /dev/ stateDir=%s", stateDir); + + if ((ret = virAsprintf(&path, + "/.oldroot/%s/%s.dev", + stateDir, + def->name)) < 0) + return ret; + + VIR_DEBUG("Trying to move %s to /dev", path); + + if ((ret = mount(path, "/dev", + NULL, MS_MOVE, NULL)) < 0) { + virReportSystemError(errno, + _("Failed to mount %s on /dev"), + path); + } + + VIR_FREE(path); + return ret; +} + static int lxcContainerMountFSDevPTS(virDomainDefPtr def, const char *stateDir) { @@ -847,22 +852,10 @@ cleanup: return ret; } -static int lxcContainerPopulateDevices(char **ttyPaths, size_t nttyPaths) +static int lxcContainerSetupDevices(char **ttyPaths, size_t nttyPaths) { size_t i; const struct { - int maj; - int min; - mode_t mode; - const char *path; - } devs[] = { - { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_NULL, 0666, "/dev/null" }, - { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_ZERO, 0666, "/dev/zero" }, - { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_FULL, 0666, "/dev/full" }, - { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_RANDOM, 0666, "/dev/random" }, - { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_URANDOM, 0666, "/dev/urandom" }, - }; - const struct { const char *src; const char *dst; } links[] = { @@ -872,18 +865,6 @@ static int lxcContainerPopulateDevices(char **ttyPaths, size_t nttyPaths) { "/proc/self/fd", "/dev/fd" }, }; - /* Populate /dev/ with a few important bits */ - for (i = 0; i < ARRAY_CARDINALITY(devs); i++) { - dev_t dev = makedev(devs[i].maj, devs[i].min); - if (mknod(devs[i].path, S_IFCHR, dev) < 0 || - chmod(devs[i].path, devs[i].mode)) { - virReportSystemError(errno, - _("Failed to make device %s"), - devs[i].path); - return -1; - } - } - for (i = 0; i < ARRAY_CARDINALITY(links); i++) { if (symlink(links[i].src, links[i].dst) < 0) { virReportSystemError(errno, @@ -903,15 +884,6 @@ static int lxcContainerPopulateDevices(char **ttyPaths, size_t nttyPaths) _("Failed to bind /dev/pts/ptmx on to /dev/ptmx")); return -1; } - } else { - /* Legacy devpts, so we need to just use shared one */ - dev_t dev = makedev(LXC_DEV_MAJ_TTY, LXC_DEV_MIN_PTMX); - if (mknod("/dev/ptmx", S_IFCHR, dev) < 0 || - chmod("/dev/ptmx", 0666)) { - virReportSystemError(errno, "%s", - _("Failed to make device /dev/ptmx")); - return -1; - } } for (i = 0; i < nttyPaths; i++) { @@ -1813,7 +1785,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, goto cleanup; /* Mounts the core /proc, /sys, etc filesystems */ - if (lxcContainerMountBasicFS(sec_mount_options) < 0) + if (lxcContainerMountBasicFS() < 0) goto cleanup; /* Mounts /proc/meminfo etc sysinfo */ @@ -1825,12 +1797,16 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, if (virCgroupIsolateMount(cgroup, "/.oldroot/", sec_mount_options) < 0) goto cleanup; + /* Mounts /dev */ + if (lxcContainerMountFSDev(vmDef, stateDir) < 0) + goto cleanup; + /* Mounts /dev/pts */ if (lxcContainerMountFSDevPTS(vmDef, stateDir) < 0) goto cleanup; - /* Populates device nodes in /dev/ */ - if (lxcContainerPopulateDevices(ttyPaths, nttyPaths) < 0) + /* Setup device nodes in /dev/ */ + if (lxcContainerSetupDevices(ttyPaths, nttyPaths) < 0) goto cleanup; /* Sets up any non-root mounts from guest config */ diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 0a2e3ac..e9808f3 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1184,6 +1184,132 @@ cleanup: } +static int +virLXCControllerSetupDev(virLXCControllerPtr ctrl) +{ + char *mount_options = NULL; + char *opts = NULL; + char *dev = NULL; + int ret = -1; + + VIR_DEBUG("Setting up /dev/ for container"); + + mount_options = virSecurityManagerGetMountOptions(ctrl->securityManager, + ctrl->def); + + if (virAsprintf(&dev, "/%s/%s.dev", + LXC_STATE_DIR, ctrl->def->name) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (virFileMakePath(dev) < 0) { + virReportSystemError(errno, + _("Failed to make path %s"), + dev); + goto cleanup; + } + + /* + * tmpfs is limited to 64kb, since we only have device nodes in there + * and don't want to DOS the entire OS RAM usage + */ + + if (virAsprintf(&opts, + "mode=755,size=65536%s", mount_options) < 0) { + virReportOOMError(); + goto cleanup; + } + + + VIR_DEBUG("Mount devfs on %s type=tmpfs flags=%x, opts=%s", + dev, MS_NOSUID, opts); + if (mount("devfs", dev, "tmpfs", MS_NOSUID, opts) < 0) { + virReportSystemError(errno, + _("Failed to mount devfs (%s) on %s"), + opts, dev); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(opts); + VIR_FREE(mount_options); + VIR_FREE(dev); + return ret; +} + + +static int virLXCControllerPopulateDevices(virLXCControllerPtr ctrl) +{ + size_t i; + int ret = -1; + char *ptmx = NULL; + char *path = NULL; + const struct { + int maj; + int min; + mode_t mode; + const char *path; + } devs[] = { + { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_NULL, 0666, "/null" }, + { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_ZERO, 0666, "/zero" }, + { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_FULL, 0666, "/full" }, + { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_RANDOM, 0666, "/random" }, + { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_URANDOM, 0666, "/urandom" }, + }; + + if (virLXCControllerSetupDev(ctrl) < 0) + goto out; + + /* Populate /dev/ with a few important bits */ + for (i = 0 ; i < ARRAY_CARDINALITY(devs) ; i++) { + if (virAsprintf(&path, "/%s/%s.dev/%s", + LXC_STATE_DIR, ctrl->def->name, + devs[i].path) < 0) { + virReportOOMError(); + goto out; + } + + dev_t dev = makedev(devs[i].maj, devs[i].min); + if (mknod(path, S_IFCHR, dev) < 0 || + chmod(path, devs[i].mode)) { + virReportSystemError(errno, + _("Failed to make device %s"), + devs[i].path); + goto out; + } + VIR_FREE(path); + } + + if (virAsprintf(&ptmx, "/%s/%s.devpts/ptmx", + LXC_STATE_DIR, ctrl->def->name) < 0) { + virReportOOMError(); + goto out; + } + + if (access(ptmx, W_OK)) { + if (virAsprintf(&path, "/%s/%s.dev/ptmx", + LXC_STATE_DIR, ctrl->def->name)) { + virReportOOMError(); + goto out; + } + /* Legacy devpts, so we need to just use shared one */ + dev_t dev = makedev(LXC_DEV_MAJ_TTY, LXC_DEV_MIN_PTMX); + if (mknod(path, S_IFCHR, dev) < 0 || + chmod(path, 0666)) { + virReportSystemError(errno, _("Failed to make device %s"), path); + goto out; + } + } + + ret = 0; +out: + VIR_FREE(ptmx); + VIR_FREE(path); + return ret; +} /** * virLXCControllerMoveInterfaces @@ -1585,6 +1711,9 @@ virLXCControllerRun(virLXCControllerPtr ctrl) if (virLXCControllerSetupDevPTS(ctrl) < 0) goto cleanup; + if (virLXCControllerPopulateDevices(ctrl) < 0) + goto cleanup; + if (virLXCControllerSetupFuse(ctrl) < 0) goto cleanup; @@ -1609,6 +1738,7 @@ virLXCControllerRun(virLXCControllerPtr ctrl) if (virLXCControllerSetupUserns(ctrl) < 0) goto cleanup; + if (virLXCControllerMoveInterfaces(ctrl) < 0) goto cleanup; -- 1.8.1.4

Make codes clearer and reduce some virAsprintf. Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com&gt; --- src/lxc/lxc_controller.c | 44 +++++++++++++++++++------------------------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index e9808f3..7d10660 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1245,7 +1245,6 @@ static int virLXCControllerPopulateDevices(virLXCControllerPtr ctrl) { size_t i; int ret = -1; - char *ptmx = NULL; char *path = NULL; const struct { int maj; @@ -1283,30 +1282,8 @@ static int virLXCControllerPopulateDevices(virLXCControllerPtr ctrl) VIR_FREE(path); } - if (virAsprintf(&ptmx, "/%s/%s.devpts/ptmx", - LXC_STATE_DIR, ctrl->def->name) < 0) { - virReportOOMError(); - goto out; - } - - if (access(ptmx, W_OK)) { - if (virAsprintf(&path, "/%s/%s.dev/ptmx", - LXC_STATE_DIR, ctrl->def->name)) { - virReportOOMError(); - goto out; - } - /* Legacy devpts, so we need to just use shared one */ - dev_t dev = makedev(LXC_DEV_MAJ_TTY, LXC_DEV_MIN_PTMX); - if (mknod(path, S_IFCHR, dev) < 0 || - chmod(path, 0666)) { - virReportSystemError(errno, _("Failed to make device %s"), path); - goto out; - } - } - ret = 0; out: - VIR_FREE(ptmx); VIR_FREE(path); return ret; } @@ -1492,6 +1469,7 @@ virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl) char *mount_options = NULL; char *opts = NULL; char *devpts = NULL; + char *path = NULL; int ret = -1; VIR_DEBUG("Setting up private /dev/pts"); @@ -1537,9 +1515,25 @@ virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl) goto cleanup; } + if (access(ctrl->devptmx, W_OK) < 0) { + if (virAsprintf(&path, "/%s/%s.dev/ptmx", + LXC_STATE_DIR, ctrl->def->name)) { + virReportOOMError(); + goto cleanup; + } + /* Legacy devpts, so we need to just use shared one */ + dev_t dev = makedev(LXC_DEV_MAJ_TTY, LXC_DEV_MIN_PTMX); + if (mknod(path, S_IFCHR, dev) < 0 || + chmod(path, 0666)) { + virReportSystemError(errno, _("Failed to make device %s"), path); + goto cleanup; + } + } + ret = 0; cleanup: + VIR_FREE(path); VIR_FREE(opts); VIR_FREE(devpts); VIR_FREE(mount_options); @@ -1708,10 +1702,10 @@ virLXCControllerRun(virLXCControllerPtr ctrl) if (virLXCControllerSetupResourceLimits(ctrl, cgroup) < 0) goto cleanup; - if (virLXCControllerSetupDevPTS(ctrl) < 0) + if (virLXCControllerPopulateDevices(ctrl) < 0) goto cleanup; - if (virLXCControllerPopulateDevices(ctrl) < 0) + if (virLXCControllerSetupDevPTS(ctrl) < 0) goto cleanup; if (virLXCControllerSetupFuse(ctrl) < 0) -- 1.8.1.4

Otherwise we will fail to mount the meminfo file. This patch also allows any users to access the fuse mount point. Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com&gt; --- src/lxc/lxc_fuse.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index b6df99c..ddb737c 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -49,6 +49,9 @@ static int lxcProcGetattr(const char *path, struct stat *stbuf) char *mempath = NULL; struct stat sb; + struct fuse_context *context = fuse_get_context(); + virDomainDefPtr def = (virDomainDefPtr)context->private_data; + memset(stbuf, 0, sizeof(struct stat)); if (virAsprintf(&mempath, "/proc/%s", path) < 0) { virReportOOMError(); @@ -66,6 +69,8 @@ static int lxcProcGetattr(const char *path, struct stat *stbuf) goto cleanup; } + stbuf->st_uid = def->idmap.uidmap ? def->idmap.uidmap[0].target : 0; + stbuf->st_gid = def->idmap.gidmap ? def->idmap.gidmap[0].target : 0; stbuf->st_mode = sb.st_mode; stbuf->st_nlink = 1; stbuf->st_blksize = sb.st_blksize; @@ -307,6 +312,7 @@ int lxcSetupFuse(virLXCFusePtr *f, virDomainDefPtr def) /* process name is libvirt_lxc */ if (fuse_opt_add_arg(&args, "libvirt_lxc") == -1 || fuse_opt_add_arg(&args, "-odirect_io") == -1 || + fuse_opt_add_arg(&args, "-oallow_other") == -1 || fuse_opt_add_arg(&args, "-ofsname=libvirt") == -1) goto cleanup1; -- 1.8.1.4

On Thu, May 23, 2013 at 12:06:51PM +0800, Gao feng wrote: We actually need the following fix even without userns work: So I've sent a patch to add that separately. 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 :|

Hi! ----- Ursprüngliche Mail ----- Why do you free ttyHostPath here? You already do it in the exit path. Double free? Thanks, //richard

----- Ursprüngliche Mail ----- Ahhh, there is some hidden magic. Now it makes sense. :D Thanks, //richard

container will create /dev/pts directory in /dev. the owner of /dev should be the root user of container. Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com&gt; --- src/lxc/lxc_controller.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 4660f25..f892ce3 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1191,6 +1191,13 @@ virLXCControllerSetupDev(virLXCControllerPtr ctrl) char *opts = NULL; char *dev = NULL; int ret = -1; + uid_t uid = (uid_t)-1; + gid_t gid = (gid_t)-1; + + if (ctrl->def->idmap.uidmap) { + uid = ctrl->def->idmap.uidmap[0].target; + gid = ctrl->def->idmap.gidmap[0].target; + } VIR_DEBUG("Setting up /dev/ for container"); @@ -1231,6 +1238,13 @@ virLXCControllerSetupDev(virLXCControllerPtr ctrl) goto cleanup; } + if (chown(dev, uid, gid) < 0) { + virReportSystemError(errno, + _("Failed to change owner of %s to %d:%d"), + dev, uid, gid); + goto cleanup; + } + ret = 0; cleanup: -- 1.8.1.4

Since these devices are created for the container. the owner should be the root user of the container. Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com&gt; --- src/lxc/lxc_controller.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index f892ce3..b2ace20 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1260,6 +1260,8 @@ static int virLXCControllerPopulateDevices(virLXCControllerPtr ctrl) size_t i; int ret = -1; char *path = NULL; + uid_t uid = (uid_t)-1; + gid_t gid = (gid_t)-1; const struct { int maj; int min; @@ -1276,6 +1278,11 @@ static int virLXCControllerPopulateDevices(virLXCControllerPtr ctrl) if (virLXCControllerSetupDev(ctrl) < 0) goto out; + if (ctrl->def->idmap.uidmap) { + uid = ctrl->def->idmap.uidmap[0].target; + gid = ctrl->def->idmap.gidmap[0].target; + } + /* Populate /dev/ with a few important bits */ for (i = 0 ; i < ARRAY_CARDINALITY(devs) ; i++) { if (virAsprintf(&path, "/%s/%s.dev/%s", @@ -1293,6 +1300,13 @@ static int virLXCControllerPopulateDevices(virLXCControllerPtr ctrl) devs[i].path); goto out; } + + if (chown(path, uid, gid) < 0) { + virReportSystemError(errno, + _("Failed to change owner of %s to %u:%u"), + devs[i].path, uid, gid); + goto out; + } VIR_FREE(path); } -- 1.8.1.4

On Thu, May 23, 2013 at 12:06:54PM +0800, Gao feng wrote: ACK 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 :|

Hi! ----- Ursprüngliche Mail ----- You're using this pattern a few times. Maybe it's worth a helper function. Thanks, //richard

On Thu, May 23, 2013 at 12:06:55PM +0800, Gao feng wrote: ACK 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 :|

On Thu, May 23, 2013 at 12:06:56PM +0800, Gao feng wrote: I suggest you move this patch to be before current #7. That way patches 7-11 can use this function right away, avoiding all the code below 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 :|