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

Re: [libvirt] [RFC PATCH 4/6] LXC: Creating devices for container on host side



On Mon, Mar 11, 2013 at 02:26:50PM +0800, Gao feng wrote:
> user namespace doesn't allow to create devices in
> uninit userns. We should create devices on host side.
> 
> Signed-off-by: Gao feng <gaofeng cn fujitsu com>
> ---
>  src/lxc/lxc_container.c  | 47 +++++++--------------------
>  src/lxc/lxc_controller.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 94 insertions(+), 36 deletions(-)

We actually need this change independently of user namespaces. Currently
the cgroup devices setup we do allows 'mknod' permission, when it really
ought to be blocked. If we move the setup code into the controller then
we can fix the cgroup devices setup to block mknod too.

> diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> index 92af3e5..58d6ee5 100644
> --- a/src/lxc/lxc_container.c
> +++ b/src/lxc/lxc_container.c
> @@ -681,22 +681,10 @@ static int lxcContainerMountFSDevPTS(virDomainFSDefPtr root)
>      return rc;
>  }
>  
> -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[] = {
> @@ -706,18 +694,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,
> @@ -737,15 +713,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++) {
> @@ -1988,8 +1955,8 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef,
>      if (lxcContainerMountFSDevPTS(root) < 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 */
> @@ -2306,6 +2273,14 @@ static int lxcContainerChild(void *data)
>          goto cleanup;
>      }
>  
> +    /* Wait for controller creating devices for container */
> +    if (lxcContainerWaitForContinue(argv->monitor) < 0) {
> +        virReportSystemError(errno, "%s",
> +                             _("Failed to read the container continue message"));
> +        goto cleanup;
> +    }
> +    VIR_DEBUG("Received container continue message, create devices success.");
> +
>      ret = 0;
>  cleanup:
>      VIR_FREE(ttyPath);
> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> index f17142f..c6f8c3b 100644
> --- a/src/lxc/lxc_controller.c
> +++ b/src/lxc/lxc_controller.c
> @@ -1092,6 +1092,79 @@ cleanup:
>  }
>  
>  
> +static int virLXCControllerPopulateDevices(virLXCControllerPtr ctrl)
> +{
> +    size_t i;
> +    char *ptmx = NULL;
> +    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" },
> +    };
> +    virDomainFSDefPtr root = virDomainGetRootFilesystem(ctrl->def);
> +
> +    if (root == NULL || root->src == NULL)
> +        return 0;
> +
> +    /* Populate /dev/ with a few important bits */
> +    for (i = 0 ; i < ARRAY_CARDINALITY(devs) ; i++) {
> +        char *path = NULL;
> +        if (virAsprintf(&path, "/proc/%llu/root/%s",
> +                        (unsigned long long)ctrl->initpid,
> +                        devs[i].path) < 0) {
> +            virReportOOMError();
> +            return -1;
> +        }
> +        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"),
> +                                 path);
> +            VIR_FREE(path);
> +            return -1;
> +        }
> +        VIR_FREE(path);
> +    }
> +
> +    if (virAsprintf(&ptmx, "/proc/%llu/root/dev/pts/ptmx",
> +                    (unsigned long long)ctrl->initpid) < 0) {
> +        virReportOOMError();
> +        return -1;
> +    }
> +
> +    if (access(ptmx, W_OK)) {
> +        char *path = NULL;
> +        if (virAsprintf(&path, "/proc/%llu/root/dev/ptmx",
> +                        (unsigned long long)ctrl->initpid) < 0) {
> +            virReportOOMError();
> +            VIR_FREE(ptmx);
> +            return -1;
> +        }
> +        /* 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);
> +            VIR_FREE(path);
> +            VIR_FREE(ptmx);
> +            return -1;
> +        }
> +        VIR_FREE(path);
> +    }
> +    VIR_FREE(ptmx);
> +    return 0;
> +}
> +
> +
>  /**
>   * virLXCControllerMoveInterfaces
>   * @nveths: number of interfaces
> @@ -1535,6 +1608,16 @@ virLXCControllerRun(virLXCControllerPtr ctrl)
>          goto cleanup;
>      }
>  
> +    /* Populate devices for container */
> +    if (virLXCControllerPopulateDevices(ctrl) < 0)
> +        goto cleanup;

We ought to be able to invoke this before lxcContainerStart, so
that we can avoid the send/wait continue bit


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]