machined: make image locking runtime scope aware, too

We cannot create an image lock in /run if we are unpriv, hence create it
in $XDG_RUNTIME_DIR instead.

diff --git a/src/machine/image-dbus.c b/src/machine/image-dbus.c
index 41dfc8703edd9bd01147d9369a8e8f1221539d58..9d4db0521fc191e776c8940c7472586abd1f9f73 100644 (file)
--- a/src/machine/image-dbus.c
+++ b/src/machine/image-dbus.c
@@ -205,7 +205,7 @@ int bus_image_method_mark_read_only(
                 sd_bus_error *error) {
 
         Image *image = userdata;
-        Manager *m = image->userdata;
+        Manager *m = ASSERT_PTR(image->userdata);
         int read_only, r;
 
         assert(message);
@@ -234,7 +234,7 @@ int bus_image_method_mark_read_only(
                         return 1; /* Will call us back */
         }
 
-        r = image_read_only(image, read_only);
+        r = image_read_only(image, read_only, m->runtime_scope);
         if (r < 0)
                 return r;
 
@@ -290,11 +290,12 @@ int bus_image_method_get_hostname(
                 void *userdata,
                 sd_bus_error *error) {
 
-        Image *image = userdata;
+        Image *image = ASSERT_PTR(userdata);
+        Manager *m = ASSERT_PTR(image->userdata);
         int r;
 
         if (!image->metadata_valid) {
-                r = image_read_metadata(image, &image_policy_container);
+                r = image_read_metadata(image, &image_policy_container, m->runtime_scope);
                 if (r < 0)
                         return sd_bus_error_set_errnof(error, r, "Failed to read image metadata: %m");
         }
@@ -308,11 +309,12 @@ int bus_image_method_get_machine_id(
                 sd_bus_error *error) {
 
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
-        Image *image = userdata;
+        Image *image = ASSERT_PTR(userdata);
+        Manager *m = ASSERT_PTR(image->userdata);
         int r;
 
         if (!image->metadata_valid) {
-                r = image_read_metadata(image, &image_policy_container);
+                r = image_read_metadata(image, &image_policy_container, m->runtime_scope);
                 if (r < 0)
                         return sd_bus_error_set_errnof(error, r, "Failed to read image metadata: %m");
         }
@@ -336,11 +338,12 @@ int bus_image_method_get_machine_info(
                 void *userdata,
                 sd_bus_error *error) {
 
-        Image *image = userdata;
+        Image *image = ASSERT_PTR(userdata);
+        Manager *m = ASSERT_PTR(image->userdata);
         int r;
 
         if (!image->metadata_valid) {
-                r = image_read_metadata(image, &image_policy_container);
+                r = image_read_metadata(image, &image_policy_container, m->runtime_scope);
                 if (r < 0)
                         return sd_bus_error_set_errnof(error, r, "Failed to read image metadata: %m");
         }
@@ -353,11 +356,12 @@ int bus_image_method_get_os_release(
                 void *userdata,
                 sd_bus_error *error) {
 
-        Image *image = userdata;
+        Image *image = ASSERT_PTR(userdata);
+        Manager *m = ASSERT_PTR(image->userdata);
         int r;
 
         if (!image->metadata_valid) {
-                r = image_read_metadata(image, &image_policy_container);
+                r = image_read_metadata(image, &image_policy_container, m->runtime_scope);
                 if (r < 0)
                         return sd_bus_error_set_errnof(error, r, "Failed to read image metadata: %m");
         }

diff --git a/src/machine/image-varlink.c b/src/machine/image-varlink.c
index df9e765a6a5f8b09e7279dfe403833052482ee53..b2a3bd4c93193f3262e892381489decbb07148b4 100644 (file)
--- a/src/machine/image-varlink.c
+++ b/src/machine/image-varlink.c
@@ -85,7 +85,7 @@ int vl_method_update_image(sd_varlink *link, sd_json_variant *parameters, sd_var
         }
 
         if (p.read_only >= 0) {
-                r = image_read_only(image, p.read_only);
+                r = image_read_only(image, p.read_only, manager->runtime_scope);
                 if (r < 0)
                         RET_GATHER(ret, log_debug_errno(r, "Failed to toggle image read only, ignoring: %m"));
         }

diff --git a/src/machine/machined-varlink.c b/src/machine/machined-varlink.c
index 0756d985df263a508bc2fcc96d2f3433264a330a..a00dda1b732b1362ebf595fb39da68e57a7179a9 100644 (file)
--- a/src/machine/machined-varlink.c
+++ b/src/machine/machined-varlink.c
@@ -615,14 +615,15 @@ static int vl_method_open_root_directory(sd_varlink *link, sd_json_variant *para
         return lookup_machine_and_call_method(link, parameters, flags, userdata, vl_method_open_root_directory_internal);
 }
 
-static int list_image_one_and_maybe_read_metadata(sd_varlink *link, Image *image, bool more, AcquireMetadata am) {
+static int list_image_one_and_maybe_read_metadata(Manager *m, sd_varlink *link, Image *image, bool more, AcquireMetadata am) {
         int r;
 
+        assert(m);
         assert(link);
         assert(image);
 
         if (should_acquire_metadata(am) && !image->metadata_valid) {
-                r = image_read_metadata(image, &image_policy_container);
+                r = image_read_metadata(image, &image_policy_container, m->runtime_scope);
                 if (r < 0 && am != ACQUIRE_METADATA_GRACEFUL)
                         return log_debug_errno(r, "Failed to read image metadata: %m");
                 if (r < 0)
@@ -698,7 +699,7 @@ static int vl_method_list_images(sd_varlink *link, sd_json_variant *parameters,
                 if (r < 0)
                         return log_debug_errno(r, "Failed to find image: %m");
 
-                return list_image_one_and_maybe_read_metadata(link, found, /* more = */ false, p.acquire_metadata);
+                return list_image_one_and_maybe_read_metadata(m, link, found, /* more = */ false, p.acquire_metadata);
         }
 
         if (!FLAGS_SET(flags, SD_VARLINK_METHOD_MORE))
@@ -712,7 +713,7 @@ static int vl_method_list_images(sd_varlink *link, sd_json_variant *parameters,
         Image *image, *previous = NULL;
         HASHMAP_FOREACH(image, images) {
                 if (previous) {
-                        r = list_image_one_and_maybe_read_metadata(link, previous, /* more = */ true, p.acquire_metadata);
+                        r = list_image_one_and_maybe_read_metadata(m, link, previous, /* more = */ true, p.acquire_metadata);
                         if (r < 0)
                                 return r;
                 }
@@ -721,7 +722,7 @@ static int vl_method_list_images(sd_varlink *link, sd_json_variant *parameters,
         }
 
         if (previous)
-                return list_image_one_and_maybe_read_metadata(link, previous, /* more = */ false, p.acquire_metadata);
+                return list_image_one_and_maybe_read_metadata(m, link, previous, /* more = */ false, p.acquire_metadata);
 
         return sd_varlink_error(link, VARLINK_ERROR_MACHINE_IMAGE_NO_SUCH_IMAGE, NULL);
 }

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index adc550ed2447514b4ba8d433ab17758230824a6f..a790f2d8a337892eed22bfc2741dd3272c131d44 100644 (file)
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -6055,6 +6055,7 @@ static int run(int argc, char *argv[]) {
                         /* We take an exclusive lock on this image, since it's our private, ephemeral copy
                          * only owned by us and no one else. */
                         r = image_path_lock(
+                                        arg_privileged ? RUNTIME_SCOPE_SYSTEM : RUNTIME_SCOPE_USER,
                                         np,
                                         LOCK_EX|LOCK_NB,
                                         arg_privileged ? &tree_global_lock : NULL,
@@ -6091,6 +6092,7 @@ static int run(int argc, char *argv[]) {
                                 goto finish;
 
                         r = image_path_lock(
+                                        arg_privileged ? RUNTIME_SCOPE_SYSTEM : RUNTIME_SCOPE_USER,
                                         arg_directory,
                                         (arg_read_only ? LOCK_SH : LOCK_EX) | LOCK_NB,
                                         arg_privileged ? &tree_global_lock : NULL,
@@ -6218,6 +6220,7 @@ static int run(int argc, char *argv[]) {
 
                         /* Always take an exclusive lock on our own ephemeral copy. */
                         r = image_path_lock(
+                                        arg_privileged ? RUNTIME_SCOPE_SYSTEM : RUNTIME_SCOPE_USER,
                                         np,
                                         LOCK_EX|LOCK_NB,
                                         arg_privileged ? &tree_global_lock : NULL,
@@ -6245,6 +6248,7 @@ static int run(int argc, char *argv[]) {
                         remove_image = true;
                 } else {
                         r = image_path_lock(
+                                        arg_privileged ? RUNTIME_SCOPE_SYSTEM : RUNTIME_SCOPE_USER,
                                         arg_image,
                                         (arg_read_only ? LOCK_SH : LOCK_EX) | LOCK_NB,
                                         arg_privileged ? &tree_global_lock : NULL,

diff --git a/src/portable/portabled-image-bus.c b/src/portable/portabled-image-bus.c
index 332b62a72142a9c0df881c60bf2036197069ffa1..68cf91692466a2f9aa4bba35020e272c7decc842 100644 (file)
--- a/src/portable/portabled-image-bus.c
+++ b/src/portable/portabled-image-bus.c
@@ -61,7 +61,7 @@ int bus_image_common_get_os_release(
                 return 1;
 
         if (!image->metadata_valid) {
-                r = image_read_metadata(image, &image_policy_service);
+                r = image_read_metadata(image, &image_policy_service, m->runtime_scope);
                 if (r < 0)
                         return sd_bus_error_set_errnof(error, r, "Failed to read image metadata: %m");
         }
@@ -801,7 +801,7 @@ int bus_image_common_mark_read_only(
         if (r == 0)
                 return 1; /* Will call us back */
 
-        r = image_read_only(image, read_only);
+        r = image_read_only(image, read_only, m->runtime_scope);
         if (r < 0)
                 return r;
 

diff --git a/src/shared/discover-image.c b/src/shared/discover-image.c
index ad4f21c99ddc8ca4f783c27bb5c6c3e7c8c2a4cb..15875968b78e00ee083484192601555cca596b7e 100644 (file)
--- a/src/shared/discover-image.c
+++ b/src/shared/discover-image.c
@@ -19,6 +19,7 @@
 #include "chase.h"
 #include "chattr-util.h"
 #include "copy.h"
+#include "devnum-util.h"
 #include "dirent-util.h"
 #include "discover-image.h"
 #include "dissect-image.h"
@@ -37,6 +38,7 @@
 #include "mkdir.h"
 #include "nulstr-util.h"
 #include "os-util.h"
+#include "path-lookup.h"
 #include "path-util.h"
 #include "rm-rf.h"
 #include "runtime-scope.h"
@@ -1149,7 +1151,7 @@ int image_remove(Image *i, RuntimeScope scope) {
                 return r;
 
         /* Make sure we don't interfere with a running nspawn */
-        r = image_path_lock(i->path, LOCK_EX|LOCK_NB, &global_lock, &local_lock);
+        r = image_path_lock(scope, i->path, LOCK_EX|LOCK_NB, &global_lock, &local_lock);
         if (r < 0)
                 return r;
 
@@ -1244,14 +1246,14 @@ int image_rename(Image *i, const char *new_name, RuntimeScope scope) {
                 return r;
 
         /* Make sure we don't interfere with a running nspawn */
-        r = image_path_lock(i->path, LOCK_EX|LOCK_NB, &global_lock, &local_lock);
+        r = image_path_lock(scope, i->path, LOCK_EX|LOCK_NB, &global_lock, &local_lock);
         if (r < 0)
                 return r;
 
         /* Make sure nobody takes the new name, between the time we
          * checked it is currently unused in all search paths, and the
          * time we take possession of it */
-        r = image_name_lock(new_name, LOCK_EX|LOCK_NB, &name_lock);
+        r = image_name_lock(scope, new_name, LOCK_EX|LOCK_NB, &name_lock);
         if (r < 0)
                 return r;
 
@@ -1418,7 +1420,7 @@ int image_clone(Image *i, const char *new_name, bool read_only, RuntimeScope sco
         /* Make sure nobody takes the new name, between the time we
          * checked it is currently unused in all search paths, and the
          * time we take possession of it */
-        r = image_name_lock(new_name, LOCK_EX|LOCK_NB, &name_lock);
+        r = image_name_lock(scope, new_name, LOCK_EX|LOCK_NB, &name_lock);
         if (r < 0)
                 return r;
 
@@ -1485,7 +1487,7 @@ int image_clone(Image *i, const char *new_name, bool read_only, RuntimeScope sco
         return 0;
 }
 
-int image_read_only(Image *i, bool b) {
+int image_read_only(Image *i, bool b, RuntimeScope scope) {
         _cleanup_(release_lock_file) LockFile global_lock = LOCK_FILE_INIT, local_lock = LOCK_FILE_INIT;
         int r;
 
@@ -1495,7 +1497,7 @@ int image_read_only(Image *i, bool b) {
                 return -EROFS;
 
         /* Make sure we don't interfere with a running nspawn */
-        r = image_path_lock(i->path, LOCK_EX|LOCK_NB, &global_lock, &local_lock);
+        r = image_path_lock(scope, i->path, LOCK_EX|LOCK_NB, &global_lock, &local_lock);
         if (r < 0)
                 return r;
 
@@ -1572,12 +1574,33 @@ int image_read_only(Image *i, bool b) {
         return 0;
 }
 
-static void make_lock_dir(void) {
-        (void) mkdir_p("/run/systemd/nspawn", 0755);
-        (void) mkdir("/run/systemd/nspawn/locks", 0700);
+static int make_lock_dir(RuntimeScope scope) {
+        int r;
+
+        _cleanup_free_ char *p = NULL;
+        r = runtime_directory_generic(scope, "systemd", &p);
+        if (r < 0)
+                return r;
+
+        _cleanup_close_ int pfd = open_mkdir_at(AT_FDCWD, p, O_CLOEXEC, 0755);
+        if (pfd < 0)
+                return pfd;
+
+        _cleanup_close_ int nfd = open_mkdir_at(pfd, "nspawn", O_CLOEXEC, 0755);
+        if (nfd < 0)
+                return nfd;
+
+        r = RET_NERRNO(mkdirat(nfd, "locks", 0700));
+        if (r == -EEXIST)
+                return 0;
+        if (r < 0)
+                return r;
+
+        return 1;
 }
 
 int image_path_lock(
+                RuntimeScope scope,
                 const char *path,
                 int operation,
                 LockFile *ret_global,
@@ -1593,7 +1616,7 @@ int image_path_lock(
         assert(ret_local);
 
         /* Locks an image path. This actually creates two locks: one "local" one, next to the image path
-         * itself, which might be shared via NFS. And another "global" one, in /run, that uses the
+         * itself, which might be shared via NFS. And another "global" one, in /run/, that uses the
          * device/inode number. This has the benefit that we can even lock a tree that is a mount point,
          * correctly. */
 
@@ -1637,15 +1660,21 @@ int image_path_lock(
         }
 
         if (ret_global) {
-                if (stat(path, &st) >= 0) {
+                if (stat(path, &st) < 0)
+                        log_debug_errno(errno, "Failed to stat() image '%s', not locking image: %m", path);
+                else {
+                        r = runtime_directory_generic(scope, "systemd/nspawn/locks/", &p);
+                        if (r < 0)
+                                return r;
+
                         if (S_ISBLK(st.st_mode))
-                                r = asprintf(&p, "/run/systemd/nspawn/locks/block-%u:%u", major(st.st_rdev), minor(st.st_rdev));
+                                r = strextendf(&p, "block-" DEVNUM_FORMAT_STR, DEVNUM_FORMAT_VAL(st.st_rdev));
                         else if (S_ISDIR(st.st_mode) || S_ISREG(st.st_mode))
-                                r = asprintf(&p, "/run/systemd/nspawn/locks/inode-%lu:%lu", (unsigned long) st.st_dev, (unsigned long) st.st_ino);
+                                r = strextendf(&p, "inode-%" PRIu64 ":%" PRIu64, (uint64_t) st.st_dev, (uint64_t) st.st_ino);
                         else
                                 return -ENOTTY;
                         if (r < 0)
-                                return -ENOMEM;
+                                return r;
                 }
         }
 
@@ -1654,15 +1683,15 @@ int image_path_lock(
         if (!path_startswith(path, "/dev/")) {
                 r = make_lock_file_for(path, operation, &t);
                 if (r < 0) {
-                        if (!exclusive && r == -EROFS)
-                                log_debug_errno(r, "Failed to create shared lock for '%s', ignoring: %m", path);
-                        else
+                        if (exclusive || r != -EROFS)
                                 return r;
+
+                        log_debug_errno(r, "Failed to create shared lock for '%s', ignoring: %m", path);
                 }
         }
 
         if (p) {
-                make_lock_dir();
+                (void) make_lock_dir(scope);
 
                 r = make_lock_file(p, operation, ret_global);
                 if (r < 0) {
@@ -1728,13 +1757,13 @@ int image_set_pool_limit(RuntimeScope scope, ImageClass class, uint64_t referenc
         return 0;
 }
 
-int image_read_metadata(Image *i, const ImagePolicy *image_policy) {
+int image_read_metadata(Image *i, const ImagePolicy *image_policy, RuntimeScope scope) {
         _cleanup_(release_lock_file) LockFile global_lock = LOCK_FILE_INIT, local_lock = LOCK_FILE_INIT;
         int r;
 
         assert(i);
 
-        r = image_path_lock(i->path, LOCK_SH|LOCK_NB, &global_lock, &local_lock);
+        r = image_path_lock(scope, i->path, LOCK_SH|LOCK_NB, &global_lock, &local_lock);
         if (r < 0)
                 return r;
 
@@ -1864,8 +1893,13 @@ int image_read_metadata(Image *i, const ImagePolicy *image_policy) {
         return 0;
 }
 
-int image_name_lock(const char *name, int operation, LockFile *ret) {
-        const char *p;
+int image_name_lock(
+                RuntimeScope scope,
+                const char *name,
+                int operation,
+                LockFile *ret) {
+
+        int r;
 
         assert(name);
         assert(ret);
@@ -1883,9 +1917,16 @@ int image_name_lock(const char *name, int operation, LockFile *ret) {
                 return 0;
         }
 
-        make_lock_dir();
+        (void) make_lock_dir(scope);
+
+        _cleanup_free_ char *p = NULL;
+        r = runtime_directory_generic(scope, "systemd/nspawn/locks/name-", &p);
+        if (r < 0)
+                return r;
+
+        if (!strextend(&p, name))
+                return -ENOMEM;
 
-        p = strjoina("/run/systemd/nspawn/locks/name-", name);
         return make_lock_file(p, operation, ret);
 }
 

diff --git a/src/shared/discover-image.h b/src/shared/discover-image.h
index 6e5813b9759b8f69e508953e83032d5d9955d260..55476259d7b07e987fec4ef481ea72e8fd02a3c3 100644 (file)
--- a/src/shared/discover-image.h
+++ b/src/shared/discover-image.h
@@ -58,18 +58,18 @@ int image_discover(RuntimeScope scope, ImageClass class, const char *root, Hashm
 int image_remove(Image *i, RuntimeScope scope);
 int image_rename(Image *i, const char *new_name, RuntimeScope scope);
 int image_clone(Image *i, const char *new_name, bool read_only, RuntimeScope scope);
-int image_read_only(Image *i, bool b);
+int image_read_only(Image *i, bool b, RuntimeScope scope);
 
 const char* image_type_to_string(ImageType t) _const_;
 ImageType image_type_from_string(const char *s) _pure_;
 
-int image_path_lock(const char *path, int operation, LockFile *global, LockFile *local);
-int image_name_lock(const char *name, int operation, LockFile *ret);
+int image_path_lock(RuntimeScope scope, const char *path, int operation, LockFile *global, LockFile *local);
+int image_name_lock(RuntimeScope scope, const char *name, int operation, LockFile *ret);
 
 int image_set_limit(Image *i, uint64_t referenced_max);
 int image_set_pool_limit(RuntimeScope scope, ImageClass class, uint64_t referenced_max);
 
-int image_read_metadata(Image *i, const ImagePolicy *image_policy);
+int image_read_metadata(Image *i, const ImagePolicy *image_policy, RuntimeScope scope);
 
 bool image_in_search_path(RuntimeScope scope, ImageClass class, const char *root, const char *image);
 

diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c
index c8c75d68bd07703f5757efe68a55f65755694a6f..5255ac1df1cf083735127cfd0de09091755023db 100644 (file)
--- a/src/sysext/sysext.c
+++ b/src/sysext/sysext.c
@@ -2127,7 +2127,7 @@ static int image_discover_and_read_metadata(ImageClass image_class, Hashmap **re
                 return log_error_errno(r, "Failed to discover images: %m");
 
         HASHMAP_FOREACH(img, images) {
-                r = image_read_metadata(img, image_class_info[image_class].default_image_policy);
+                r = image_read_metadata(img, image_class_info[image_class].default_image_policy, RUNTIME_SCOPE_SYSTEM);
                 if (r < 0)
                         return log_error_errno(r, "Failed to read metadata for image %s: %m", img->name);
         }