Merge pull request #16566 from poettering/nspawn-osrelease-fixesv246-rc2

nspawn: os-release reorganization, second try

8 files changed, 75 insertions, 42 deletions

diff --git a/NEWS b/NEWS
index 108013f0d2..4c5b63f76e 100644
--- a/NEWS
+++ b/NEWS
@@ -546,7 +546,7 @@ CHANGES WITH 246:
           has been extended by a set of environment variables that expose
           select fields from the host's os-release file to the container
           payload. Similarly, host's os-release files can be mounted into the
-          container underneath /run/hosts. Together, those mechanisms provide a
+          container underneath /run/host. Together, those mechanisms provide a
           standardized way to expose information about the host to the
           container payload. Both interfaces are implemented in systemd-nspawn.
 
diff --git a/TODO b/TODO
index 7d4d54aa7a..58fdc45e1e 100644
--- a/TODO
+++ b/TODO
@@ -17,6 +17,10 @@ Janitorial Clean-ups:
 
 Features:
 
+* nspawn: move "incoming mount" directory to /run/host, move "inaccessible"
+  nodes to /run/host, move notify socket (for sd_notify() between payload and
+  container manager)
+
 * cryptsetup: if keyfile specified in crypttab is AF_UNIX socket, connect to it
   and read from it (like we do elsewhere with READ_FULL_FILE_CONNECT_SOCKET)
 
diff --git a/man/os-release.xml b/man/os-release.xml
index 675daf3ede..a2164436c3 100644
--- a/man/os-release.xml
+++ b/man/os-release.xml
@@ -342,10 +342,9 @@
 
     <para>Container and sandbox runtime managers may make the host's
     identification data available to applications by providing the host's
-    <filename>/etc/os-release</filename> and
-    <filename>/usr/lib/os-release</filename> as respectively
-    <filename>/run/host/etc/os-release</filename> and
-    <filename>/run/host/usr/lib/os-release</filename>.</para>
+    <filename>/etc/os-release</filename> (if available, otherwise
+    <filename>/usr/lib/os-release</filename> as a fallback) as
+    <filename>/run/host/os-release</filename>.</para>
   </refsect1>
 
   <refsect1>
diff --git a/src/nspawn/nspawn-def.h b/src/nspawn/nspawn-def.h
index 9b54cda2f7..ac3a1a02c4 100644
--- a/src/nspawn/nspawn-def.h
+++ b/src/nspawn/nspawn-def.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: LGPL-2.1+ */
 #pragma once
 
 #include <sys/types.h>
diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c
index 4687ac4c18..a48c7f62da 100644
--- a/src/nspawn/nspawn-mount.c
+++ b/src/nspawn/nspawn-mount.c
@@ -563,15 +563,16 @@ int mount_all(const char *dest,
                   MOUNT_FATAL|MOUNT_MKDIR },
                 { "tmpfs",                  "/run",                         "tmpfs", "mode=755" TMPFS_LIMITS_RUN,      MS_NOSUID|MS_NODEV|MS_STRICTATIME,
                   MOUNT_FATAL|MOUNT_MKDIR },
-                { "/usr/lib/os-release",    "/run/host/usr/lib/os-release", NULL,    NULL,                             MS_BIND,
-                  MOUNT_FATAL|MOUNT_MKDIR|MOUNT_TOUCH }, /* As per kernel interface requirements, bind mount first (creating mount points) and make read-only later */
-                { NULL,                     "/run/host/usr/lib/os-release", NULL,    NULL,                             MS_BIND|MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_REMOUNT,
-                  0 },
-                { "/etc/os-release",        "/run/host/etc/os-release",     NULL,    NULL,                             MS_BIND,
-                  MOUNT_MKDIR|MOUNT_TOUCH },
-                { NULL,                     "/run/host/etc/os-release",     NULL,    NULL,                             MS_BIND|MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_REMOUNT,
-                  0 },
-
+                { "/run/host",              "/run/host",                    NULL,    NULL,                             MS_BIND,
+                  MOUNT_FATAL|MOUNT_MKDIR|MOUNT_PREFIX_ROOT }, /* Prepare this so that we can make it read-only when we are done */
+                { "/etc/os-release",        "/run/host/os-release",         NULL,    NULL,                             MS_BIND,
+                  MOUNT_TOUCH }, /* As per kernel interface requirements, bind mount first (creating mount points) and make read-only later */
+                { "/usr/lib/os-release",    "/run/host/os-release",         NULL,    NULL,                             MS_BIND,
+                  MOUNT_FATAL }, /* If /etc/os-release doesn't exist use the version in /usr/lib as fallback */
+                { NULL,                     "/run/host/os-release",         NULL,    NULL,                             MS_BIND|MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_REMOUNT,
+                  MOUNT_FATAL },
+                { NULL,                     "/run/host",                    NULL,    NULL,                             MS_BIND|MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_REMOUNT,
+                  MOUNT_FATAL|MOUNT_IN_USERNS },
 #if HAVE_SELINUX
                 { "/sys/fs/selinux",        "/sys/fs/selinux",              NULL,    NULL,                             MS_BIND,
                   MOUNT_MKDIR },  /* Bind mount first (mkdir/chown the mount point in case /sys/ is mounted as minimal skeleton tmpfs) */
@@ -589,9 +590,9 @@ int mount_all(const char *dest,
         int r;
 
         for (k = 0; k < ELEMENTSOF(mount_table); k++) {
-                _cleanup_free_ char *where = NULL, *options = NULL;
-                const char *o;
+                _cleanup_free_ char *where = NULL, *options = NULL, *prefixed = NULL;
                 bool fatal = FLAGS_SET(mount_table[k].mount_settings, MOUNT_FATAL);
+                const char *o;
 
                 if (in_userns != FLAGS_SET(mount_table[k].mount_settings, MOUNT_IN_USERNS))
                         continue;
@@ -616,20 +617,9 @@ int mount_all(const char *dest,
                                 return log_error_errno(r, "Failed to detect whether %s is a mount point: %m", where);
                         if (r > 0)
                                 continue;
-
-                        /* Shortcut for optional bind mounts: if the source can't be found skip ahead to avoid creating
-                         * empty and unused directories. */
-                        if (!fatal && FLAGS_SET(mount_table[k].mount_settings, MOUNT_MKDIR) && FLAGS_SET(mount_table[k].flags, MS_BIND)) {
-                                r = access(mount_table[k].what, F_OK);
-                                if (r < 0) {
-                                        if (errno == ENOENT)
-                                                continue;
-                                        return log_error_errno(errno, "Failed to stat %s: %m", mount_table[k].what);
-                                }
-                        }
                 }
 
-                if (FLAGS_SET(mount_table[k].mount_settings, MOUNT_MKDIR)) {
+                if ((mount_table[k].mount_settings & (MOUNT_MKDIR|MOUNT_TOUCH)) != 0) {
                         uid_t u = (use_userns && !in_userns) ? uid_shift : UID_INVALID;
 
                         if (FLAGS_SET(mount_table[k].mount_settings, MOUNT_TOUCH))
@@ -647,13 +637,17 @@ int mount_all(const char *dest,
                                 if (r != -EROFS)
                                         continue;
                         }
-                        if (FLAGS_SET(mount_table[k].mount_settings, MOUNT_TOUCH)) {
-                                r = touch(where);
-                                if (r < 0 && r != -EEXIST) {
-                                        if (fatal)
-                                                return log_error_errno(r, "Failed to create mount point %s: %m", where);
-                                        log_debug_errno(r, "Failed to create mount point %s: %m", where);
-                                }
+                }
+
+                if (FLAGS_SET(mount_table[k].mount_settings, MOUNT_TOUCH)) {
+                        r = touch(where);
+                        if (r < 0 && r != -EEXIST) {
+                                if (fatal && r != -EROFS)
+                                        return log_error_errno(r, "Failed to create file %s: %m", where);
+
+                                log_debug_errno(r, "Failed to create file %s: %m", where);
+                                if (r != -EROFS)
+                                        continue;
                         }
                 }
 
@@ -666,8 +660,18 @@ int mount_all(const char *dest,
                                 o = options;
                 }
 
+                if (FLAGS_SET(mount_table[k].mount_settings, MOUNT_PREFIX_ROOT)) {
+                        /* Optionally prefix the mount source with the root dir. This is useful in bind
+                         * mounts to be created within the container image before we transition into it. Note
+                         * that MOUNT_IN_USERNS is run after we transitioned hence prefixing is not ncessary
+                         * for those. */
+                        r = chase_symlinks(mount_table[k].what, dest, CHASE_PREFIX_ROOT, &prefixed, NULL);
+                        if (r < 0)
+                                return log_error_errno(r, "Failed to resolve %s/%s: %m", dest, mount_table[k].what);
+                }
+
                 r = mount_verbose(fatal ? LOG_ERR : LOG_DEBUG,
-                                  mount_table[k].what,
+                                  prefixed ?: mount_table[k].what,
                                   where,
                                   mount_table[k].type,
                                   mount_table[k].flags,
diff --git a/src/nspawn/nspawn-mount.h b/src/nspawn/nspawn-mount.h
index 062ed8b57d..3898c74f18 100644
--- a/src/nspawn/nspawn-mount.h
+++ b/src/nspawn/nspawn-mount.h
@@ -18,6 +18,7 @@ typedef enum MountSettingsMask {
         MOUNT_NON_ROOT_ONLY      = 1 << 7, /* if set, only non-root mounts are mounted */
         MOUNT_MKDIR              = 1 << 8, /* if set, make directory to mount over first */
         MOUNT_TOUCH              = 1 << 9, /* if set, touch file to mount over first */
+        MOUNT_PREFIX_ROOT        = 1 << 10,/* if set, prefix the source path with the container's root directory */
 } MountSettingsMask;
 
 typedef enum CustomMountType {
diff --git a/src/portable/portable.c b/src/portable/portable.c
index 48294d4c49..3a1367ec2b 100644
--- a/src/portable/portable.c
+++ b/src/portable/portable.c
@@ -695,17 +695,28 @@ static int install_chroot_dropin(
         if (!text)
                 return -ENOMEM;
 
-        if (endswith(m->name, ".service"))
+        if (endswith(m->name, ".service")) {
+                const char *os_release_source;
+
+                if (access("/etc/os-release", F_OK) < 0) {
+                        if (errno != ENOENT)
+                                return log_debug_errno(errno, "Failed to check if /etc/os-release exists: %m");
+
+                        os_release_source = "/usr/lib/os-release";
+                } else
+                        os_release_source = "/etc/os-release";
+
                 if (!strextend(&text,
                                "\n"
                                "[Service]\n",
                                IN_SET(type, IMAGE_DIRECTORY, IMAGE_SUBVOLUME) ? "RootDirectory=" : "RootImage=", image_path, "\n"
                                "Environment=PORTABLE=", basename(image_path), "\n"
-                               "BindReadOnlyPaths=-/etc/os-release:/run/host/etc/os-release /usr/lib/os-release:/run/host/usr/lib/os-release\n"
+                               "BindReadOnlyPaths=", os_release_source, ":/run/host/os-release\n"
                                "LogExtraFields=PORTABLE=", basename(image_path), "\n",
                                NULL))
 
                         return -ENOMEM;
+        }
 
         r = write_string_file(dropin, text, WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_ATOMIC);
         if (r < 0)
diff --git a/test/units/testsuite-13.sh b/test/units/testsuite-13.sh
index 0ad75ac8b0..b8fbf00ea6 100755
--- a/test/units/testsuite-13.sh
+++ b/test/units/testsuite-13.sh
@@ -66,12 +66,25 @@ if [ -n "${ID:+set}" ] && [ "${ID}" != "${container_host_id}" ]; then exit 1; fi
 if [ -n "${VERSION_ID:+set}" ] && [ "${VERSION_ID}" != "${container_host_version_id}" ]; then exit 1; fi
 if [ -n "${BUILD_ID:+set}" ] && [ "${BUILD_ID}" != "${container_host_build_id}" ]; then exit 1; fi
 if [ -n "${VARIANT_ID:+set}" ] && [ "${VARIANT_ID}" != "${container_host_variant_id}" ]; then exit 1; fi
-cd /tmp; (cd /run/host/usr/lib; md5sum os-release) | md5sum -c
-if echo test >> /run/host/usr/lib/os-release; then exit 1; fi
-if echo test >> /run/host/etc/os-release; then exit 1; fi
+cd /tmp; (cd /run/host; md5sum os-release) | md5sum -c
+if echo test >> /run/host/os-release; then exit 1; fi
 '
 
-    systemd-nspawn --register=no -D /testsuite-13.nc-container --bind=/etc/os-release:/tmp/os-release /bin/sh -x -e -c "$_cmd"
+    local _os_release_source="/etc/os-release"
+    if [ ! -r "${_os_release_source}" ]; then
+        _os_release_source="/usr/lib/os-release"
+    elif [ -L "${_os_release_source}" ] && rm /etc/os-release; then
+        # Ensure that /etc always wins if available
+        cp /usr/lib/os-release /etc
+        echo MARKER=1 >> /etc/os-release
+    fi
+
+    systemd-nspawn --register=no -D /testsuite-13.nc-container --bind="${_os_release_source}":/tmp/os-release /bin/sh -x -e -c "$_cmd"
+
+    if grep -q MARKER /etc/os-release; then
+        rm /etc/os-release
+        ln -s ../usr/lib/os-release /etc/os-release
+    fi
 }
 
 function run {