[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/2] lxc: Add virCgroupSetOwner()



On Sat, Feb 08, 2014 at 06:37:43PM +0100, Richard Weinberger wrote:
> Add a new helper function to change the permissions
> of a control group.
> 
> Signed-off-by: Richard Weinberger <richard nod at>
> ---
>  src/lxc/lxc_controller.c |  7 +++++++
>  src/util/vircgroup.c     | 43 +++++++++++++++++++++++++++++++++++++++++++
>  src/util/vircgroup.h     |  2 ++
>  3 files changed, 52 insertions(+)
> 
> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> index f7b614b..6e348b3 100644
> --- a/src/lxc/lxc_controller.c
> +++ b/src/lxc/lxc_controller.c
> @@ -2223,6 +2223,13 @@ virLXCControllerRun(virLXCControllerPtr ctrl)
>          goto cleanup;
>      }
>  
> +    /* setup control group permissions for user namespace */
> +    if (ctrl->def->idmap.uidmap) {
> +        if (virCgroupSetOwner(ctrl->cgroup, ctrl->def->idmap.uidmap[0].target,
> +            ctrl->def->idmap.gidmap[0].target))
> +            goto cleanup;
> +    }
> +

I wonder is it possible to just do the ownership change in the
virLXCCgroupCreate method ?   When the container then does the
bind mount, it should preserve the ownership correctly. This
would avoid any need for the extra handshake synchronization.

> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index a6d60c5..b66ffed 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -3252,6 +3252,49 @@ cleanup:
>      return ret;
>  }
>  
> +int virCgroupSetOwner(virCgroupPtr cgroup, uid_t uid, gid_t gid) {
> +    size_t i;
> +
> +    for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {

I think we only want to set ownership for VIR_CGROUP_CONTROLLER_SYSTEMD.
We don't want to give the container permission to change resource
controllers, this that would allow it to potentially escape its resource
limits. You might think the hierarchy would lock things down, but I
believe the kernel developers still say it is unsafe to delegate cgroup
resource controllers to unprivileged users.

Regards,
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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]