[Devel] [PATCH rh7] cgroups: Drop virtualization code, v4

Cyrill Gorcunov gorcunov at odin.com
Thu May 7 02:12:37 PDT 2015


On Thu, May 07, 2015 at 11:59:54AM +0300, Vladimir Davydov wrote:
> > > 
> > > So we are not allowed to create new cgroup hierarchies from a container,
> > > but we still can mount existing ones? If so, I think we should forbid
> > > this either.
> > 
> > Yes existing ones are not forbidden. I'm not sure though if we should
> > not allow to reuse existing hierarchies. You have some secutiry scenario
> > in your mind or mean to make everything as strict as possible until
> > the reverse is not explicitly needed?
> 
> If a container is able to mount real cgroup root, it can see how many
> containers are running on the node and their parameters. Looks like a
> sort of security issue to me.

Yes, except one need to properly guess the mount options of cgroups.
Still true, this is security problem.

I think something like

cgroup_mount
 ...
#ifdef CONFIG_VE
	/*
	 * Cgroups mounting from inside of VE is not allowed
	 * until we get some iron prove that we are to.
	 */
	if (!(flags & MS_KERNMOUNT) && !ve_is_super(get_exec_env())) {
		ret = -EACCES;
		goto out_err;
	}
#endif

right at the begginning of the cgroup_mount should help us. Looks good?

> > > Another question that keeps bothering me is about permissions check. The
> > > admin of a container should be allowed to create and tune memory cgroups
> > > under its bind mounted cgroup root, but he must not be able to modify
> > > parameters of the bind mounted root itself, because that would affect
> > > the container's configuration. How are we going to prevent this?
> > 
> > At moment we don't, but looks like we need to add some check if
> > cgroup been modified is not a top one when write happens from
> > inside of container maybe?
> 
> I guess so.
> 
> Besides, I think we should not bind mount all cgroups inside any
> container, because allowing a container to create an arbitrary number of
> cgroups can affect the overall performance badly. IMO this should be
> configured in the config file of a container.

I see, thanks. Letme think of it.



More information about the Devel mailing list