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

[Alban Crequy]

On Sun, Mar 29, 2015 at 5:24 PM, Tom Gundersen <teg at jklm.no> wrote:
>
> On Mar 29, 2015 5:18 PM, "Alban Crequy" <alban.crequy at gmail.com> 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.
>
> Could we just always use bind mounts and avoid the two code paths?

It's possible but I avoided it in order not to add to many entries in
/proc/self/mounts and /proc/self/mountinfo in the normal case when
mknod is not restricted.

I don't have a strong opinion about this. If you think my concern
about the mount entries is less important than avoiding two code
paths, I can prepare another patch.

Alban

> Tom
>
>> 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 .
>> ---
>>  src/nspawn/nspawn.c | 27 ++++++++++++++-------------
>>  1 file changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
>> index 300b6df..09fff38 100644
>> --- a/src/nspawn/nspawn.c
>> +++ b/src/nspawn/nspawn.c
>> @@ -1449,8 +1449,17 @@ 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. */
>> +                                if (touch(to) < 0)
>> +                                        return log_error_errno(errno,
>> "touch (%s) failed: %m", to);
>> +                                if (mount(from, to, "bind", 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 +1490,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 +1497,17 @@ 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");
>> +        if (touch(to) < 0)
>> +                return log_error_errno(errno, "touch() for /dev/console
>> failed: %m");
>>
>>          if (mount(console, to, "bind", 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
>
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>