[systemd-devel] [PATCH] [PATCH v3] nspawn: fallback on bind mount when mknod fails

[Dave Reisner]

On Tue, Mar 31, 2015 at 05:14:48PM +0200, Alban Crequy wrote:
> From: Alban Crequy <alban at endocode.com>
> 
> Some systems abusively restrict mknod, even when the device node already
> exists in /dev. This is unfortunate because it prevents systemd-nspawn
> from creating the basic devices in /dev in the container.
> 
> This patch implements a workaround: when mknod fails, fallback on bind
> mounts.
> 
> Additionally, /dev/console was created with a mknod with the same
> major/minor as /dev/null before bind mounting a pts on it. This patch
> removes the mknod and creates an empty regular file instead.
> 
> In order to test this patch, I used the following configuration, which I
> think should replicate the system with the abusive restriction on mknod:
> 
>   # grep devices /proc/self/cgroup
>   4:devices:/user.slice/restrict
>   # cat /sys/fs/cgroup/devices/user.slice/restrict/devices.list
>   c 1:9 r
>   c 5:2 rw
>   c 136:* rw
>   # systemd-nspawn --register=false -D .
> 
> v2:
>  - remove "bind", it is not needed since there is already MS_BIND
> v3:
>  - fix error management when calling touch()
>  - fix lowercase in error message
> ---
>  src/nspawn/nspawn.c | 29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
> index b830141..7e56cf2 100644
> --- a/src/nspawn/nspawn.c
> +++ b/src/nspawn/nspawn.c
> @@ -1449,8 +1449,18 @@ static int copy_devnodes(const char *dest) {
>                                  return -r;
>                          }
>  
> -                        if (mknod(to, st.st_mode, st.st_rdev) < 0)
> -                                return log_error_errno(errno, "mknod(%s) failed: %m", to);
> +                        if (mknod(to, st.st_mode, st.st_rdev) < 0) {
> +                                if (errno != EPERM)
> +                                        return log_error_errno(errno, "mknod(%s) failed: %m", to);
> +
> +                                /* Some systems abusively restrict mknod but
> +                                 * allow bind mounts. */
> +                                r = touch(to);
> +                                if (r < 0)
> +                                        return log_error_errno(r, "touch (%s) failed: %m", to);

Is this really what you wanted? It's not obvious that errno will have
the correct value when %m is evaluated. I would have used strerror(-r)
here.

> +                                if (mount(from, to, NULL, MS_BIND, NULL) < 0)
> +                                        return log_error_errno(errno, "Both mknod and bind mount (%s) failed: %m", to);
> +                        }
>  
>                          if (arg_userns && arg_uid_shift != UID_INVALID)
>                                  if (lchown(to, arg_uid_shift, arg_uid_shift) < 0)
> @@ -1481,7 +1491,6 @@ static int setup_ptmx(const char *dest) {
>  static int setup_dev_console(const char *dest, const char *console) {
>          _cleanup_umask_ mode_t u;
>          const char *to;
> -        struct stat st;
>          int r;
>  
>          assert(dest);
> @@ -1489,24 +1498,18 @@ static int setup_dev_console(const char *dest, const char *console) {
>  
>          u = umask(0000);
>  
> -        if (stat("/dev/null", &st) < 0)
> -                return log_error_errno(errno, "Failed to stat /dev/null: %m");
> -
>          r = chmod_and_chown(console, 0600, 0, 0);
>          if (r < 0)
>                  return log_error_errno(r, "Failed to correct access mode for TTY: %m");
>  
>          /* We need to bind mount the right tty to /dev/console since
>           * ptys can only exist on pts file systems. To have something
> -         * to bind mount things on we create a device node first, and
> -         * use /dev/null for that since we the cgroups device policy
> -         * allows us to create that freely, while we cannot create
> -         * /dev/console. (Note that the major minor doesn't actually
> -         * matter here, since we mount it over anyway). */
> +         * to bind mount things on we create a empty regular file. */
>  
>          to = strjoina(dest, "/dev/console");
> -        if (mknod(to, (st.st_mode & ~07777) | 0600, st.st_rdev) < 0)
> -                return log_error_errno(errno, "mknod() for /dev/console failed: %m");
> +        r = touch(to);
> +        if (r < 0)
> +                return log_error_errno(r, "touch() for /dev/console failed: %m");
>  
>          if (mount(console, to, NULL, MS_BIND, NULL) < 0)
>                  return log_error_errno(errno, "Bind mount for /dev/console failed: %m");
> -- 
> 2.1.4
> 
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel