From 53cbf5f9a6e2544b8fbe883c05c9463d2a5f54f6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 8 Apr 2023 17:15:09 +0900 Subject: [PATCH 01/13] os-util: drop fopen_extension_release() --- src/basic/os-util.c | 36 ++++++------------------------------ src/basic/os-util.h | 5 ----- 2 files changed, 6 insertions(+), 35 deletions(-) diff --git a/src/basic/os-util.c b/src/basic/os-util.c index 5f1c26b87c..0bc1aeb25c 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -248,40 +248,16 @@ int open_extension_release(const char *root, ImageClass image_class, const char return 0; } -int fopen_extension_release(const char *root, ImageClass image_class, const char *extension, bool relax_extension_release_check, char **ret_path, FILE **ret_file) { - _cleanup_free_ char *p = NULL; - _cleanup_close_ int fd = -EBADF; - FILE *f; - int r; - - if (!ret_file) - return open_extension_release(root, image_class, extension, relax_extension_release_check, ret_path, NULL); - - r = open_extension_release(root, image_class, extension, relax_extension_release_check, ret_path ? &p : NULL, &fd); - if (r < 0) - return r; - - f = take_fdopen(&fd, "r"); - if (!f) - return -errno; - - if (ret_path) - *ret_path = TAKE_PTR(p); - *ret_file = f; - - return 0; -} - static int parse_release_internal(const char *root, ImageClass image_class, bool relax_extension_release_check, const char *extension, va_list ap) { - _cleanup_fclose_ FILE *f = NULL; + _cleanup_close_ int fd = -EBADF; _cleanup_free_ char *p = NULL; int r; - r = fopen_extension_release(root, image_class, extension, relax_extension_release_check, &p, &f); + r = open_extension_release(root, image_class, extension, relax_extension_release_check, &p, &fd); if (r < 0) return r; - return parse_env_filev(f, p, ap); + return parse_env_file_fdv(fd, p, ap); } int _parse_extension_release(const char *root, ImageClass image_class, bool relax_extension_release_check, const char *extension, ...) { @@ -339,15 +315,15 @@ int load_os_release_pairs_with_prefix(const char *root, const char *prefix, char } int load_extension_release_pairs(const char *root, ImageClass image_class, const char *extension, bool relax_extension_release_check, char ***ret) { - _cleanup_fclose_ FILE *f = NULL; + _cleanup_close_ int fd = -EBADF; _cleanup_free_ char *p = NULL; int r; - r = fopen_extension_release(root, image_class, extension, relax_extension_release_check, &p, &f); + r = open_extension_release(root, image_class, extension, relax_extension_release_check, &p, &fd); if (r < 0) return r; - return load_env_file_pairs(f, p, ret); + return load_env_file_pairs_fd(fd, p, ret); } int os_release_support_ended(const char *support_end, bool quiet, usec_t *ret_eol) { diff --git a/src/basic/os-util.h b/src/basic/os-util.h index 01dcde7a80..374b4ee584 100644 --- a/src/basic/os-util.h +++ b/src/basic/os-util.h @@ -33,11 +33,6 @@ static inline int open_os_release(const char *root, char **ret_path, int *ret_fd return open_extension_release(root, _IMAGE_CLASS_INVALID, NULL, false, ret_path, ret_fd); } -int fopen_extension_release(const char *root, ImageClass image_class, const char *extension, bool relax_extension_release_check, char **ret_path, FILE **ret_file); -static inline int fopen_os_release(const char *root, char **ret_path, FILE **ret_file) { - return fopen_extension_release(root, _IMAGE_CLASS_INVALID, NULL, false, ret_path, ret_file); -} - int _parse_extension_release(const char *root, ImageClass image_class, bool relax_extension_release_check, const char *extension, ...) _sentinel_; int _parse_os_release(const char *root, ...) _sentinel_; #define parse_extension_release(root, image_class, relax_extension_release_check, extension, ...) _parse_extension_release(root, image_class, relax_extension_release_check, extension, __VA_ARGS__, NULL) From 396ec9587ce075cac06ab653cda5f4850e2855cd Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 8 Apr 2023 18:00:31 +0900 Subject: [PATCH 02/13] os-util: make open_extension_release() return O_PATH fd --- src/basic/os-util.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/basic/os-util.c b/src/basic/os-util.c index 0bc1aeb25c..c1874e2572 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -230,18 +230,8 @@ int open_extension_release(const char *root, ImageClass image_class, const char if (r < 0) return r; - if (ret_fd) { - int real_fd; - - /* Convert the O_PATH fd into a proper, readable one */ - real_fd = fd_reopen(fd, O_RDONLY|O_CLOEXEC|O_NOCTTY); - safe_close(fd); - if (real_fd < 0) - return real_fd; - - *ret_fd = real_fd; - } - + if (ret_fd) + *ret_fd = TAKE_FD(fd); if (ret_path) *ret_path = TAKE_PTR(q); From 6f0f4d14889e459f9c05887aff5abf5413f3b24b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 9 Apr 2023 01:11:52 +0900 Subject: [PATCH 03/13] os-util: fix fd leak on failure --- src/basic/os-util.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/basic/os-util.c b/src/basic/os-util.c index c1874e2572..79056ccd1e 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -123,8 +123,9 @@ static int extension_release_strict_xattr_value(int extension_release_fd, const } int open_extension_release(const char *root, ImageClass image_class, const char *extension, bool relax_extension_release_check, char **ret_path, int *ret_fd) { + _cleanup_close_ int fd = -EBADF; _cleanup_free_ char *q = NULL; - int r, fd; + int r; if (extension) { assert(image_class >= 0); From a84677e0f43e142094ebb1a870cd840cb2cac158 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 9 Apr 2023 01:02:13 +0900 Subject: [PATCH 04/13] os-util: split-out open_os_release() from open_extension_release() The logics of opening os-release and extension-release are completely different. No functional change, just refactoring. --- src/basic/os-util.c | 186 +++++++++++++++++++++++--------------------- src/basic/os-util.h | 4 +- 2 files changed, 100 insertions(+), 90 deletions(-) diff --git a/src/basic/os-util.c b/src/basic/os-util.c index 79056ccd1e..17ab376ac2 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -122,111 +122,123 @@ static int extension_release_strict_xattr_value(int extension_release_fd, const return false; } -int open_extension_release(const char *root, ImageClass image_class, const char *extension, bool relax_extension_release_check, char **ret_path, int *ret_fd) { +int open_os_release(const char *root, char **ret_path, int *ret_fd) { + const char *e; + int r; + + e = secure_getenv("SYSTEMD_OS_RELEASE"); + if (e) + return chase(e, root, 0, ret_path, ret_fd); + + FOREACH_STRING(path, "/etc/os-release", "/usr/lib/os-release") { + r = chase(path, root, CHASE_PREFIX_ROOT, ret_path, ret_fd); + if (r != -ENOENT) + return r; + } + + return -ENOENT; +} + +int open_extension_release( + const char *root, + ImageClass image_class, + const char *extension, + bool relax_extension_release_check, + char **ret_path, + int *ret_fd) { + _cleanup_close_ int fd = -EBADF; _cleanup_free_ char *q = NULL; int r; - if (extension) { - assert(image_class >= 0); - assert(image_class < _IMAGE_CLASS_MAX); + assert(!extension || (image_class >= 0 && image_class < _IMAGE_CLASS_MAX)); - const char *extension_full_path; + if (!extension) + return open_os_release(root, ret_path, ret_fd); - if (!image_name_is_valid(extension)) - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), - "The extension name %s is invalid.", extension); + const char *extension_full_path; - extension_full_path = strjoina(image_class_release_info[image_class].release_file_path_prefix, extension); - r = chase(extension_full_path, root, CHASE_PREFIX_ROOT, ret_path ? &q : NULL, ret_fd ? &fd : NULL); - log_full_errno_zerook(LOG_DEBUG, MIN(r, 0), "Checking for %s: %m", extension_full_path); + if (!image_name_is_valid(extension)) + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "The extension name %s is invalid.", extension); - /* Cannot find the expected extension-release file? The image filename might have been - * mangled on deployment, so fallback to checking for any file in the extension-release.d - * directory, and return the first one with a user.extension-release xattr instead. - * The user.extension-release.strict xattr is checked to ensure the author of the image - * considers it OK if names do not match. */ - if (r == -ENOENT) { - _cleanup_free_ char *extension_release_dir_path = NULL; - _cleanup_closedir_ DIR *extension_release_dir = NULL; + extension_full_path = strjoina(image_class_release_info[image_class].release_file_path_prefix, extension); + r = chase(extension_full_path, root, CHASE_PREFIX_ROOT, ret_path ? &q : NULL, ret_fd ? &fd : NULL); + log_full_errno_zerook(LOG_DEBUG, MIN(r, 0), "Checking for %s: %m", extension_full_path); - r = chase_and_opendir(image_class_release_info[image_class].release_file_directory, root, CHASE_PREFIX_ROOT, - &extension_release_dir_path, &extension_release_dir); - if (r < 0) - return log_debug_errno(r, "Cannot open %s%s, ignoring: %m", root, image_class_release_info[image_class].release_file_directory); + /* Cannot find the expected extension-release file? The image filename might have been mangled on + * deployment, so fallback to checking for any file in the extension-release.d directory, and return + * the first one with a user.extension-release xattr instead. The user.extension-release.strict + * xattr is checked to ensure the author of the image considers it OK if names do not match. */ + if (r == -ENOENT) { + _cleanup_free_ char *extension_release_dir_path = NULL; + _cleanup_closedir_ DIR *extension_release_dir = NULL; - r = -ENOENT; - FOREACH_DIRENT(de, extension_release_dir, return -errno) { - int k; + r = chase_and_opendir(image_class_release_info[image_class].release_file_directory, root, CHASE_PREFIX_ROOT, + &extension_release_dir_path, &extension_release_dir); + if (r < 0) + return log_debug_errno(r, "Cannot open %s%s, ignoring: %m", root, image_class_release_info[image_class].release_file_directory); - if (!IN_SET(de->d_type, DT_REG, DT_UNKNOWN)) + r = -ENOENT; + FOREACH_DIRENT(de, extension_release_dir, return -errno) { + int k; + + if (!IN_SET(de->d_type, DT_REG, DT_UNKNOWN)) + continue; + + const char *image_name = startswith(de->d_name, "extension-release."); + if (!image_name) + continue; + + if (!image_name_is_valid(image_name)) { + log_debug("%s/%s is not a valid release file name, ignoring.", + extension_release_dir_path, de->d_name); + continue; + } + + /* We already chased the directory, and checked that + * this is a real file, so we shouldn't fail to open it. */ + _cleanup_close_ int extension_release_fd = openat(dirfd(extension_release_dir), + de->d_name, + O_PATH|O_CLOEXEC|O_NOFOLLOW); + if (extension_release_fd < 0) + return log_debug_errno(errno, + "Failed to open release file %s/%s: %m", + extension_release_dir_path, + de->d_name); + + /* Really ensure it is a regular file after we open it. */ + if (fd_verify_regular(extension_release_fd) < 0) { + log_debug("%s/%s is not a regular file, ignoring.", extension_release_dir_path, de->d_name); + continue; + } + + if (!relax_extension_release_check) { + k = extension_release_strict_xattr_value(extension_release_fd, + extension_release_dir_path, + de->d_name); + if (k != 0) continue; + } - const char *image_name = startswith(de->d_name, "extension-release."); - if (!image_name) - continue; + /* We already found what we were looking for, but there's another candidate? + * We treat this as an error, as we want to enforce that there are no ambiguities + * in case we are in the fallback path. */ + if (r == 0) { + r = -ENOTUNIQ; + break; + } - if (!image_name_is_valid(image_name)) { - log_debug("%s/%s is not a valid release file name, ignoring.", - extension_release_dir_path, de->d_name); - continue; - } + r = 0; /* Found it! */ - /* We already chased the directory, and checked that - * this is a real file, so we shouldn't fail to open it. */ - _cleanup_close_ int extension_release_fd = openat(dirfd(extension_release_dir), - de->d_name, - O_PATH|O_CLOEXEC|O_NOFOLLOW); - if (extension_release_fd < 0) - return log_debug_errno(errno, - "Failed to open release file %s/%s: %m", - extension_release_dir_path, - de->d_name); + if (ret_fd) + fd = TAKE_FD(extension_release_fd); - /* Really ensure it is a regular file after we open it. */ - if (fd_verify_regular(extension_release_fd) < 0) { - log_debug("%s/%s is not a regular file, ignoring.", extension_release_dir_path, de->d_name); - continue; - } - - if (!relax_extension_release_check) { - k = extension_release_strict_xattr_value(extension_release_fd, - extension_release_dir_path, - de->d_name); - if (k != 0) - continue; - } - - /* We already found what we were looking for, but there's another candidate? - * We treat this as an error, as we want to enforce that there are no ambiguities - * in case we are in the fallback path. */ - if (r == 0) { - r = -ENOTUNIQ; - break; - } - - r = 0; /* Found it! */ - - if (ret_fd) - fd = TAKE_FD(extension_release_fd); - - if (ret_path) { - q = path_join(extension_release_dir_path, de->d_name); - if (!q) - return -ENOMEM; - } + if (ret_path) { + q = path_join(extension_release_dir_path, de->d_name); + if (!q) + return -ENOMEM; } } - } else { - const char *var = secure_getenv("SYSTEMD_OS_RELEASE"); - if (var) - r = chase(var, root, 0, ret_path ? &q : NULL, ret_fd ? &fd : NULL); - else - FOREACH_STRING(path, "/etc/os-release", "/usr/lib/os-release") { - r = chase(path, root, CHASE_PREFIX_ROOT, ret_path ? &q : NULL, ret_fd ? &fd : NULL); - if (r != -ENOENT) - break; - } } if (r < 0) return r; diff --git a/src/basic/os-util.h b/src/basic/os-util.h index 374b4ee584..bd9e8949e6 100644 --- a/src/basic/os-util.h +++ b/src/basic/os-util.h @@ -29,9 +29,7 @@ static inline int path_is_os_tree(const char *path) { } int open_extension_release(const char *root, ImageClass image_class, const char *extension, bool relax_extension_release_check, char **ret_path, int *ret_fd); -static inline int open_os_release(const char *root, char **ret_path, int *ret_fd) { - return open_extension_release(root, _IMAGE_CLASS_INVALID, NULL, false, ret_path, ret_fd); -} +int open_os_release(const char *root, char **ret_path, int *ret_fd); int _parse_extension_release(const char *root, ImageClass image_class, bool relax_extension_release_check, const char *extension, ...) _sentinel_; int _parse_os_release(const char *root, ...) _sentinel_; From 7213c75045accf542fd2e3d38317ef4dfa4219a4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 9 Apr 2023 01:09:09 +0900 Subject: [PATCH 05/13] os-util: return earlier when extension release file is found No functional change, just refactoring. --- src/basic/os-util.c | 125 ++++++++++++++++++++++---------------------- 1 file changed, 63 insertions(+), 62 deletions(-) diff --git a/src/basic/os-util.c b/src/basic/os-util.c index 17ab376ac2..619a2bae96 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -162,82 +162,83 @@ int open_extension_release( return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "The extension name %s is invalid.", extension); extension_full_path = strjoina(image_class_release_info[image_class].release_file_path_prefix, extension); - r = chase(extension_full_path, root, CHASE_PREFIX_ROOT, ret_path ? &q : NULL, ret_fd ? &fd : NULL); + r = chase(extension_full_path, root, CHASE_PREFIX_ROOT, ret_path, ret_fd); log_full_errno_zerook(LOG_DEBUG, MIN(r, 0), "Checking for %s: %m", extension_full_path); + if (r != -ENOENT) + return r; /* Cannot find the expected extension-release file? The image filename might have been mangled on * deployment, so fallback to checking for any file in the extension-release.d directory, and return * the first one with a user.extension-release xattr instead. The user.extension-release.strict * xattr is checked to ensure the author of the image considers it OK if names do not match. */ - if (r == -ENOENT) { - _cleanup_free_ char *extension_release_dir_path = NULL; - _cleanup_closedir_ DIR *extension_release_dir = NULL; - r = chase_and_opendir(image_class_release_info[image_class].release_file_directory, root, CHASE_PREFIX_ROOT, - &extension_release_dir_path, &extension_release_dir); - if (r < 0) - return log_debug_errno(r, "Cannot open %s%s, ignoring: %m", root, image_class_release_info[image_class].release_file_directory); + _cleanup_free_ char *extension_release_dir_path = NULL; + _cleanup_closedir_ DIR *extension_release_dir = NULL; - r = -ENOENT; - FOREACH_DIRENT(de, extension_release_dir, return -errno) { - int k; + r = chase_and_opendir(image_class_release_info[image_class].release_file_directory, root, CHASE_PREFIX_ROOT, + &extension_release_dir_path, &extension_release_dir); + if (r < 0) + return log_debug_errno(r, "Cannot open %s%s, ignoring: %m", root, image_class_release_info[image_class].release_file_directory); - if (!IN_SET(de->d_type, DT_REG, DT_UNKNOWN)) + r = -ENOENT; + FOREACH_DIRENT(de, extension_release_dir, return -errno) { + int k; + + if (!IN_SET(de->d_type, DT_REG, DT_UNKNOWN)) + continue; + + const char *image_name = startswith(de->d_name, "extension-release."); + if (!image_name) + continue; + + if (!image_name_is_valid(image_name)) { + log_debug("%s/%s is not a valid release file name, ignoring.", + extension_release_dir_path, de->d_name); + continue; + } + + /* We already chased the directory, and checked that this is a real file, so we shouldn't + * fail to open it. */ + _cleanup_close_ int extension_release_fd = openat(dirfd(extension_release_dir), + de->d_name, + O_PATH|O_CLOEXEC|O_NOFOLLOW); + if (extension_release_fd < 0) + return log_debug_errno(errno, + "Failed to open release file %s/%s: %m", + extension_release_dir_path, + de->d_name); + + /* Really ensure it is a regular file after we open it. */ + if (fd_verify_regular(extension_release_fd) < 0) { + log_debug("%s/%s is not a regular file, ignoring.", extension_release_dir_path, de->d_name); + continue; + } + + if (!relax_extension_release_check) { + k = extension_release_strict_xattr_value(extension_release_fd, + extension_release_dir_path, + de->d_name); + if (k != 0) continue; + } - const char *image_name = startswith(de->d_name, "extension-release."); - if (!image_name) - continue; + /* We already found what we were looking for, but there's another candidate? We treat this as + * an error, as we want to enforce that there are no ambiguities in case we are in the + * fallback path. */ + if (r == 0) { + r = -ENOTUNIQ; + break; + } - if (!image_name_is_valid(image_name)) { - log_debug("%s/%s is not a valid release file name, ignoring.", - extension_release_dir_path, de->d_name); - continue; - } + r = 0; /* Found it! */ - /* We already chased the directory, and checked that - * this is a real file, so we shouldn't fail to open it. */ - _cleanup_close_ int extension_release_fd = openat(dirfd(extension_release_dir), - de->d_name, - O_PATH|O_CLOEXEC|O_NOFOLLOW); - if (extension_release_fd < 0) - return log_debug_errno(errno, - "Failed to open release file %s/%s: %m", - extension_release_dir_path, - de->d_name); + if (ret_fd) + fd = TAKE_FD(extension_release_fd); - /* Really ensure it is a regular file after we open it. */ - if (fd_verify_regular(extension_release_fd) < 0) { - log_debug("%s/%s is not a regular file, ignoring.", extension_release_dir_path, de->d_name); - continue; - } - - if (!relax_extension_release_check) { - k = extension_release_strict_xattr_value(extension_release_fd, - extension_release_dir_path, - de->d_name); - if (k != 0) - continue; - } - - /* We already found what we were looking for, but there's another candidate? - * We treat this as an error, as we want to enforce that there are no ambiguities - * in case we are in the fallback path. */ - if (r == 0) { - r = -ENOTUNIQ; - break; - } - - r = 0; /* Found it! */ - - if (ret_fd) - fd = TAKE_FD(extension_release_fd); - - if (ret_path) { - q = path_join(extension_release_dir_path, de->d_name); - if (!q) - return -ENOMEM; - } + if (ret_path) { + q = path_join(extension_release_dir_path, de->d_name); + if (!q) + return -ENOMEM; } } if (r < 0) From 7421f20c7e32d55cd9b21db0b481911b2eaf8ccb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 9 Apr 2023 01:16:26 +0900 Subject: [PATCH 06/13] os-util: return earlier when unsupported image class is specified --- src/basic/os-util.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/basic/os-util.c b/src/basic/os-util.c index 619a2bae96..35c7555461 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -156,6 +156,9 @@ int open_extension_release( if (!extension) return open_os_release(root, ret_path, ret_fd); + if (!IN_SET(image_class, IMAGE_SYSEXT, IMAGE_CONFEXT)) + return -EINVAL; + const char *extension_full_path; if (!image_name_is_valid(extension)) From c9d64f8a2cc743d8309bb1b2246f49f834437a06 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 9 Apr 2023 01:31:29 +0900 Subject: [PATCH 07/13] os-util: do not use 'r' for storing loop status The variable 'r' is usually used for storing return value of functional call. Let's introduce another boolean to store the current loop status. No functional change, just refactoring. --- src/basic/os-util.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/basic/os-util.c b/src/basic/os-util.c index 35c7555461..0ca307064e 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -149,6 +149,7 @@ int open_extension_release( _cleanup_close_ int fd = -EBADF; _cleanup_free_ char *q = NULL; + bool found = false; int r; assert(!extension || (image_class >= 0 && image_class < _IMAGE_CLASS_MAX)); @@ -183,7 +184,6 @@ int open_extension_release( if (r < 0) return log_debug_errno(r, "Cannot open %s%s, ignoring: %m", root, image_class_release_info[image_class].release_file_directory); - r = -ENOENT; FOREACH_DIRENT(de, extension_release_dir, return -errno) { int k; @@ -228,12 +228,10 @@ int open_extension_release( /* We already found what we were looking for, but there's another candidate? We treat this as * an error, as we want to enforce that there are no ambiguities in case we are in the * fallback path. */ - if (r == 0) { - r = -ENOTUNIQ; - break; - } + if (found) + return -ENOTUNIQ; - r = 0; /* Found it! */ + found = true; if (ret_fd) fd = TAKE_FD(extension_release_fd); @@ -244,8 +242,8 @@ int open_extension_release( return -ENOMEM; } } - if (r < 0) - return r; + if (!found) + return -ENOENT; if (ret_fd) *ret_fd = TAKE_FD(fd); From 59c4707594d2c6ec4f8496175fcf9c45ef6d54f3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 9 Apr 2023 01:37:16 +0900 Subject: [PATCH 08/13] os-util: log one more error cause --- src/basic/os-util.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/basic/os-util.c b/src/basic/os-util.c index 0ca307064e..f7b26e6f82 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -212,8 +212,9 @@ int open_extension_release( de->d_name); /* Really ensure it is a regular file after we open it. */ - if (fd_verify_regular(extension_release_fd) < 0) { - log_debug("%s/%s is not a regular file, ignoring.", extension_release_dir_path, de->d_name); + r = fd_verify_regular(extension_release_fd); + if (r < 0) { + log_debug_errno(r, "%s/%s is not a regular file, ignoring: %m", extension_release_dir_path, de->d_name); continue; } From 61acfd8311bee7deadad1c41397ab32846575a3a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 9 Apr 2023 01:38:34 +0900 Subject: [PATCH 09/13] os-util: shorten temporal variable names No functional change, just refactoring. --- src/basic/os-util.c | 68 +++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 40 deletions(-) diff --git a/src/basic/os-util.c b/src/basic/os-util.c index f7b26e6f82..ff31a48d3e 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -147,9 +147,11 @@ int open_extension_release( char **ret_path, int *ret_fd) { - _cleanup_close_ int fd = -EBADF; - _cleanup_free_ char *q = NULL; + _cleanup_free_ char *dir_path = NULL, *path_found = NULL; + _cleanup_close_ int fd_found = -EBADF; + _cleanup_closedir_ DIR *dir = NULL; bool found = false; + const char *p; int r; assert(!extension || (image_class >= 0 && image_class < _IMAGE_CLASS_MAX)); @@ -160,14 +162,12 @@ int open_extension_release( if (!IN_SET(image_class, IMAGE_SYSEXT, IMAGE_CONFEXT)) return -EINVAL; - const char *extension_full_path; - if (!image_name_is_valid(extension)) return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "The extension name %s is invalid.", extension); - extension_full_path = strjoina(image_class_release_info[image_class].release_file_path_prefix, extension); - r = chase(extension_full_path, root, CHASE_PREFIX_ROOT, ret_path, ret_fd); - log_full_errno_zerook(LOG_DEBUG, MIN(r, 0), "Checking for %s: %m", extension_full_path); + p = strjoina(image_class_release_info[image_class].release_file_path_prefix, extension); + r = chase(p, root, CHASE_PREFIX_ROOT, ret_path, ret_fd); + log_full_errno_zerook(LOG_DEBUG, MIN(r, 0), "Checking for %s: %m", p); if (r != -ENOENT) return r; @@ -176,55 +176,43 @@ int open_extension_release( * the first one with a user.extension-release xattr instead. The user.extension-release.strict * xattr is checked to ensure the author of the image considers it OK if names do not match. */ - _cleanup_free_ char *extension_release_dir_path = NULL; - _cleanup_closedir_ DIR *extension_release_dir = NULL; - - r = chase_and_opendir(image_class_release_info[image_class].release_file_directory, root, CHASE_PREFIX_ROOT, - &extension_release_dir_path, &extension_release_dir); + p = image_class_release_info[image_class].release_file_directory; + r = chase_and_opendir(p, root, CHASE_PREFIX_ROOT, &dir_path, &dir); if (r < 0) - return log_debug_errno(r, "Cannot open %s%s, ignoring: %m", root, image_class_release_info[image_class].release_file_directory); + return log_debug_errno(r, "Cannot open %s%s, ignoring: %m", root, p); - FOREACH_DIRENT(de, extension_release_dir, return -errno) { - int k; + FOREACH_DIRENT(de, dir, return -errno) { + _cleanup_close_ int fd = -EBADF; + const char *image_name; if (!IN_SET(de->d_type, DT_REG, DT_UNKNOWN)) continue; - const char *image_name = startswith(de->d_name, "extension-release."); + image_name = startswith(de->d_name, "extension-release."); if (!image_name) continue; if (!image_name_is_valid(image_name)) { - log_debug("%s/%s is not a valid release file name, ignoring.", - extension_release_dir_path, de->d_name); + log_debug("%s/%s is not a valid release file name, ignoring.", dir_path, de->d_name); continue; } /* We already chased the directory, and checked that this is a real file, so we shouldn't * fail to open it. */ - _cleanup_close_ int extension_release_fd = openat(dirfd(extension_release_dir), - de->d_name, - O_PATH|O_CLOEXEC|O_NOFOLLOW); - if (extension_release_fd < 0) - return log_debug_errno(errno, - "Failed to open release file %s/%s: %m", - extension_release_dir_path, - de->d_name); + fd = openat(dirfd(dir), de->d_name, O_PATH|O_CLOEXEC|O_NOFOLLOW); + if (fd < 0) + return log_debug_errno(errno, "Failed to open release file %s/%s: %m", dir_path, de->d_name); /* Really ensure it is a regular file after we open it. */ - r = fd_verify_regular(extension_release_fd); + r = fd_verify_regular(fd); if (r < 0) { - log_debug_errno(r, "%s/%s is not a regular file, ignoring: %m", extension_release_dir_path, de->d_name); + log_debug_errno(r, "%s/%s is not a regular file, ignoring: %m", dir_path, de->d_name); continue; } - if (!relax_extension_release_check) { - k = extension_release_strict_xattr_value(extension_release_fd, - extension_release_dir_path, - de->d_name); - if (k != 0) - continue; - } + if (!relax_extension_release_check && + extension_release_strict_xattr_value(fd, dir_path, de->d_name) != 0) + continue; /* We already found what we were looking for, but there's another candidate? We treat this as * an error, as we want to enforce that there are no ambiguities in case we are in the @@ -235,11 +223,11 @@ int open_extension_release( found = true; if (ret_fd) - fd = TAKE_FD(extension_release_fd); + fd_found = TAKE_FD(fd); if (ret_path) { - q = path_join(extension_release_dir_path, de->d_name); - if (!q) + path_found = path_join(dir_path, de->d_name); + if (!path_found) return -ENOMEM; } } @@ -247,9 +235,9 @@ int open_extension_release( return -ENOENT; if (ret_fd) - *ret_fd = TAKE_FD(fd); + *ret_fd = TAKE_FD(fd_found); if (ret_path) - *ret_path = TAKE_PTR(q); + *ret_path = TAKE_PTR(path_found); return 0; } From 7ef43c78dfb2abf3f3b98b325c10f2ca347f4c72 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 8 Apr 2023 22:10:25 +0900 Subject: [PATCH 10/13] os-util: invert order of arguments in extension release parser For consistency with other functions. Unfortunately, va_start() requires that the previous argument is a pointer, hence the order of the arguments in the internal function cannot be changed. --- src/basic/os-util.h | 2 +- src/test/test-os-util.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/basic/os-util.h b/src/basic/os-util.h index bd9e8949e6..e6acf21195 100644 --- a/src/basic/os-util.h +++ b/src/basic/os-util.h @@ -33,7 +33,7 @@ int open_os_release(const char *root, char **ret_path, int *ret_fd); int _parse_extension_release(const char *root, ImageClass image_class, bool relax_extension_release_check, const char *extension, ...) _sentinel_; int _parse_os_release(const char *root, ...) _sentinel_; -#define parse_extension_release(root, image_class, relax_extension_release_check, extension, ...) _parse_extension_release(root, image_class, relax_extension_release_check, extension, __VA_ARGS__, NULL) +#define parse_extension_release(root, image_class, extension, relax_extension_release_check, ...) _parse_extension_release(root, image_class, relax_extension_release_check, extension, __VA_ARGS__, NULL) #define parse_os_release(root, ...) _parse_os_release(root, __VA_ARGS__, NULL) int load_extension_release_pairs(const char *root, ImageClass image_class, const char *extension, bool relax_extension_release_check, char ***ret); diff --git a/src/test/test-os-util.c b/src/test/test-os-util.c index 94680389fd..84e55e1754 100644 --- a/src/test/test-os-util.c +++ b/src/test/test-os-util.c @@ -75,7 +75,7 @@ TEST(parse_extension_release) { if (r < 0) log_error_errno(r, "Failed to write file: %m"); - assert_se(parse_extension_release(tempdir, IMAGE_SYSEXT, false, "test", "ID", &id, "VERSION_ID", &version_id) == 0); + assert_se(parse_extension_release(tempdir, IMAGE_SYSEXT, "test", false, "ID", &id, "VERSION_ID", &version_id) == 0); log_info("ID: %s VERSION_ID: %s", id, version_id); assert_se(streq(id, "the-id")); assert_se(streq(version_id, "the-version-id")); @@ -87,16 +87,16 @@ TEST(parse_extension_release) { if (r < 0) log_error_errno(r, "Failed to write file: %m"); - assert_se(parse_extension_release(tempdir, IMAGE_CONFEXT, false, "tester", "ID", &id, "VERSION_ID", &version_id) == 0); + assert_se(parse_extension_release(tempdir, IMAGE_CONFEXT, "tester", false, "ID", &id, "VERSION_ID", &version_id) == 0); log_info("ID: %s VERSION_ID: %s", id, version_id); assert_se(streq(id, "the-id")); assert_se(streq(version_id, "the-version-id")); - assert_se(parse_extension_release(tempdir, IMAGE_CONFEXT, false, "tester", "FOOBAR", &foobar) == 0); + assert_se(parse_extension_release(tempdir, IMAGE_CONFEXT, "tester", false, "FOOBAR", &foobar) == 0); log_info("FOOBAR: %s", strnull(foobar)); assert_se(foobar == NULL); - assert_se(parse_extension_release(tempdir, IMAGE_SYSEXT, false, "test", "FOOBAR", &foobar) == 0); + assert_se(parse_extension_release(tempdir, IMAGE_SYSEXT, "test", false, "FOOBAR", &foobar) == 0); log_info("FOOBAR: %s", strnull(foobar)); assert_se(foobar == NULL); } From f4a1d32c82489d2b734a3c266aa47ba697100312 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 8 Apr 2023 18:16:04 +0900 Subject: [PATCH 11/13] os-util: merge parse_{extension,os}_release() --- src/basic/os-util.c | 33 +++++++++------------------------ src/basic/os-util.h | 9 +++++---- 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/src/basic/os-util.c b/src/basic/os-util.c index ff31a48d3e..16fe123ce6 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -242,40 +242,25 @@ int open_extension_release( return 0; } -static int parse_release_internal(const char *root, ImageClass image_class, bool relax_extension_release_check, const char *extension, va_list ap) { +int parse_extension_release_sentinel( + const char *root, + ImageClass image_class, + bool relax_extension_release_check, + const char *extension, + ...) { + _cleanup_close_ int fd = -EBADF; _cleanup_free_ char *p = NULL; + va_list ap; int r; r = open_extension_release(root, image_class, extension, relax_extension_release_check, &p, &fd); if (r < 0) return r; - return parse_env_file_fdv(fd, p, ap); -} - -int _parse_extension_release(const char *root, ImageClass image_class, bool relax_extension_release_check, const char *extension, ...) { - va_list ap; - int r; - - assert(image_class >= 0); - assert(image_class < _IMAGE_CLASS_MAX); - va_start(ap, extension); - r = parse_release_internal(root, image_class, relax_extension_release_check, extension, ap); + r = parse_env_file_fdv(fd, p, ap); va_end(ap); - - return r; -} - -int _parse_os_release(const char *root, ...) { - va_list ap; - int r; - - va_start(ap, root); - r = parse_release_internal(root, _IMAGE_CLASS_INVALID, /* relax_extension_release_check= */ false, NULL, ap); - va_end(ap); - return r; } diff --git a/src/basic/os-util.h b/src/basic/os-util.h index e6acf21195..243d3c58ea 100644 --- a/src/basic/os-util.h +++ b/src/basic/os-util.h @@ -31,10 +31,11 @@ static inline int path_is_os_tree(const char *path) { int open_extension_release(const char *root, ImageClass image_class, const char *extension, bool relax_extension_release_check, char **ret_path, int *ret_fd); int open_os_release(const char *root, char **ret_path, int *ret_fd); -int _parse_extension_release(const char *root, ImageClass image_class, bool relax_extension_release_check, const char *extension, ...) _sentinel_; -int _parse_os_release(const char *root, ...) _sentinel_; -#define parse_extension_release(root, image_class, extension, relax_extension_release_check, ...) _parse_extension_release(root, image_class, relax_extension_release_check, extension, __VA_ARGS__, NULL) -#define parse_os_release(root, ...) _parse_os_release(root, __VA_ARGS__, NULL) +int parse_extension_release_sentinel(const char *root, ImageClass image_class, bool relax_extension_release_check, const char *extension, ...) _sentinel_; +#define parse_extension_release(root, image_class, extension, relax_extension_release_check, ...) \ + parse_extension_release_sentinel(root, image_class, relax_extension_release_check, extension, __VA_ARGS__, NULL) +#define parse_os_release(root, ...) \ + parse_extension_release_sentinel(root, _IMAGE_CLASS_INVALID, false, NULL, __VA_ARGS__, NULL) int load_extension_release_pairs(const char *root, ImageClass image_class, const char *extension, bool relax_extension_release_check, char ***ret); static inline int load_os_release_pairs(const char *root, char ***ret) { From 5cf69e709eb928662f03944ac4064157784b0d76 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 8 Apr 2023 22:33:40 +0900 Subject: [PATCH 12/13] os-util: make $SYSTEMD_OS_RELEASE prefixed with the root directory To make it consistent with other env vars, e.g. $SYSTEMD_ESP_PATH or $SYSTEMD_XBOOTLDR_PATH. This is useful when the root is specified by a file descriptor, instead of a path. --- docs/ENVIRONMENT.md | 3 +-- src/basic/os-util.c | 2 +- test/test-systemctl-enable.sh | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/ENVIRONMENT.md b/docs/ENVIRONMENT.md index 292a0f1e0e..445131b479 100644 --- a/docs/ENVIRONMENT.md +++ b/docs/ENVIRONMENT.md @@ -45,8 +45,7 @@ All tools: * `$SYSTEMD_OS_RELEASE` — if set, use this path instead of `/etc/os-release` or `/usr/lib/os-release`. When operating under some root (e.g. `systemctl - --root=…`), the path is taken relative to the outside root. Only useful for - debugging. + --root=…`), the path is prefixed with the root. Only useful for debugging. * `$SYSTEMD_FSTAB` — if set, use this path instead of `/etc/fstab`. Only useful for debugging. diff --git a/src/basic/os-util.c b/src/basic/os-util.c index 16fe123ce6..0f123aaf82 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -128,7 +128,7 @@ int open_os_release(const char *root, char **ret_path, int *ret_fd) { e = secure_getenv("SYSTEMD_OS_RELEASE"); if (e) - return chase(e, root, 0, ret_path, ret_fd); + return chase(e, root, CHASE_PREFIX_ROOT, ret_path, ret_fd); FOREACH_STRING(path, "/etc/os-release", "/usr/lib/os-release") { r = chase(path, root, CHASE_PREFIX_ROOT, ret_path, ret_fd); diff --git a/test/test-systemctl-enable.sh b/test/test-systemctl-enable.sh index 7d5667f297..e22a3ef628 100644 --- a/test/test-systemctl-enable.sh +++ b/test/test-systemctl-enable.sh @@ -695,4 +695,4 @@ cat >"$root/etc/os-release2" < Date: Sat, 8 Apr 2023 18:48:57 +0900 Subject: [PATCH 13/13] os-util: introduce several _at() variants of os-release parsers --- src/basic/os-util.c | 128 ++++++++++++++++++++++++++++++++++++++------ src/basic/os-util.h | 8 +++ 2 files changed, 121 insertions(+), 15 deletions(-) diff --git a/src/basic/os-util.c b/src/basic/os-util.c index 0f123aaf82..dd8faf2376 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -122,16 +122,18 @@ static int extension_release_strict_xattr_value(int extension_release_fd, const return false; } -int open_os_release(const char *root, char **ret_path, int *ret_fd) { +int open_os_release_at(int rfd, char **ret_path, int *ret_fd) { const char *e; int r; + assert(rfd >= 0 || rfd == AT_FDCWD); + e = secure_getenv("SYSTEMD_OS_RELEASE"); if (e) - return chase(e, root, CHASE_PREFIX_ROOT, ret_path, ret_fd); + return chaseat(rfd, e, CHASE_AT_RESOLVE_IN_ROOT, ret_path, ret_fd); FOREACH_STRING(path, "/etc/os-release", "/usr/lib/os-release") { - r = chase(path, root, CHASE_PREFIX_ROOT, ret_path, ret_fd); + r = chaseat(rfd, path, CHASE_AT_RESOLVE_IN_ROOT, ret_path, ret_fd); if (r != -ENOENT) return r; } @@ -139,8 +141,33 @@ int open_os_release(const char *root, char **ret_path, int *ret_fd) { return -ENOENT; } -int open_extension_release( - const char *root, +int open_os_release(const char *root, char **ret_path, int *ret_fd) { + _cleanup_close_ int rfd = -EBADF, fd = -EBADF; + _cleanup_free_ char *p = NULL; + int r; + + rfd = open(empty_to_root(root), O_CLOEXEC | O_DIRECTORY | O_PATH); + if (rfd < 0) + return -errno; + + r = open_os_release_at(rfd, ret_path ? &p : NULL, ret_fd ? &fd : NULL); + if (r < 0) + return r; + + if (ret_path) { + r = path_prefix_root_cwd(p, root, ret_path); + if (r < 0) + return r; + } + + if (ret_fd) + *ret_fd = TAKE_FD(fd); + + return 0; +} + +int open_extension_release_at( + int rfd, ImageClass image_class, const char *extension, bool relax_extension_release_check, @@ -154,10 +181,11 @@ int open_extension_release( const char *p; int r; + assert(rfd >= 0 || rfd == AT_FDCWD); assert(!extension || (image_class >= 0 && image_class < _IMAGE_CLASS_MAX)); if (!extension) - return open_os_release(root, ret_path, ret_fd); + return open_os_release_at(rfd, ret_path, ret_fd); if (!IN_SET(image_class, IMAGE_SYSEXT, IMAGE_CONFEXT)) return -EINVAL; @@ -166,7 +194,7 @@ int open_extension_release( return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "The extension name %s is invalid.", extension); p = strjoina(image_class_release_info[image_class].release_file_path_prefix, extension); - r = chase(p, root, CHASE_PREFIX_ROOT, ret_path, ret_fd); + r = chaseat(rfd, p, CHASE_AT_RESOLVE_IN_ROOT, ret_path, ret_fd); log_full_errno_zerook(LOG_DEBUG, MIN(r, 0), "Checking for %s: %m", p); if (r != -ENOENT) return r; @@ -177,9 +205,9 @@ int open_extension_release( * xattr is checked to ensure the author of the image considers it OK if names do not match. */ p = image_class_release_info[image_class].release_file_directory; - r = chase_and_opendir(p, root, CHASE_PREFIX_ROOT, &dir_path, &dir); + r = chase_and_opendirat(rfd, p, CHASE_AT_RESOLVE_IN_ROOT, &dir_path, &dir); if (r < 0) - return log_debug_errno(r, "Cannot open %s%s, ignoring: %m", root, p); + return log_debug_errno(r, "Cannot open %s, ignoring: %m", p); FOREACH_DIRENT(de, dir, return -errno) { _cleanup_close_ int fd = -EBADF; @@ -242,6 +270,77 @@ int open_extension_release( return 0; } +int open_extension_release( + const char *root, + ImageClass image_class, + const char *extension, + bool relax_extension_release_check, + char **ret_path, + int *ret_fd) { + + _cleanup_close_ int rfd = -EBADF, fd = -EBADF; + _cleanup_free_ char *p = NULL; + int r; + + rfd = open(empty_to_root(root), O_CLOEXEC | O_DIRECTORY | O_PATH); + if (rfd < 0) + return -errno; + + r = open_extension_release_at(rfd, image_class, extension, relax_extension_release_check, + ret_path ? &p : NULL, ret_fd ? &fd : NULL); + if (r < 0) + return r; + + if (ret_path) { + r = path_prefix_root_cwd(p, root, ret_path); + if (r < 0) + return r; + } + + if (ret_fd) + *ret_fd = TAKE_FD(fd); + + return 0; +} + +static int parse_extension_release_atv( + int rfd, + ImageClass image_class, + const char *extension, + bool relax_extension_release_check, + va_list ap) { + + _cleanup_close_ int fd = -EBADF; + _cleanup_free_ char *p = NULL; + int r; + + assert(rfd >= 0 || rfd == AT_FDCWD); + + r = open_extension_release_at(rfd, image_class, extension, relax_extension_release_check, &p, &fd); + if (r < 0) + return r; + + return parse_env_file_fdv(fd, p, ap); +} + +int parse_extension_release_at_sentinel( + int rfd, + ImageClass image_class, + bool relax_extension_release_check, + const char *extension, + ...) { + + va_list ap; + int r; + + assert(rfd >= 0 || rfd == AT_FDCWD); + + va_start(ap, extension); + r = parse_extension_release_atv(rfd, image_class, extension, relax_extension_release_check, ap); + va_end(ap); + return r; +} + int parse_extension_release_sentinel( const char *root, ImageClass image_class, @@ -249,17 +348,16 @@ int parse_extension_release_sentinel( const char *extension, ...) { - _cleanup_close_ int fd = -EBADF; - _cleanup_free_ char *p = NULL; + _cleanup_close_ int rfd = -EBADF; va_list ap; int r; - r = open_extension_release(root, image_class, extension, relax_extension_release_check, &p, &fd); - if (r < 0) - return r; + rfd = open(empty_to_root(root), O_CLOEXEC | O_DIRECTORY | O_PATH); + if (rfd < 0) + return -errno; va_start(ap, extension); - r = parse_env_file_fdv(fd, p, ap); + r = parse_extension_release_atv(rfd, image_class, extension, relax_extension_release_check, ap); va_end(ap); return r; } diff --git a/src/basic/os-util.h b/src/basic/os-util.h index 243d3c58ea..480f71e614 100644 --- a/src/basic/os-util.h +++ b/src/basic/os-util.h @@ -29,7 +29,9 @@ static inline int path_is_os_tree(const char *path) { } int open_extension_release(const char *root, ImageClass image_class, const char *extension, bool relax_extension_release_check, char **ret_path, int *ret_fd); +int open_extension_release_at(int rfd, ImageClass image_class, const char *extension, bool relax_extension_release_check, char **ret_path, int *ret_fd); int open_os_release(const char *root, char **ret_path, int *ret_fd); +int open_os_release_at(int rfd, char **ret_path, int *ret_fd); int parse_extension_release_sentinel(const char *root, ImageClass image_class, bool relax_extension_release_check, const char *extension, ...) _sentinel_; #define parse_extension_release(root, image_class, extension, relax_extension_release_check, ...) \ @@ -37,6 +39,12 @@ int parse_extension_release_sentinel(const char *root, ImageClass image_class, b #define parse_os_release(root, ...) \ parse_extension_release_sentinel(root, _IMAGE_CLASS_INVALID, false, NULL, __VA_ARGS__, NULL) +int parse_extension_release_at_sentinel(int rfd, ImageClass image_class, bool relax_extension_release_check, const char *extension, ...) _sentinel_; +#define parse_extension_release_at(rfd, image_class, extension, relax_extension_release_check, ...) \ + parse_extension_release_at_sentinel(rfd, image_class, relax_extension_release_check, extension, __VA_ARGS__, NULL) +#define parse_os_release_at(rfd, ...) \ + parse_extension_release_at_sentinel(rfd, _IMAGE_CLASS_INVALID, false, NULL, __VA_ARGS__, NULL) + int load_extension_release_pairs(const char *root, ImageClass image_class, const char *extension, bool relax_extension_release_check, char ***ret); static inline int load_os_release_pairs(const char *root, char ***ret) { return load_extension_release_pairs(root, _IMAGE_CLASS_INVALID, NULL, false, ret);