[Devel] [PATCH 2/2] hook_ct: bind mount the host /dev directory in CT

[Andrew Vagin]

On Mon, Jan 20, 2014 at 03:48:01PM +0400, Andrey Vagin wrote:
> All modern distributions require devtmpfs in /dev. devtmpfs can't
> be mounted from userns. This patch bind-mounts the host /dev.
> It's secure, because permissions are handled according with uid and
> gid maps for the user namespace.
> 
> Signed-off-by: Andrey Vagin <avagin at openvz.org>
> ---
>  src/lib/hooks_ct.c | 78 ++++++++++++++++++------------------------------------
>  1 file changed, 26 insertions(+), 52 deletions(-)
> 
> diff --git a/src/lib/hooks_ct.c b/src/lib/hooks_ct.c
> index 2a0b54c..23201c0 100644
> --- a/src/lib/hooks_ct.c
> +++ b/src/lib/hooks_ct.c
> @@ -10,6 +10,7 @@
>  #include <fcntl.h>
>  #include <sched.h>
>  #include <dirent.h>
> +#include <sys/vfs.h> 
>  
>  #include "vzerror.h"
>  #include "env.h"
> @@ -98,7 +99,21 @@ static int ct_destroy(vps_handler *h, envid_t veid)
>  	return destroy_container(veid);
>  }
>  
> -int ct_chroot(const char *root)
> +static int mount_devtmpfs(const char *oldroot)
> +{
> +	char oldpath[PATH_MAX];
> +
> +	snprintf(oldpath, sizeof(oldpath), "%s/dev", oldroot);
> +
> +	if (mount(oldpath, "/dev", NULL, MS_BIND, NULL)) {
> +		logger(-1, errno, "Failed to mount /dev");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +int ct_chroot(const char *root, bool userns)
>  {
>  	char oldroot[] = "vzctl-old-root.XXXXXX";
>  	int ret = VZ_RESOURCE_ERROR;
> @@ -153,6 +168,14 @@ int ct_chroot(const char *root)
>  		goto rmdir;
>  	}
>  
> +	/*
> +	 * devtmpfs can't be mounted from userns. The host /dev is mounted
> +	 * in CT. All permissions are handled according with uid and gid maps
> +	 * for the user namespace.
> +	 */
> +	if (userns && mount_devtmpfs(oldroot))
> +		goto rmdir;

/dev must be bind-mounted with MS_REC, because here is a few locked
children. I will resend this patch.

And probably it's the wrong place for mounting devtmpfs...

> +
>  	if (umount2(oldroot, MNT_DETACH)) {
>  		logger(-1, 0, "Can't umount old mounts");
>  		goto rmdir;
> @@ -269,51 +292,6 @@ out:
>  	return ret;
>  }
>  
> -/*
> - * Those devices should exist in the container, and be valid device nodes with
> - * user access permission. But we need to be absolutely sure this is the case,
> - * so we will provide our own versions. That could actually happen since some
> - * distributions may come with emptied /dev's, waiting for udev to populate them.
> - * That won't happen, we do it ourselves.
> - */
> -static void create_devices(vps_handler *h, envid_t veid, const char *root)
> -{
> -	unsigned int i;
> -	char *devices[] = {
> -		"/dev/null",
> -		"/dev/zero",
> -		"/dev/random",
> -		"/dev/urandom",
> -	};
> -
> -	/*
> -	 * We will tolerate errors, and keep the container running, because it is
> -	 * likely we will be able to boot it to a barely functional state. But
> -	 * be vocal about it
> -	 */
> -	for (i = 0; i < ARRAY_SIZE(devices); i++) {
> -		char ct_devname[STR_SIZE];
> -		int ret;
> -
> -		snprintf(ct_devname, sizeof(ct_devname), "%s%s", root, devices[i]);
> -
> -		/*
> -		 * No need to be crazy about file flags. When we bind mount, the
> -		 * source permissions will be inherited.
> -		 */
> -		ret = open(ct_devname, O_RDWR|O_CREAT, 0);
> -		if (ret < 0) {
> -			logger(-1, errno, "Could not touch device %s", devices[i]);
> -			continue;
> -		}
> -		close(ret);
> -
> -		ret = mount(devices[i], ct_devname, "", MS_BIND, 0);
> -		if (ret < 0)
> -			logger(-1, errno, "Could not bind mount device %s", devices[i]);
> -	}
> -}
> -
>  static int _env_create(void *data)
>  {
>  	struct arg_start *arg = data;
> @@ -338,11 +316,7 @@ static int _env_create(void *data)
>  	if (arg->userns_p != -1)
>  		close(arg->userns_p);
>  
> -	if (arg->h->can_join_userns) {
> -		create_devices(arg->h, arg->veid, arg->res->fs.root);
> -	}
> -
> -	ret = ct_chroot(arg->res->fs.root);
> +	ret = ct_chroot(arg->res->fs.root, arg->h->can_join_userns);
>  	/* Probably means chroot failed */
>  	if (ret)
>  		return ret;
> @@ -605,7 +579,7 @@ static int ct_enter(vps_handler *h, envid_t veid, const char *root, int flags)
>  	 * As a matter of fact, we won't even be able to see the container
>  	 * directories to jump to
>  	 */
> -	if (!joined_mnt_ns && (ret = ct_chroot(root)))
> +	if (!joined_mnt_ns && (ret = ct_chroot(root, false)))
>  		goto out;
>  
>  	/*
> -- 
> 1.8.3.1
>