On Tue, Mar 31, 2015 at 5:35 PM, Dave Reisner <d...@falconindy.com> wrote:
> On Tue, Mar 31, 2015 at 05:14:48PM +0200, Alban Crequy wrote:
>> From: Alban Crequy <al...@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.
log_error_errno() is a macro that ends up calling log_internalv() and
it will assign errno to the parameter r in order to get %m working:
src/shared/log.c: log_internalv():
/* Make sure that %m maps to the specified error */
if (error != 0)
errno = error;
>
>> + 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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> _______________________________________________
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
