From d9c5566c0f9663a4e24bab7f715ccdc2789a183b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 15 Jul 2025 17:21:52 +0200 Subject: [PATCH] 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. --- src/machine/image-dbus.c | 24 ++++---- src/machine/image-varlink.c | 2 +- src/machine/machined-varlink.c | 11 ++-- src/nspawn/nspawn.c | 4 ++ src/portable/portabled-image-bus.c | 4 +- src/shared/discover-image.c | 89 ++++++++++++++++++++++-------- src/shared/discover-image.h | 8 +-- src/sysext/sysext.c | 2 +- 8 files changed, 97 insertions(+), 47 deletions(-) diff --git a/src/machine/image-dbus.c b/src/machine/image-dbus.c index 41dfc8703ed..9d4db0521fc 100644 --- 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 df9e765a6a5..b2a3bd4c931 100644 --- 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 0756d985df2..a00dda1b732 100644 --- 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 adc550ed244..a790f2d8a33 100644 --- 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 332b62a7214..68cf9169246 100644 --- 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 ad4f21c99dd..15875968b78 100644 --- 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 6e5813b9759..55476259d7b 100644 --- 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 c8c75d68bd0..5255ac1df1c 100644 --- 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); } -- 2.47.3