From 6957023208a1c3d5028dbd064d447032689072a2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Aug 2022 11:34:40 +0200 Subject: [PATCH 1/6] chase-symlinks: honour CHASE_NOFOLLOW flag properly in some cases --- src/basic/chase-symlinks.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/basic/chase-symlinks.c b/src/basic/chase-symlinks.c index e93419d635..ab80e76688 100644 --- a/src/basic/chase-symlinks.c +++ b/src/basic/chase-symlinks.c @@ -422,7 +422,7 @@ int chase_symlinks_and_open( if (empty_or_root(root) && !ret_path && (chase_flags & (CHASE_NO_AUTOFS|CHASE_SAFE)) == 0) { /* Shortcut this call if none of the special features of this call are requested */ - r = open(path, open_flags); + r = open(path, open_flags | (FLAGS_SET(chase_flags, CHASE_NOFOLLOW) ? O_NOFOLLOW : 0)); if (r < 0) return -errno; @@ -507,7 +507,8 @@ int chase_symlinks_and_stat( if (empty_or_root(root) && !ret_path && (chase_flags & (CHASE_NO_AUTOFS|CHASE_SAFE)) == 0) { /* Shortcut this call if none of the special features of this call are requested */ - if (stat(path, ret_stat) < 0) + + if (fstatat(AT_FDCWD, path, ret_stat, FLAGS_SET(chase_flags, CHASE_NOFOLLOW) ? AT_SYMLINK_NOFOLLOW : 0) < 0) return -errno; return 1; From 37b9bc56fc8a1baecce68a8809034d6e387fd706 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Aug 2022 11:35:43 +0200 Subject: [PATCH 2/6] chase-symlinks: fix shortcut condition --- src/basic/chase-symlinks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/chase-symlinks.c b/src/basic/chase-symlinks.c index ab80e76688..2561da0e8c 100644 --- a/src/basic/chase-symlinks.c +++ b/src/basic/chase-symlinks.c @@ -505,7 +505,7 @@ int chase_symlinks_and_stat( if (chase_flags & CHASE_NONEXISTENT) return -EINVAL; - if (empty_or_root(root) && !ret_path && (chase_flags & (CHASE_NO_AUTOFS|CHASE_SAFE)) == 0) { + if (empty_or_root(root) && !ret_path && (chase_flags & (CHASE_NO_AUTOFS|CHASE_SAFE)) == 0 && !ret_fd) { /* Shortcut this call if none of the special features of this call are requested */ if (fstatat(AT_FDCWD, path, ret_stat, FLAGS_SET(chase_flags, CHASE_NOFOLLOW) ? AT_SYMLINK_NOFOLLOW : 0) < 0) From 2b2caea21df4f5c2b91888c037b9bc04da40dec3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Aug 2022 11:36:12 +0200 Subject: [PATCH 3/6] chase-symlinks: add chase_symlinks_and_access() helper This is similar to chase_symlinks_and_access() but wraps access() rather than stat() --- src/basic/chase-symlinks.c | 43 ++++++++++++++++++++++++++++++++++++++ src/basic/chase-symlinks.h | 2 +- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/basic/chase-symlinks.c b/src/basic/chase-symlinks.c index 2561da0e8c..a5d89e1349 100644 --- a/src/basic/chase-symlinks.c +++ b/src/basic/chase-symlinks.c @@ -530,6 +530,49 @@ int chase_symlinks_and_stat( return 1; } +int chase_symlinks_and_access( + const char *path, + const char *root, + ChaseSymlinksFlags chase_flags, + int access_mode, + char **ret_path, + int *ret_fd) { + + _cleanup_close_ int path_fd = -1; + _cleanup_free_ char *p = NULL; + int r; + + assert(path); + + if (chase_flags & (CHASE_NONEXISTENT|CHASE_STEP)) + return -EINVAL; + + if (empty_or_root(root) && !ret_path && (chase_flags & (CHASE_NO_AUTOFS|CHASE_SAFE)) == 0 && !ret_fd) { + /* Shortcut this call if none of the special features of this call are requested */ + + if (faccessat(AT_FDCWD, path, access_mode, FLAGS_SET(chase_flags, CHASE_NOFOLLOW) ? AT_SYMLINK_NOFOLLOW : 0) < 0) + return -errno; + + return 1; + } + + r = chase_symlinks(path, root, chase_flags, ret_path ? &p : NULL, &path_fd); + if (r < 0) + return r; + assert(path_fd >= 0); + + r = access_fd(path_fd, access_mode); + if (r < 0) + return r; + + if (ret_path) + *ret_path = TAKE_PTR(p); + if (ret_fd) + *ret_fd = TAKE_FD(path_fd); + + return 1; +} + int chase_symlinks_and_fopen_unlocked( const char *path, const char *root, diff --git a/src/basic/chase-symlinks.h b/src/basic/chase-symlinks.h index 491138a698..7e45b0cbab 100644 --- a/src/basic/chase-symlinks.h +++ b/src/basic/chase-symlinks.h @@ -28,5 +28,5 @@ int chase_symlinks(const char *path_with_prefix, const char *root, ChaseSymlinks int chase_symlinks_and_open(const char *path, const char *root, ChaseSymlinksFlags chase_flags, int open_flags, char **ret_path); int chase_symlinks_and_opendir(const char *path, const char *root, ChaseSymlinksFlags chase_flags, char **ret_path, DIR **ret_dir); int chase_symlinks_and_stat(const char *path, const char *root, ChaseSymlinksFlags chase_flags, char **ret_path, struct stat *ret_stat, int *ret_fd); - +int chase_symlinks_and_access(const char *path, const char *root, ChaseSymlinksFlags chase_flags, int access_mode, char **ret_path, int *ret_fd); int chase_symlinks_and_fopen_unlocked(const char *path, const char *root, ChaseSymlinksFlags chase_flags, const char *open_flags, char **ret_path, FILE **ret_file); From 3730dc5d5b4b7c4e1e7d0957c88568cc45de2390 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Aug 2022 14:06:49 +0200 Subject: [PATCH 4/6] bootctl: port some more code over to chase_symlinks() from prefix_roota() This is far from complete, but let's start moving this way. --- src/boot/bootctl.c | 56 +++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index ddeeed0c3d..33bde8b30e 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -476,7 +476,7 @@ static int enumerate_binaries( bool *is_first) { _cleanup_closedir_ DIR *d = NULL; - const char *p; + _cleanup_free_ char *p = NULL; int c = 0, r; assert(esp_path); @@ -484,14 +484,11 @@ static int enumerate_binaries( assert(previous); assert(is_first); - p = prefix_roota(esp_path, path); - d = opendir(p); - if (!d) { - if (errno == ENOENT) - return 0; - - return log_error_errno(errno, "Failed to read \"%s\": %m", p); - } + r = chase_symlinks_and_opendir(path, esp_path, CHASE_PREFIX_ROOT, &p, &d); + if (r == -ENOENT) + return 0; + if (r < 0) + return log_error_errno(r, "Failed to read \"%s/%s\": %m", esp_path, path); FOREACH_DIRENT(de, d, break) { _cleanup_free_ char *v = NULL; @@ -1116,11 +1113,15 @@ static int remove_from_order(uint16_t slot) { return 0; } -static int install_variables(const char *esp_path, - uint32_t part, uint64_t pstart, uint64_t psize, - sd_id128_t uuid, const char *path, - bool first) { - const char *p; +static int install_variables( + const char *esp_path, + uint32_t part, + uint64_t pstart, + uint64_t psize, + sd_id128_t uuid, + const char *path, + bool first) { + uint16_t slot; int r; @@ -1135,13 +1136,11 @@ static int install_variables(const char *esp_path, return 0; } - p = prefix_roota(esp_path, path); - if (access(p, F_OK) < 0) { - if (errno == ENOENT) - return 0; - - return log_error_errno(errno, "Cannot access \"%s\": %m", p); - } + r = chase_symlinks_and_access(path, esp_path, CHASE_PREFIX_ROOT, F_OK, NULL, NULL); + if (r == -ENOENT) + return 0; + if (r < 0) + return log_error_errno(r, "Cannot access \"%s/%s\": %m", esp_path, path); r = find_slot(uuid, path, &slot); if (r < 0) @@ -1165,17 +1164,14 @@ static int install_variables(const char *esp_path, static int remove_boot_efi(const char *esp_path) { _cleanup_closedir_ DIR *d = NULL; - const char *p; + _cleanup_free_ char *p = NULL; int r, c = 0; - p = prefix_roota(esp_path, "/EFI/BOOT"); - d = opendir(p); - if (!d) { - if (errno == ENOENT) - return 0; - - return log_error_errno(errno, "Failed to open directory \"%s\": %m", p); - } + r = chase_symlinks_and_opendir("/EFI/BOOT", esp_path, CHASE_PREFIX_ROOT, &p, &d); + if (r == -ENOENT) + return 0; + if (r < 0) + return log_error_errno(r, "Failed to open directory \"%s/EFI/BOOT\": %m", esp_path); FOREACH_DIRENT(de, d, break) { _cleanup_close_ int fd = -1; From 3979ea86443e6fd7c0657ce5eadf196b3cb67182 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Aug 2022 14:06:10 +0200 Subject: [PATCH 5/6] hwdb-test: don't rely on --root= quirk Previously, the test would rely on the fact that systemd-hwdb would follow symlinks outside of the --root= hierarchy. That's a bug however, and systemd-hwdb shouldn't do that. Hence let's remove the fact that the test relies on it, so that we can then fix systemd-hwdb (specifically: conf_files_list()) accordingly. --- test/hwdb-test.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/hwdb-test.sh b/test/hwdb-test.sh index 29183e6829..3efbad21b6 100755 --- a/test/hwdb-test.sh +++ b/test/hwdb-test.sh @@ -22,7 +22,7 @@ D="$(mktemp --tmpdir --directory "hwdb-test.XXXXXXXXXX")" # shellcheck disable=SC2064 trap "rm -rf '$D'" EXIT INT QUIT PIPE mkdir -p "$D/etc/udev" -ln -s "$ROOTDIR/hwdb.d" "$D/etc/udev/hwdb.d" +cp -a "$ROOTDIR/hwdb.d" "$D/etc/udev/hwdb.d" # Test "good" properties" — no warnings or errors allowed err=$("$SYSTEMD_HWDB" update --root "$D" 2>&1 >/dev/null) && rc= || rc=$? @@ -41,9 +41,9 @@ if [ ! -e "$D/etc/udev/hwdb.bin" ]; then fi # Test "bad" properties" — warnings required, errors not allowed -rm -f "$D/etc/udev/hwdb.bin" "$D/etc/udev/hwdb.d" +rm -rf "$D/etc/udev/hwdb.bin" "$D/etc/udev/hwdb.d" -ln -s "$ROOTDIR/test/hwdb.d" "$D/etc/udev/hwdb.d" +cp -a "$ROOTDIR/test/hwdb.d" "$D/etc/udev/hwdb.d" err=$("$SYSTEMD_HWDB" update --root "$D" 2>&1 >/dev/null) && rc= || rc=$? if [ -n "$rc" ]; then echo "$SYSTEMD_HWDB returned $rc" From 1d1c226f6c30f5fdbb34b0708621ee58e528d207 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Aug 2022 14:07:27 +0200 Subject: [PATCH 6/6] conf-files: port conf_files_list() over to chase_symlinks() from prefix_roota() --- src/basic/conf-files.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index 82c6dc5677..532c9d19b8 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -5,6 +5,7 @@ #include #include +#include "chase-symlinks.h" #include "conf-files.h" #include "def.h" #include "dirent-util.h" @@ -28,23 +29,19 @@ static int files_add( unsigned flags, const char *path) { + _cleanup_free_ char *dirpath = NULL; _cleanup_closedir_ DIR *dir = NULL; - const char *dirpath; int r; assert(h); assert((flags & CONF_FILES_FILTER_MASKED) == 0 || masked); assert(path); - dirpath = prefix_roota(root, path); - - dir = opendir(dirpath); - if (!dir) { - if (errno == ENOENT) - return 0; - - return log_debug_errno(errno, "Failed to open directory '%s': %m", dirpath); - } + r = chase_symlinks_and_opendir(path, root, CHASE_PREFIX_ROOT, &dirpath, &dir); + if (r == -ENOENT) + return 0; + if (r < 0) + return log_debug_errno(r, "Failed to open directory '%s/%s': %m", empty_or_root(root) ? "" : root, dirpath); FOREACH_DIRENT(de, dir, return -errno) { struct stat st;