diff --git a/src/basic/recurse-dir.c b/src/basic/recurse-dir.c index 8f691d9229..6e0ce5cd4b 100644 --- a/src/basic/recurse-dir.c +++ b/src/basic/recurse-dir.c @@ -27,6 +27,35 @@ static bool ignore_dirent(const struct dirent *de, RecurseDirFlags flags) { /* Depending on flag either ignore everything starting with ".", or just "." itself and ".." */ + if ((flags & _RECURSE_DIR_MUST_BE_MASK) != 0) { + RecurseDirFlags f; + + switch (de->d_type) { + + case DT_DIR: + f = RECURSE_DIR_MUST_BE_DIRECTORY; + break; + + case DT_REG: + f = RECURSE_DIR_MUST_BE_REGULAR; + break; + + case DT_LNK: + f = RECURSE_DIR_MUST_BE_SYMLINK; + break; + + case DT_SOCK: + f = RECURSE_DIR_MUST_BE_SOCKET; + break; + + default: + return true; + } + + if (!FLAGS_SET(flags, f)) + return true; + } + return FLAGS_SET(flags, RECURSE_DIR_IGNORE_DOT) ? de->d_name[0] == '.' : dot_or_dot_dot(de->d_name); @@ -39,6 +68,9 @@ int readdir_all(int dir_fd, RecurseDirFlags flags, DirectoryEntries **ret) { assert(dir_fd >= 0); + if ((flags & _RECURSE_DIR_MUST_BE_MASK) != 0) /* We need the type to validate it */ + flags |= RECURSE_DIR_ENSURE_TYPE; + /* Returns an array with pointers to "struct dirent" directory entries, optionally sorted. * * Start with space for up to 8 directory entries. We expect at least 2 ("." + ".."), hence hopefully @@ -87,9 +119,6 @@ int readdir_all(int dir_fd, RecurseDirFlags flags, DirectoryEntries **ret) { de->n_entries = 0; struct dirent *entry; FOREACH_DIRENT_IN_BUFFER(entry, de->buffer, de->buffer_size) { - if (ignore_dirent(entry, flags)) - continue; - if (FLAGS_SET(flags, RECURSE_DIR_ENSURE_TYPE)) { r = dirent_ensure_type(dir_fd, entry); if (r == -ENOENT) @@ -99,6 +128,9 @@ int readdir_all(int dir_fd, RecurseDirFlags flags, DirectoryEntries **ret) { return r; } + if (ignore_dirent(entry, flags)) + continue; + de->n_entries++; } @@ -117,14 +149,14 @@ int readdir_all(int dir_fd, RecurseDirFlags flags, DirectoryEntries **ret) { j = 0; FOREACH_DIRENT_IN_BUFFER(entry, de->buffer, de->buffer_size) { - if (ignore_dirent(entry, flags)) - continue; - /* If d_type == DT_UNKNOWN that means we failed to ensure the type in the earlier loop and * didn't include the dentry in de->n_entries and as such should skip it here as well. */ if (FLAGS_SET(flags, RECURSE_DIR_ENSURE_TYPE) && entry->d_type == DT_UNKNOWN) continue; + if (ignore_dirent(entry, flags)) + continue; + de->entries[j++] = entry; } assert(j == de->n_entries); @@ -166,6 +198,9 @@ int recurse_dir( assert(dir_fd >= 0); assert(func); + /* We cannot descend into dirs if we are supposed to ignore them */ + assert((flags & _RECURSE_DIR_MUST_BE_MASK) == 0 || FLAGS_SET(flags, RECURSE_DIR_MUST_BE_DIRECTORY)); + /* This is a lot like ftw()/nftw(), but a lot more modern, i.e. built around openat()/statx()/O_PATH, * and under the assumption that fds are not as 'expensive' as they used to be. */ diff --git a/src/basic/recurse-dir.h b/src/basic/recurse-dir.h index af33c239f7..6b6eac3639 100644 --- a/src/basic/recurse-dir.h +++ b/src/basic/recurse-dir.h @@ -7,14 +7,19 @@ typedef enum RecurseDirFlags { /* Interpreted by readdir_all() */ - RECURSE_DIR_SORT = 1 << 0, /* sort file directory entries before processing them */ - RECURSE_DIR_IGNORE_DOT = 1 << 1, /* ignore all dot files ("." and ".." are always ignored) */ - RECURSE_DIR_ENSURE_TYPE = 1 << 2, /* guarantees that 'd_type' field of 'de' is not DT_UNKNOWN */ + RECURSE_DIR_SORT = 1 << 0, /* sort file directory entries before processing them */ + RECURSE_DIR_IGNORE_DOT = 1 << 1, /* ignore all dot files ("." and ".." are always ignored) */ + RECURSE_DIR_ENSURE_TYPE = 1 << 2, /* guarantees that 'd_type' field of 'de' is not DT_UNKNOWN */ + RECURSE_DIR_MUST_BE_DIRECTORY = 1 << 3, /* ignore all entries that aren't directories */ + RECURSE_DIR_MUST_BE_REGULAR = 1 << 4, /* ignore all entries that aren't regular files */ + RECURSE_DIR_MUST_BE_SYMLINK = 1 << 5, /* ignore all entries that aren't symlinks */ + RECURSE_DIR_MUST_BE_SOCKET = 1 << 6, /* ignore all entries that aren't socket */ + _RECURSE_DIR_MUST_BE_MASK = RECURSE_DIR_MUST_BE_DIRECTORY|RECURSE_DIR_MUST_BE_REGULAR|RECURSE_DIR_MUST_BE_SYMLINK|RECURSE_DIR_MUST_BE_SOCKET, /* Interpreted by recurse_dir() */ - RECURSE_DIR_SAME_MOUNT = 1 << 3, /* skips over subdirectories that are submounts */ - RECURSE_DIR_INODE_FD = 1 << 4, /* passes an opened inode fd (O_DIRECTORY fd in case of dirs, O_PATH otherwise) */ - RECURSE_DIR_TOPLEVEL = 1 << 5, /* call RECURSE_DIR_ENTER/RECURSE_DIR_LEAVE once for top-level dir, too, with dir_fd=-1 and NULL dirent */ + RECURSE_DIR_SAME_MOUNT = 1 << 7, /* skips over subdirectories that are submounts */ + RECURSE_DIR_INODE_FD = 1 << 8, /* passes an opened inode fd (O_DIRECTORY fd in case of dirs, O_PATH otherwise) */ + RECURSE_DIR_TOPLEVEL = 1 << 9, /* call RECURSE_DIR_ENTER/RECURSE_DIR_LEAVE once for top-level dir, too, with dir_fd=-1 and NULL dirent */ } RecurseDirFlags; typedef struct DirectoryEntries { diff --git a/src/debug-generator/debug-generator.c b/src/debug-generator/debug-generator.c index 9ef271343c..700ad52893 100644 --- a/src/debug-generator/debug-generator.c +++ b/src/debug-generator/debug-generator.c @@ -279,7 +279,7 @@ static int process_unit_credentials(const char *credentials_dir) { assert(credentials_dir); - r = readdir_all_at(AT_FDCWD, credentials_dir, RECURSE_DIR_SORT|RECURSE_DIR_IGNORE_DOT|RECURSE_DIR_ENSURE_TYPE, &des); + r = readdir_all_at(AT_FDCWD, credentials_dir, RECURSE_DIR_SORT|RECURSE_DIR_IGNORE_DOT|RECURSE_DIR_MUST_BE_REGULAR, &des); if (r < 0) return log_error_errno(r, "Failed to enumerate credentials from credentials directory '%s': %m", credentials_dir); @@ -287,9 +287,6 @@ static int process_unit_credentials(const char *credentials_dir) { struct dirent *de = *i; const char *unit, *dropin; - if (de->d_type != DT_REG) - continue; - unit = startswith(de->d_name, "systemd.extra-unit."); dropin = startswith(de->d_name, "systemd.unit-dropin."); diff --git a/src/kernel-install/kernel-install.c b/src/kernel-install/kernel-install.c index c77b23d4bc..132ae803df 100644 --- a/src/kernel-install/kernel-install.c +++ b/src/kernel-install/kernel-install.c @@ -11,7 +11,6 @@ #include "build.h" #include "chase.h" #include "conf-files.h" -#include "dirent-util.h" #include "dissect-image.h" #include "env-file.h" #include "env-util.h" @@ -1247,21 +1246,11 @@ static int verb_add_all(int argc, char *argv[], uintptr_t _data, void *userdata) return log_error_errno(fd, "Failed to open %s/usr/lib/modules/: %m", strempty(arg_root)); _cleanup_free_ DirectoryEntries *de = NULL; - r = readdir_all(fd, RECURSE_DIR_SORT|RECURSE_DIR_IGNORE_DOT, &de); + r = readdir_all(fd, RECURSE_DIR_SORT|RECURSE_DIR_IGNORE_DOT|RECURSE_DIR_MUST_BE_DIRECTORY, &de); if (r < 0) return log_error_errno(r, "Failed to numerate /usr/lib/modules/ contents: %m"); FOREACH_ARRAY(d, de->entries, de->n_entries) { - r = dirent_ensure_type(fd, *d); - if (r < 0) { - if (r != -ENOENT) /* don't log if just gone by now */ - log_debug_errno(r, "Failed to check if '%s/usr/lib/modules/%s' is a directory, ignoring: %m", strempty(arg_root), (*d)->d_name); - continue; - } - - if ((*d)->d_type != DT_DIR) - continue; - _cleanup_free_ char *fn = path_join((*d)->d_name, "vmlinuz"); if (!fn) return log_oom(); @@ -1483,7 +1472,7 @@ static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) { return log_error_errno(fd, "Failed to open %s/usr/lib/modules/: %m", strempty(arg_root)); _cleanup_free_ DirectoryEntries *de = NULL; - r = readdir_all(fd, RECURSE_DIR_SORT|RECURSE_DIR_IGNORE_DOT, &de); + r = readdir_all(fd, RECURSE_DIR_SORT|RECURSE_DIR_IGNORE_DOT|RECURSE_DIR_MUST_BE_DIRECTORY, &de); if (r < 0) return log_error_errno(r, "Failed to numerate /usr/lib/modules/ contents: %m"); @@ -1501,16 +1490,6 @@ static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) { if (!j) return log_oom(); - r = dirent_ensure_type(fd, *d); - if (r < 0) { - if (r != -ENOENT) /* don't log if just gone by now */ - log_debug_errno(r, "Failed to check if '%s/%s' is a directory, ignoring: %m", strempty(arg_root), j); - continue; - } - - if ((*d)->d_type != DT_DIR) - continue; - _cleanup_free_ char *fn = path_join((*d)->d_name, "vmlinuz"); if (!fn) return log_oom(); diff --git a/src/libsystemd/sd-varlink/varlink-util.c b/src/libsystemd/sd-varlink/varlink-util.c index 1eace7724c..d454fc7f48 100644 --- a/src/libsystemd/sd-varlink/varlink-util.c +++ b/src/libsystemd/sd-varlink/varlink-util.c @@ -272,7 +272,7 @@ ssize_t varlink_execute_directory( return log_debug_errno(errno, "Failed to open '%s': %m", path); _cleanup_free_ DirectoryEntries *dentries = NULL; - r = readdir_all(fd, RECURSE_DIR_IGNORE_DOT|RECURSE_DIR_ENSURE_TYPE, &dentries); + r = readdir_all(fd, RECURSE_DIR_IGNORE_DOT|RECURSE_DIR_MUST_BE_SOCKET|RECURSE_DIR_MUST_BE_SYMLINK, &dentries); if (r < 0) return log_debug_errno(r, "Failed to enumerate '%s': %m", path); @@ -282,9 +282,6 @@ ssize_t varlink_execute_directory( FOREACH_ARRAY(dp, dentries->entries, dentries->n_entries) { struct dirent *de = *dp; - if (!IN_SET(de->d_type, DT_SOCK, DT_LNK)) - continue; - t++; _cleanup_free_ char *j = path_join(path, de->d_name); diff --git a/src/nsresourced/userns-registry.c b/src/nsresourced/userns-registry.c index 0bfb1edbde..0da6a6a7c6 100644 --- a/src/nsresourced/userns-registry.c +++ b/src/nsresourced/userns-registry.c @@ -1219,7 +1219,7 @@ int userns_registry_per_uid(int dir_fd, uid_t owner) { _cleanup_free_ DirectoryEntries *de = NULL; - r = readdir_all_at(dir_fd, uid_fn, RECURSE_DIR_IGNORE_DOT|RECURSE_DIR_ENSURE_TYPE, &de); + r = readdir_all_at(dir_fd, uid_fn, RECURSE_DIR_IGNORE_DOT|RECURSE_DIR_MUST_BE_REGULAR, &de); if (r == -ENOENT) return 0; if (r < 0) @@ -1228,9 +1228,6 @@ int userns_registry_per_uid(int dir_fd, uid_t owner) { FOREACH_ARRAY(i, de->entries, de->n_entries) { struct dirent *e = *i; - if (e->d_type != DT_REG) - continue; - if (!startswith(e->d_name, "i") || !endswith(e->d_name, ".userns")) continue; diff --git a/src/shared/creds-util.c b/src/shared/creds-util.c index e20b64291a..739973aa46 100644 --- a/src/shared/creds-util.c +++ b/src/shared/creds-util.c @@ -1805,16 +1805,13 @@ int pick_up_credentials(const PickUpCredential *table, size_t n_table_entry) { return log_error_errno(credential_dir_fd, "Failed to open credentials directory: %m"); _cleanup_free_ DirectoryEntries *des = NULL; - r = readdir_all(credential_dir_fd, RECURSE_DIR_SORT|RECURSE_DIR_IGNORE_DOT|RECURSE_DIR_ENSURE_TYPE, &des); + r = readdir_all(credential_dir_fd, RECURSE_DIR_SORT|RECURSE_DIR_IGNORE_DOT|RECURSE_DIR_MUST_BE_REGULAR, &des); if (r < 0) return log_error_errno(r, "Failed to enumerate credentials: %m"); FOREACH_ARRAY(i, des->entries, des->n_entries) { struct dirent *de = *i; - if (de->d_type != DT_REG) - continue; - FOREACH_ARRAY(t, table, n_table_entry) { r = pick_up_credential_one(credential_dir_fd, de->d_name, t); if (r != 0) { diff --git a/src/storage/storagectl.c b/src/storage/storagectl.c index 7e366c11a9..b899513616 100644 --- a/src/storage/storagectl.c +++ b/src/storage/storagectl.c @@ -348,16 +348,13 @@ static int verb_providers(int argc, char *argv[], uintptr_t data, void *userdata return log_error_errno(errno, "Failed to open '%s': %m", socket_path); } else { _cleanup_free_ DirectoryEntries *dentries = NULL; - r = readdir_all(fd, RECURSE_DIR_SORT|RECURSE_DIR_IGNORE_DOT|RECURSE_DIR_ENSURE_TYPE, &dentries); + r = readdir_all(fd, RECURSE_DIR_SORT|RECURSE_DIR_IGNORE_DOT|RECURSE_DIR_MUST_BE_SOCKET, &dentries); if (r < 0) return log_error_errno(r, "Failed to enumerate '%s': %m", socket_path); FOREACH_ARRAY(dp, dentries->entries, dentries->n_entries) { struct dirent *de = *dp; - if (de->d_type != DT_SOCK) - continue; - if (!storage_provider_name_is_valid(de->d_name)) continue; diff --git a/src/test/test-recurse-dir.c b/src/test/test-recurse-dir.c index b29a7eab9d..453b89d172 100644 --- a/src/test/test-recurse-dir.c +++ b/src/test/test-recurse-dir.c @@ -2,14 +2,17 @@ #include #include +#include #include "fd-util.h" #include "log.h" #include "recurse-dir.h" +#include "rm-rf.h" #include "stat-util.h" #include "strv.h" #include "tests.h" #include "time-util.h" +#include "tmpfile-util.h" static char **list_nftw = NULL; @@ -119,6 +122,71 @@ static int recurse_dir_callback( return RECURSE_DIR_CONTINUE; } +static void assert_entries(DirectoryEntries *de, char **expected) { + ASSERT_NOT_NULL(de); + + /* Verifies that the directory entries enumerated in 'de' are exactly the ones listed in + * 'expected' (order is irrelevant). */ + + ASSERT_EQ(de->n_entries, strv_length(expected)); + + FOREACH_ARRAY(i, de->entries, de->n_entries) + ASSERT_TRUE(strv_contains(expected, (*i)->d_name)); + + STRV_FOREACH(e, expected) { + bool found = false; + + FOREACH_ARRAY(i, de->entries, de->n_entries) + if (streq((*i)->d_name, *e)) { + found = true; + break; + } + + ASSERT_TRUE(found); + } +} + +static void check_readdir_all(int tfd, RecurseDirFlags flags, char **expected) { + _cleanup_free_ DirectoryEntries *de = NULL; + _cleanup_close_ int fd = -EBADF; + + /* readdir_all() consumes the directory fd offset, hence reopen a fresh fd for each enumeration. */ + ASSERT_OK(fd = fd_reopen(tfd, O_DIRECTORY|O_CLOEXEC)); + ASSERT_OK(readdir_all(fd, flags, &de)); + assert_entries(de, expected); +} + +static void test_must_be_flags(void) { + _cleanup_(rm_rf_physical_and_freep) char *t = NULL; + _cleanup_close_ int tfd = -EBADF, reg_fd = -EBADF; + + log_info("/* %s */", __func__); + + /* Populate a temporary directory with one entry of each interesting type and verify that the + * RECURSE_DIR_MUST_BE_* flags select exactly the right subset. */ + + ASSERT_OK(tfd = mkdtemp_open(NULL, O_DIRECTORY|O_CLOEXEC, &t)); + + ASSERT_OK_ERRNO(mkdirat(tfd, "dir", 0777)); + ASSERT_OK_ERRNO(reg_fd = openat(tfd, "reg", O_CREAT|O_EXCL|O_WRONLY|O_CLOEXEC, 0666)); + reg_fd = safe_close(reg_fd); + ASSERT_OK_ERRNO(symlinkat("reg", tfd, "lnk")); + ASSERT_OK_ERRNO(mknodat(tfd, "sock", S_IFSOCK|0666, 0)); + + /* Without any MUST_BE flag we get all four entries. */ + check_readdir_all(tfd, 0, STRV_MAKE("dir", "reg", "lnk", "sock")); + + /* A single MUST_BE flag selects exactly the entries of the matching type. */ + check_readdir_all(tfd, RECURSE_DIR_MUST_BE_DIRECTORY, STRV_MAKE("dir")); + check_readdir_all(tfd, RECURSE_DIR_MUST_BE_REGULAR, STRV_MAKE("reg")); + check_readdir_all(tfd, RECURSE_DIR_MUST_BE_SYMLINK, STRV_MAKE("lnk")); + check_readdir_all(tfd, RECURSE_DIR_MUST_BE_SOCKET, STRV_MAKE("sock")); + + /* The flags may be combined, in which case we get the union of the matching entries. */ + check_readdir_all(tfd, RECURSE_DIR_MUST_BE_REGULAR|RECURSE_DIR_MUST_BE_SYMLINK, STRV_MAKE("reg", "lnk")); + check_readdir_all(tfd, RECURSE_DIR_MUST_BE_DIRECTORY|RECURSE_DIR_MUST_BE_SOCKET, STRV_MAKE("dir", "sock")); +} + int main(int argc, char *argv[]) { _cleanup_strv_free_ char **list_recurse_dir = NULL; const char *p; @@ -128,6 +196,8 @@ int main(int argc, char *argv[]) { log_show_color(true); test_setup_logging(LOG_INFO); + test_must_be_flags(); + if (argc > 1) p = argv[1]; else