On Thu, Feb 13, 2014 at 05:15:22PM +0000, Daniel P. Berrange wrote:
> From: Richard Weinberger <richard@xxxxxx>
>
> Add a new helper function to change the permissions of a control
> group. This function is needed for user namespaces, we need to
> chmod() the cgroup to the initial uid/gid such that systemd is
> allowed to use the cgroup.
>
> Only the systemd controller is made accessible to the container.
> Others must remain read-only since it is generally not safe
> to delegate resource controller write access to unprivileged
> processes.
>
> Signed-off-by: Richard Weinberger <richard@xxxxxx>
> ---
> src/libvirt_private.syms | 1 +
> src/lxc/lxc_cgroup.c | 9 ++++++++
> src/util/vircgroup.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/util/vircgroup.h | 5 +++++
> 4 files changed, 69 insertions(+)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 0b28bac..cfa9f75 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1056,6 +1056,7 @@ virCgroupSetMemory;
> virCgroupSetMemoryHardLimit;
> virCgroupSetMemorySoftLimit;
> virCgroupSetMemSwapHardLimit;
> +virCgroupSetOwner;
> virCgroupSupportsCpuBW;
>
>
> diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
> index cc0d5e8..0d0d9c0 100644
> --- a/src/lxc/lxc_cgroup.c
> +++ b/src/lxc/lxc_cgroup.c
> @@ -484,6 +484,15 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def)
> &cgroup) < 0)
> goto cleanup;
>
> + /* setup control group permissions for user namespace */
> + if (def->idmap.uidmap) {
> + if (virCgroupSetOwner(cgroup,
> + def->idmap.uidmap[0].target,
> + def->idmap.gidmap[0].target,
> + (1 << VIR_CGROUP_CONTROLLER_SYSTEMD)))
This should be "if (virCgroupSetOwner() < 0)" to go with the rest.
> + goto cleanup;
> + }
> +
virCgroupNewMachine() guarantees that the cgroup is NULL in case of an
error, but you don't guarantee that in virCgroupSetOwner(), so the
errors from it won't propagate anywhere, because you don't return NULL
from this function.
> cleanup:
> return cgroup;
> }
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index a6d60c5..2dc6986 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -3253,6 +3253,60 @@ cleanup:
> }
>
>
> +int virCgroupSetOwner(virCgroupPtr cgroup,
> + uid_t uid,
> + gid_t gid,
> + int controllers)
> +{
> + size_t i;
> +
> + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
> + char *base, *entry;
> + DIR *dh;
> + struct dirent *de;
> +
> + if (!((1 << i) & controllers))
> + continue;
> +
> + if (!cgroup->controllers[i].mountPoint)
> + continue;
> +
> + if (virAsprintf(&base, "%s%s", cgroup->controllers[i].mountPoint,
> + cgroup->controllers[i].placement) < 0) {
> + virReportOOMError();
Double OOM reporting.
> + return -1;
> + }
> +
> + dh = opendir(base);
> + while ((de = readdir(dh)) != NULL) {
> + if (STREQ(de->d_name, ".") ||
> + STREQ(de->d_name, ".."))
> + continue;
> +
> + if (virAsprintf(&entry, "%s/%s", base, de->d_name) < 0) {
> + VIR_FREE(base);
> + virReportOOMError();
Same here, plus you continue the loop and don't return -1.
> + }
> +
> + if (chown(entry, uid, gid) < 0)
> + virReportSystemError(errno, _("cannot chown '%s' to (%u, %u)"),
> + entry, uid, gid);
Indentation's off and you continue the loop again.
> +
> + VIR_FREE(entry);
> + }
> + closedir(dh);
> +
> + if (chown(base, uid, gid) < 0)
> + virReportSystemError(errno, _("cannot chown '%s' to (%u, %u)"),
> + base, uid, gid);
Again reporting an error, but returning 0 even in case of an error.
> +
> + VIR_FREE(base);
> + }
> +
> + return 0;
> +}
> +
> +
> /**
> * virCgroupSupportsCpuBW():
> * Check whether the host supports CFS bandwidth.
> diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
> index a70eb18..38d94f3 100644
> --- a/src/util/vircgroup.h
> +++ b/src/util/vircgroup.h
> @@ -225,4 +225,9 @@ int virCgroupIsolateMount(virCgroupPtr group,
>
> bool virCgroupSupportsCpuBW(virCgroupPtr cgroup);
>
> +int virCgroupSetOwner(virCgroupPtr cgroup,
> + uid_t uid,
> + gid_t gid,
> + int controllers);
> +
> #endif /* __VIR_CGROUP_H__ */
> --
> 1.8.5.3
>
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list