Merge pull request #14508 from poettering/namespace-ro-fix

core: be nicer to read-only images

src/basic/path-util.c

@@ -1115,3 +1115,13 @@ bool empty_or_root(const char *root) {
 
         return root[strspn(root, "/")] == 0;
 }
+
+bool path_strv_contains(char **l, const char *path) {
+        char **i;
+
+        STRV_FOREACH(i, l)
+                if (path_equal(*i, path))
+                        return true;
+
+        return false;
+}

src/basic/path-util.h

@@ -71,17 +71,7 @@ static inline bool path_equal_ptr(const char *a, const char *b) {
 }
 
 /* Note: the search terminates on the first NULL item. */
-#define PATH_IN_SET(p, ...)                                     \
-        ({                                                      \
-                char **_s;                                      \
-                bool _found = false;                            \
-                STRV_FOREACH(_s, STRV_MAKE(__VA_ARGS__))        \
-                        if (path_equal(p, *_s)) {               \
-                               _found = true;                   \
-                               break;                           \
-                        }                                       \
-                _found;                                         \
-        })
+#define PATH_IN_SET(p, ...) path_strv_contains(STRV_MAKE(__VA_ARGS__), p)
 
 char* path_startswith_strv(const char *p, char **set);
 #define PATH_STARTSWITH_SET(p, ...) path_startswith_strv(p, STRV_MAKE(__VA_ARGS__))
@@ -181,3 +171,5 @@ bool empty_or_root(const char *root);
 static inline const char *empty_to_root(const char *path) {
         return isempty(path) ? "/" : path;
 }
+
+bool path_strv_contains(char **l, const char *path);

src/core/namespace.c

@@ -1190,6 +1190,57 @@ static void normalize_mounts(const char *root_directory, MountEntry *mounts, siz
         drop_nop(mounts, n_mounts);
 }
 
+static bool root_read_only(
+                char **read_only_paths,
+                ProtectSystem protect_system) {
+
+        /* Determine whether the root directory is going to be read-only given the configured settings. */
+
+        if (protect_system == PROTECT_SYSTEM_STRICT)
+                return true;
+
+        if (path_strv_contains(read_only_paths, "/"))
+                return true;
+
+        return false;
+}
+
+static bool home_read_only(
+                char** read_only_paths,
+                char** inaccessible_paths,
+                char** empty_directories,
+                const BindMount *bind_mounts,
+                size_t n_bind_mounts,
+                const TemporaryFileSystem *temporary_filesystems,
+                size_t n_temporary_filesystems,
+                ProtectHome protect_home) {
+
+        size_t i;
+
+        /* Determine whether the /home directory is going to be read-only given the configured settings. Yes,
+         * this is a bit sloppy, since we don't bother checking for cases where / is affected by multiple
+         * settings. */
+
+        if (protect_home != PROTECT_HOME_NO)
+                return true;
+
+        if (path_strv_contains(read_only_paths, "/home") ||
+            path_strv_contains(inaccessible_paths, "/home") ||
+            path_strv_contains(empty_directories, "/home"))
+                return true;
+
+        for (i = 0; i < n_temporary_filesystems; i++)
+                if (path_equal(temporary_filesystems[i].path, "/home"))
+                        return true;
+
+        /* If /home is overmounted with some dir from the host it's not writable. */
+        for (i = 0; i < n_bind_mounts; i++)
+                if (path_equal(bind_mounts[i].destination, "/home"))
+                        return true;
+
+        return false;
+}
+
 int setup_namespace(
                 const char* root_directory,
                 const char* root_image,
@@ -1228,13 +1279,17 @@ int setup_namespace(
         if (root_image) {
                 dissect_image_flags |= DISSECT_IMAGE_REQUIRE_ROOT;
 
-                if (protect_system == PROTECT_SYSTEM_STRICT &&
-                    protect_home != PROTECT_HOME_NO &&
+                /* Make the whole image read-only if we can determine that we only access it in a read-only fashion. */
+                if (root_read_only(read_only_paths,
+                                   protect_system) &&
+                    home_read_only(read_only_paths, inaccessible_paths, empty_directories,
+                                   bind_mounts, n_bind_mounts, temporary_filesystems, n_temporary_filesystems,
+                                   protect_home) &&
                     strv_isempty(read_write_paths))
                         dissect_image_flags |= DISSECT_IMAGE_READ_ONLY;
 
                 r = loop_device_make_by_path(root_image,
-                                             dissect_image_flags & DISSECT_IMAGE_READ_ONLY ? O_RDONLY : O_RDWR,
+                                             FLAGS_SET(dissect_image_flags, DISSECT_IMAGE_READ_ONLY) ? O_RDONLY : -1 /* < 0 means writable if possible, read-only as fallback */,
                                              LO_FLAGS_PARTSCAN,
                                              &loop_device);
                 if (r < 0)

src/shared/install.c

@@ -105,7 +105,6 @@ DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(unit_file_type, UnitFileType);
 
 static int in_search_path(const LookupPaths *p, const char *path) {
         _cleanup_free_ char *parent = NULL;
-        char **i;
 
         assert(path);
 
@@ -113,11 +112,7 @@ static int in_search_path(const LookupPaths *p, const char *path) {
         if (!parent)
                 return -ENOMEM;
 
-        STRV_FOREACH(i, p->search_path)
-                if (path_equal(parent, *i))
-                        return true;
-
-        return false;
+        return path_strv_contains(p->search_path, parent);
 }
 
 static const char* skip_root(const LookupPaths *p, const char *path) {

src/shared/loop-util.c

@@ -14,6 +14,7 @@
 #include <unistd.h>
 
 #include "alloc-util.h"
+#include "errno-util.h"
 #include "fd-util.h"
 #include "fileio.h"
 #include "loop-util.h"
@@ -157,14 +158,30 @@ int loop_device_make(
 
 int loop_device_make_by_path(const char *path, int open_flags, uint32_t loop_flags, LoopDevice **ret) {
         _cleanup_close_ int fd = -1;
+        int r;
 
         assert(path);
         assert(ret);
-        assert(IN_SET(open_flags, O_RDWR, O_RDONLY));
+        assert(open_flags < 0 || IN_SET(open_flags, O_RDWR, O_RDONLY));
 
-        fd = open(path, O_CLOEXEC|O_NONBLOCK|O_NOCTTY|open_flags);
-        if (fd < 0)
-                return -errno;
+        /* Passing < 0 as open_flags here means we'll try to open the device writable if we can, retrying
+         * read-only if we cannot. */
+
+        fd = open(path, O_CLOEXEC|O_NONBLOCK|O_NOCTTY|(open_flags >= 0 ? open_flags : O_RDWR));
+        if (fd < 0) {
+                r = -errno;
+
+                /* Retry read-only? */
+                if (open_flags >= 0 || !(ERRNO_IS_PRIVILEGE(r) || r == -EROFS))
+                        return r;
+
+                fd = open(path, O_CLOEXEC|O_NONBLOCK|O_NOCTTY|O_RDONLY);
+                if (fd < 0)
+                        return r; /* Propagate original error */
+
+                open_flags = O_RDONLY;
+        } else if (open_flags < 0)
+                open_flags = O_RDWR;
 
         return loop_device_make(fd, open_flags, 0, 0, loop_flags, ret);
 }