From 628f45f4a35937b64833309709e840b63c84f56b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 30 Jun 2025 15:18:47 +0900 Subject: [PATCH 01/12] chase: allow to request O_PATH fd even with CHASE_NONEXISTENT --- src/basic/chase.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/basic/chase.c b/src/basic/chase.c index fe060324ef..3a929498bf 100644 --- a/src/basic/chase.c +++ b/src/basic/chase.c @@ -126,10 +126,6 @@ int chaseat(int dir_fd, const char *path, ChaseFlags flags, char **ret_path, int assert(!FLAGS_SET(flags, CHASE_STEP|CHASE_EXTRACT_FILENAME)); assert(dir_fd >= 0 || dir_fd == AT_FDCWD); - /* Either the file may be missing, or we return an fd to the final object, but both make no sense */ - if (FLAGS_SET(flags, CHASE_NONEXISTENT)) - assert(!ret_fd); - if (FLAGS_SET(flags, CHASE_STEP)) assert(!ret_fd); @@ -552,11 +548,13 @@ int chaseat(int dir_fd, const char *path, ChaseFlags flags, char **ret_path, int } if (ret_fd) { - /* Return the O_PATH fd we currently are looking to the caller. It can translate it to a - * proper fd by opening /proc/self/fd/xyz. */ - - assert(fd >= 0); - *ret_fd = TAKE_FD(fd); + if (exists) { + /* Return the O_PATH fd we currently are looking to the caller. It can translate it + * to a proper fd by opening /proc/self/fd/xyz. */ + assert(fd >= 0); + *ret_fd = TAKE_FD(fd); + } else + *ret_fd = -EBADF; } if (FLAGS_SET(flags, CHASE_STEP)) From 31b7616420b9a76273520966e28699651497e496 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 29 Jun 2025 10:12:09 +0900 Subject: [PATCH 02/12] conf-files: introduce struct ConfFile to store information of found conf file It is currently unused, will be used later. Preparation for later changes. --- src/basic/conf-files.c | 224 +++++++++++++++++++++++++++++++++-------- src/basic/conf-files.h | 18 ++++ src/basic/forward.h | 1 + 3 files changed, 200 insertions(+), 43 deletions(-) diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index 8b4d3d7548..0c5b464318 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -20,6 +20,187 @@ #include "string-util.h" #include "strv.h" +ConfFile* conf_file_free(ConfFile *c) { + if (!c) + return NULL; + + free(c->name); + free(c->result); + free(c->original_path); + free(c->resolved_path); + safe_close(c->fd); + + return mfree(c); +} + +void conf_file_free_many(ConfFile **array, size_t n) { + FOREACH_ARRAY(i, array, n) + conf_file_free(*i); + + free(array); +} + +static int prepare_dirs(const char *root, char * const *dirs, int *ret_rfd, char **ret_root, char ***ret_dirs) { + _cleanup_free_ char *root_abs = NULL; + _cleanup_strv_free_ char **dirs_abs = NULL; + int r; + + assert(ret_rfd); + assert(ret_root); + assert(ret_dirs || strv_isempty(dirs)); + + r = empty_or_root_harder_to_null(&root); + if (r < 0) + return log_debug_errno(r, "Failed to determine if '%s' points to the root directory: %m", strempty(root)); + + if (ret_dirs) { + dirs_abs = strv_copy(dirs); + if (!dirs_abs) + return log_oom(); + } + + if (root) { + /* When a non-trivial root is specified, we will prefix the result later. Hence, it is not + * necessary to modify each config directories here. but needs to normalize the root directory. */ + r = path_make_absolute_cwd(root, &root_abs); + if (r < 0) + return log_debug_errno(r, "Failed to make '%s' absolute: %m", root); + + path_simplify(root_abs); + } else if (ret_dirs) { + /* When an empty root or "/" is specified, we will open "/" below, hence we need to make + * each config directory absolute if relative. */ + r = path_strv_make_absolute_cwd(dirs_abs); + if (r < 0) + return log_debug_errno(r, "Failed to make directories absolute: %m"); + } + + _cleanup_close_ int rfd = open(empty_to_root(root_abs), O_CLOEXEC|O_DIRECTORY|O_PATH); + if (rfd < 0) + return log_debug_errno(errno, "Failed to open '%s': %m", empty_to_root(root_abs)); + + *ret_rfd = TAKE_FD(rfd); + *ret_root = TAKE_PTR(root_abs); + if (ret_dirs) + *ret_dirs = TAKE_PTR(dirs_abs); + return 0; +} + +static int conf_file_prefix_root(ConfFile *c, const char *root) { + char *p; + int r; + + assert(c); + + r = chaseat_prefix_root(c->result, root, &p); + if (r < 0) + return log_debug_errno(r, "Failed to prefix '%s' with root '%s': %m", c->result, root); + free_and_replace(c->result, p); + + r = chaseat_prefix_root(c->resolved_path, root, &p); + if (r < 0) + return log_debug_errno(r, "Failed to prefix '%s' with root '%s': %m", c->resolved_path, root); + free_and_replace(c->resolved_path, p); + + /* Do not use chaseat_prefix_root(), as it is for the result of chaseat(), but the path is not chased. */ + p = path_join(empty_to_root(root), skip_leading_slash(c->original_path)); + if (!p) + return log_oom_debug(); + path_simplify(p); + free_and_replace(c->original_path, p); + + return 0; +} + +int conf_file_new_at(const char *path, int rfd, ChaseFlags chase_flags, ConfFile **ret) { + int r; + + assert(path); + assert(rfd >= 0 || rfd == AT_FDCWD); + assert(ret); + + _cleanup_free_ char *root = NULL; + if (rfd >= 0 && DEBUG_LOGGING) + (void) fd_get_path(rfd, &root); + + _cleanup_(conf_file_freep) ConfFile *c = new(ConfFile, 1); + if (!c) + return log_oom_debug(); + + *c = (ConfFile) { + .original_path = strdup(path), + .fd = -EBADF, + }; + + if (!c->original_path) + return log_oom_debug(); + + r = path_extract_filename(path, &c->name); + if (r < 0) + return log_debug_errno(r, "Failed to extract filename from '%s': %m", path); + + _cleanup_free_ char *dirpath = NULL, *resolved_dirpath = NULL; + r = path_extract_directory(path, &dirpath); + if (r < 0 && r != -EDESTADDRREQ) + return log_debug_errno(r, "Failed to extract directory from '%s': %m", path); + if (r >= 0) { + r = chaseat(rfd, dirpath, + CHASE_AT_RESOLVE_IN_ROOT | (FLAGS_SET(chase_flags, CHASE_NONEXISTENT) ? CHASE_NONEXISTENT : CHASE_MUST_BE_DIRECTORY), + &resolved_dirpath, /* ret_fd = */ NULL); + if (r < 0) + return log_debug_errno(r, "Failed to chase '%s%s': %m", empty_to_root(root), skip_leading_slash(dirpath)); + } + + c->result = path_join(resolved_dirpath, c->name); + if (!c->result) + return log_oom_debug(); + + r = chaseat(rfd, c->result, CHASE_AT_RESOLVE_IN_ROOT | chase_flags, &c->resolved_path, &c->fd); + if (r < 0) + return log_debug_errno(r, "Failed to chase '%s%s': %m", empty_to_root(root), skip_leading_slash(c->original_path)); + + if (c->fd >= 0 && fstat(c->fd, &c->st) < 0) + return log_debug_errno(r, "Failed to stat '%s%s': %m", empty_to_root(root), skip_leading_slash(c->resolved_path)); + + *ret = TAKE_PTR(c); + return 0; +} + +int conf_file_new(const char *path, const char *root, ChaseFlags chase_flags, ConfFile **ret) { + int r; + + assert(path); + assert((chase_flags & (CHASE_PREFIX_ROOT | CHASE_STEP)) == 0); + assert(ret); + + _cleanup_free_ char *root_abs = NULL; + _cleanup_close_ int rfd = -EBADF; + r = prepare_dirs(root, /* dirs = */ NULL, &rfd, &root_abs, /* ret_dirs = */ NULL); + if (r < 0) + return r; + + _cleanup_free_ char *path_abs = NULL; + if (!root_abs) { + r = path_make_absolute_cwd(path, &path_abs); + if (r < 0) + return log_debug_errno(r, "Failed to make '%s' absolute: %m", path); + + path = path_abs; + } + + _cleanup_(conf_file_freep) ConfFile *c = NULL; + r = conf_file_new_at(path, rfd, chase_flags, &c); + if (r < 0) + return r; + + r = conf_file_prefix_root(c, root_abs); + if (r < 0) + return r; + + *ret = TAKE_PTR(c); + return 0; +} + static int files_add( DIR *dir, const char *dirpath, @@ -332,49 +513,6 @@ static int conf_files_list_impl( return 0; } -static int prepare_dirs(const char *root, char * const *dirs, int *ret_rfd, char **ret_root, char ***ret_dirs) { - _cleanup_free_ char *root_abs = NULL; - _cleanup_strv_free_ char **dirs_abs = NULL; - int r; - - assert(ret_rfd); - assert(ret_root); - assert(ret_dirs); - - r = empty_or_root_harder_to_null(&root); - if (r < 0) - return log_debug_errno(r, "Failed to determine if '%s' points to the root directory: %m", strempty(root)); - - dirs_abs = strv_copy(dirs); - if (!dirs_abs) - return log_oom(); - - if (root) { - /* WHen a non-trivial root is specified, we will prefix the result later. Hence, it is not - * necessary to modify each config directories here. but needs to normalize the root directory. */ - r = path_make_absolute_cwd(root, &root_abs); - if (r < 0) - return log_debug_errno(r, "Failed to make '%s' absolute: %m", root); - - path_simplify(root_abs); - } else { - /* When an empty root or "/" is specified, we will open "/" below, hence we need to make - * each config directory absolute if relative. */ - r = path_strv_make_absolute_cwd(dirs_abs); - if (r < 0) - return log_debug_errno(r, "Failed to make directories absolute: %m"); - } - - _cleanup_close_ int rfd = open(empty_to_root(root_abs), O_CLOEXEC|O_DIRECTORY|O_PATH); - if (rfd < 0) - return log_debug_errno(errno, "Failed to open '%s': %m", empty_to_root(root_abs)); - - *ret_rfd = TAKE_FD(rfd); - *ret_root = TAKE_PTR(root_abs); - *ret_dirs = TAKE_PTR(dirs_abs); - return 0; -} - int conf_files_list_strv( char ***ret, const char *suffix, diff --git a/src/basic/conf-files.h b/src/basic/conf-files.h index 710e74c959..3941dead24 100644 --- a/src/basic/conf-files.h +++ b/src/basic/conf-files.h @@ -1,6 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include + #include "forward.h" typedef enum ConfFilesFlags { @@ -13,6 +15,22 @@ typedef enum ConfFilesFlags { CONF_FILES_FILTER_MASKED = CONF_FILES_FILTER_MASKED_BY_SYMLINK | CONF_FILES_FILTER_MASKED_BY_EMPTY, } ConfFilesFlags; +typedef struct ConfFile { + char *name; /* name of a file found in config directories */ + char *result; /* resolved config directory with the original file name found in the directory */ + char *original_path; /* original config directory with the original file name found in the directory */ + char *resolved_path; /* fully resolved path, where the filename part of the path may be different from the original name */ + int fd; /* O_PATH fd to resolved_path, -EBADF if the resolved_path does not exist */ + struct stat st; /* stat of the file. */ +} ConfFile; + +ConfFile* conf_file_free(ConfFile *c); +DEFINE_TRIVIAL_CLEANUP_FUNC(ConfFile*, conf_file_free); +void conf_file_free_many(ConfFile **array, size_t n); + +int conf_file_new_at(const char *path, int rfd, ChaseFlags chase_flags, ConfFile **ret); +int conf_file_new(const char *path, const char *root, ChaseFlags chase_flags, ConfFile **ret); + int conf_files_list(char ***ret, const char *suffix, const char *root, ConfFilesFlags flags, const char *dir); int conf_files_list_at(char ***ret, const char *suffix, int rfd, ConfFilesFlags flags, const char *dir); int conf_files_list_strv(char ***ret, const char *suffix, const char *root, ConfFilesFlags flags, const char* const* dirs); diff --git a/src/basic/forward.h b/src/basic/forward.h index 7b8ce78b98..7175120e5b 100644 --- a/src/basic/forward.h +++ b/src/basic/forward.h @@ -104,6 +104,7 @@ typedef struct Set Set; typedef struct dual_timestamp dual_timestamp; typedef struct triple_timestamp triple_timestamp; +typedef struct ConfFile ConfFile; typedef struct LockFile LockFile; typedef struct PidRef PidRef; typedef struct Prioq Prioq; From d38f87c56c6e2596d720a363b5a01c19166f5e2a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 1 Jul 2025 10:33:54 +0900 Subject: [PATCH 03/12] conf-files: make conf_files_list() and friends internally use struct ConfFile No functional change, just refactoring. --- src/basic/conf-files.c | 233 ++++++++++++++++++++--------------------- 1 file changed, 113 insertions(+), 120 deletions(-) diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index 0c5b464318..dc3c17d291 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -201,9 +201,15 @@ int conf_file_new(const char *path, const char *root, ChaseFlags chase_flags, Co return 0; } +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + conf_file_hash_ops, + char, string_hash_func, string_compare_func, + ConfFile, conf_file_free); + static int files_add( DIR *dir, - const char *dirpath, + const char *original_dirpath, + const char *resolved_dirpath, int rfd, const char *root, /* for logging, can be NULL */ Hashmap **files, @@ -214,7 +220,8 @@ static int files_add( int r; assert(dir); - assert(dirpath); + assert(original_dirpath); + assert(resolved_dirpath); assert(rfd >= 0 || rfd == AT_FDCWD); assert(files); assert(masked); @@ -222,46 +229,52 @@ static int files_add( root = strempty(root); FOREACH_DIRENT(de, dir, return log_debug_errno(errno, "Failed to read directory '%s/%s': %m", - root, skip_leading_slash(dirpath))) { + root, skip_leading_slash(original_dirpath))) { + + _cleanup_free_ char *original_path = path_join(original_dirpath, de->d_name); + if (!original_path) + return log_oom_debug(); /* Does this match the suffix? */ - if (suffix && !endswith(de->d_name, suffix)) + if (suffix && !endswith(de->d_name, suffix)) { + log_debug("Skipping file '%s/%s' with unexpected suffix.", root, skip_leading_slash(original_path)); continue; + } /* Has this file already been found in an earlier directory? */ if (hashmap_contains(*files, de->d_name)) { - log_debug("Skipping overridden file '%s/%s/%s'.", - root, skip_leading_slash(dirpath), de->d_name); + log_debug("Skipping overridden file '%s/%s'.", root, skip_leading_slash(original_path)); continue; } /* Has this been masked in an earlier directory? */ if ((flags & CONF_FILES_FILTER_MASKED) != 0 && set_contains(*masked, de->d_name)) { - log_debug("File '%s/%s/%s' is masked by previous entry.", - root, skip_leading_slash(dirpath), de->d_name); + log_debug("File '%s/%s' is masked by previous entry.", root, skip_leading_slash(original_path)); continue; } - _cleanup_free_ char *p = path_join(dirpath, de->d_name); + _cleanup_free_ char *p = path_join(resolved_dirpath, de->d_name); if (!p) return log_oom_debug(); _cleanup_free_ char *resolved_path = NULL; + _cleanup_close_ int fd = -EBADF; bool need_stat = (flags & (CONF_FILES_FILTER_MASKED | CONF_FILES_REGULAR | CONF_FILES_DIRECTORY | CONF_FILES_EXECUTABLE)) != 0; - struct stat st; - - if (!need_stat || FLAGS_SET(flags, CONF_FILES_FILTER_MASKED_BY_SYMLINK)) { + ChaseFlags chase_flags = CHASE_AT_RESOLVE_IN_ROOT; + if (!need_stat || FLAGS_SET(flags, CONF_FILES_FILTER_MASKED_BY_SYMLINK)) /* Even if no verification is requested, let's unconditionally call chaseat(), * to drop unsafe symlinks. */ + chase_flags |= CHASE_NONEXISTENT; - r = chaseat(rfd, p, CHASE_AT_RESOLVE_IN_ROOT | CHASE_NONEXISTENT, &resolved_path, /* ret_fd = */ NULL); - if (r < 0) { - log_debug_errno(r, "Failed to chase '%s/%s', ignoring: %m", - root, skip_leading_slash(p)); - continue; - } - if (r == 0 && FLAGS_SET(flags, CONF_FILES_FILTER_MASKED_BY_SYMLINK)) { + r = chaseat(rfd, p, chase_flags, &resolved_path, &fd); + if (r < 0) { + log_debug_errno(r, "Failed to chase '%s/%s', ignoring: %m", + root, skip_leading_slash(original_path)); + continue; + } + if (r == 0) { + if (FLAGS_SET(flags, CONF_FILES_FILTER_MASKED_BY_SYMLINK)) { /* If the path points to /dev/null in a image or so, then the device node may not exist. */ if (path_equal(skip_leading_slash(resolved_path), "dev/null")) { @@ -271,29 +284,25 @@ static int files_add( return log_oom_debug(); log_debug("File '%s/%s' is a mask (symlink to /dev/null).", - root, skip_leading_slash(p)); + root, skip_leading_slash(original_path)); continue; } + } - /* If the flag is set, we need to have stat, hence, skip the entry. */ + if (need_stat) { + /* If we need to have stat, skip the entry. */ log_debug_errno(SYNTHETIC_ERRNO(ENOENT), "Failed to chase '%s/%s', ignoring: %m", - root, skip_leading_slash(p)); + root, skip_leading_slash(original_path)); continue; } + } - if (need_stat && fstatat(rfd, resolved_path, &st, AT_SYMLINK_NOFOLLOW) < 0) { - log_debug_errno(errno, "Failed to stat '%s/%s', ignoring: %m", - root, skip_leading_slash(p)); - continue; - } - - } else { - r = chase_and_statat(rfd, p, CHASE_AT_RESOLVE_IN_ROOT, &resolved_path, &st); - if (r < 0) { - log_debug_errno(r, "Failed to chase and stat '%s/%s', ignoring: %m", - root, skip_leading_slash(p)); - continue; - } + /* Even if we do not need stat, let's take stat now. The caller may use the info later. */ + struct stat st = {}; + if (fd >= 0 && fstat(fd, &st) < 0) { + log_debug_errno(errno, "Failed to stat '%s/%s', ignoring: %m", + root, skip_leading_slash(original_path)); + continue; } /* Is this a masking entry? */ @@ -303,7 +312,7 @@ static int files_add( if (r < 0) return log_oom_debug(); - log_debug("File '%s/%s' is a mask (symlink to /dev/null).", root, skip_leading_slash(p)); + log_debug("File '%s/%s' is a mask (symlink to /dev/null).", root, skip_leading_slash(original_path)); continue; } @@ -313,25 +322,25 @@ static int files_add( if (r < 0) return log_oom_debug(); - log_debug("File '%s/%s' is a mask (an empty file).", root, skip_leading_slash(p)); + log_debug("File '%s/%s' is a mask (an empty file).", root, skip_leading_slash(original_path)); continue; } if (FLAGS_SET(flags, CONF_FILES_REGULAR|CONF_FILES_DIRECTORY)) { if (!S_ISREG(st.st_mode) && !S_ISDIR(st.st_mode)) { - log_debug("Ignoring '%s/%s', as it is neither a regular file or directory.", root, skip_leading_slash(p)); + log_debug("Ignoring '%s/%s', as it is neither a regular file or directory.", root, skip_leading_slash(original_path)); continue; } } else { /* Is this node a regular file? */ if (FLAGS_SET(flags, CONF_FILES_REGULAR) && !S_ISREG(st.st_mode)) { - log_debug("Ignoring '%s/%s', as it is not a regular file.", root, skip_leading_slash(p)); + log_debug("Ignoring '%s/%s', as it is not a regular file.", root, skip_leading_slash(original_path)); continue; } /* Is this node a directory? */ if (FLAGS_SET(flags, CONF_FILES_DIRECTORY) && !S_ISDIR(st.st_mode)) { - log_debug("Ignoring '%s/%s', as it is not a directory.", root, skip_leading_slash(p)); + log_debug("Ignoring '%s/%s', as it is not a directory.", root, skip_leading_slash(original_path)); continue; } } @@ -341,104 +350,103 @@ static int files_add( * here, as we care about whether the file is marked executable at all, and not whether it is * executable for us, because if so, such errors are stuff we should log about. */ if (FLAGS_SET(flags, CONF_FILES_EXECUTABLE) && (st.st_mode & 0111) == 0) { - log_debug("Ignoring '%s/%s', as it is not marked executable.", root, skip_leading_slash(p)); + log_debug("Ignoring '%s/%s', as it is not marked executable.", root, skip_leading_slash(original_path)); continue; } - _cleanup_free_ char *n = strdup(de->d_name); - if (!n) + _cleanup_(conf_file_freep) ConfFile *c = new(ConfFile, 1); + if (!c) return log_oom_debug(); - r = hashmap_ensure_put(files, &string_hash_ops_free_free, n, p); + *c = (ConfFile) { + .name = strdup(de->d_name), + .result = TAKE_PTR(p), + .original_path = TAKE_PTR(original_path), + .resolved_path = TAKE_PTR(resolved_path), + .fd = TAKE_FD(fd), + .st = st, + }; + + if (!c->name) + return log_oom_debug(); + + r = hashmap_ensure_put(files, &conf_file_hash_ops, c->name, c); if (r < 0) { assert(r == -ENOMEM); return log_oom_debug(); } assert(r > 0); - TAKE_PTR(n); - TAKE_PTR(p); + TAKE_PTR(c); } return 0; } static int copy_and_sort_files_from_hashmap(Hashmap *fh, const char *root, ConfFilesFlags flags, char ***ret) { - _cleanup_free_ char **sv = NULL; - _cleanup_strv_free_ char **files = NULL; - size_t n = 0; + _cleanup_strv_free_ char **results = NULL; + _cleanup_free_ ConfFile **files = NULL; + size_t n_files = 0, n_results = 0; int r; assert(ret); - r = hashmap_dump_sorted(fh, (void***) &sv, /* ret_n = */ NULL); + /* The entries in the array given by hashmap_dump_sorted() are still owned by the hashmap. + * Hence, do not use conf_file_free_many() for 'entries' */ + r = hashmap_dump_sorted(fh, (void***) &files, &n_files); if (r < 0) return log_oom_debug(); - /* The entries in the array given by hashmap_dump_sorted() are still owned by the hashmap. */ - STRV_FOREACH(s, sv) { - _cleanup_free_ char *p = NULL; + FOREACH_ARRAY(i, files, n_files) { + ConfFile *c = *i; - if (FLAGS_SET(flags, CONF_FILES_BASENAME)) { - r = path_extract_filename(*s, &p); - if (r < 0) - return log_debug_errno(r, "Failed to extract filename from '%s': %m", *s); - } else if (root) { - r = chaseat_prefix_root(*s, root, &p); - if (r < 0) - return log_debug_errno(r, "Failed to prefix '%s' with root '%s': %m", *s, root); - } + if (FLAGS_SET(flags, CONF_FILES_BASENAME)) + r = strv_extend_with_size(&results, &n_results, c->name); + else if (root) { + char *p; - if (p) - r = strv_consume_with_size(&files, &n, TAKE_PTR(p)); - else - r = strv_extend_with_size(&files, &n, *s); + r = chaseat_prefix_root(c->result, root, &p); + if (r < 0) + return log_debug_errno(r, "Failed to prefix '%s' with root '%s': %m", c->result, root); + + r = strv_consume_with_size(&results, &n_results, TAKE_PTR(p)); + } else + r = strv_extend_with_size(&results, &n_results, c->result); if (r < 0) return log_oom_debug(); } - *ret = TAKE_PTR(files); + *ret = TAKE_PTR(results); return 0; } -static int insert_replacement(Hashmap **fh, char *filename_replacement, char *resolved_replacement, char **ret) { - _cleanup_free_ char *fname = ASSERT_PTR(filename_replacement), *path = ASSERT_PTR(resolved_replacement); +static int insert_replacement(Hashmap **fh, ConfFile *replacement, const ConfFile **ret) { + _cleanup_(conf_file_freep) ConfFile *c = ASSERT_PTR(replacement); int r; assert(fh); + assert(ret); - /* This consumes the input filename and path. */ + /* This consumes the input ConfFile. */ - const char *existing = hashmap_get(*fh, fname); + ConfFile *existing = hashmap_get(*fh, c->name); if (existing) { - log_debug("An entry with higher priority already exists ('%s' -> '%s'), ignoring the replacement: %s", - fname, existing, path); - if (ret) - *ret = NULL; + log_debug("An entry with higher priority '%s' -> '%s' already exists, ignoring the replacement: %s", + existing->name, existing->result, c->original_path); + *ret = NULL; return 0; } - _cleanup_free_ char *copy = NULL; - if (ret) { - copy = strdup(path); - if (!copy) - return log_oom_debug(); - } - - r = hashmap_ensure_put(fh, &string_hash_ops_free_free, fname, path); + r = hashmap_ensure_put(fh, &conf_file_hash_ops, c->name, c); if (r < 0) { assert(r == -ENOMEM); return log_oom_debug(); } assert(r > 0); - log_debug("Inserted replacement: '%s' -> '%s'", fname, path); + log_debug("Inserted replacement: '%s' -> '%s'", c->name, c->result); - TAKE_PTR(fname); - TAKE_PTR(path); - - if (ret) - *ret = TAKE_PTR(copy); + *ret = TAKE_PTR(c); return 0; } @@ -450,33 +458,21 @@ static int conf_files_list_impl( const char * const *dirs, const char *replacement, Hashmap **ret, - char **ret_inserted) { + const ConfFile **ret_inserted) { _cleanup_hashmap_free_ Hashmap *fh = NULL; _cleanup_set_free_ Set *masked = NULL; + _cleanup_(conf_file_freep) ConfFile *c = NULL; + const ConfFile *inserted = NULL; int r; assert(rfd >= 0 || rfd == AT_FDCWD); assert(ret); - _cleanup_free_ char *filename_replacement = NULL, *resolved_dirpath_replacement = NULL, *resolved_replacement = NULL, *inserted = NULL; if (replacement) { - r = path_extract_filename(replacement, &filename_replacement); + r = conf_file_new_at(replacement, rfd, CHASE_NONEXISTENT, &c); if (r < 0) return r; - - _cleanup_free_ char *d = NULL; - r = path_extract_directory(replacement, &d); - if (r < 0) - return r; - - r = chaseat(rfd, d, CHASE_AT_RESOLVE_IN_ROOT | CHASE_NONEXISTENT, &resolved_dirpath_replacement, /* ret_fd = */ NULL); - if (r < 0) - return r; - - resolved_replacement = path_join(resolved_dirpath_replacement, filename_replacement); - if (!resolved_replacement) - return log_oom_debug(); } STRV_FOREACH(p, dirs) { @@ -486,30 +482,30 @@ static int conf_files_list_impl( r = chase_and_opendirat(rfd, *p, CHASE_AT_RESOLVE_IN_ROOT, &path, &dir); if (r < 0) { if (r != -ENOENT) - log_debug_errno(r, "Failed to chase and open directory '%s%s', ignoring: %m", strempty(root), *p); + log_debug_errno(r, "Failed to chase and open directory '%s/%s', ignoring: %m", strempty(root), skip_leading_slash(*p)); continue; } - if (resolved_replacement && path_equal(resolved_dirpath_replacement, path)) { - r = insert_replacement(&fh, TAKE_PTR(filename_replacement), TAKE_PTR(resolved_replacement), ret_inserted ? &inserted : NULL); + if (c && streq_ptr(path_startswith(c->result, path), c->name)) { + r = insert_replacement(&fh, TAKE_PTR(c), &inserted); if (r < 0) return r; } - r = files_add(dir, path, rfd, root, &fh, &masked, suffix, flags); + r = files_add(dir, *p, path, rfd, root, &fh, &masked, suffix, flags); if (r == -ENOMEM) return r; } - if (resolved_replacement) { - r = insert_replacement(&fh, TAKE_PTR(filename_replacement), TAKE_PTR(resolved_replacement), ret_inserted ? &inserted : NULL); + if (c) { + r = insert_replacement(&fh, TAKE_PTR(c), &inserted); if (r < 0) return r; } *ret = TAKE_PTR(fh); if (ret_inserted) - *ret_inserted = TAKE_PTR(inserted); + *ret_inserted = inserted; return 0; } @@ -609,6 +605,7 @@ int conf_files_list_with_replacement( _cleanup_close_ int rfd = -EBADF; _cleanup_free_ char *root_abs = NULL; _cleanup_strv_free_ char **dirs_abs = NULL; + const ConfFile *c = NULL; int r; assert(ret_files); @@ -618,18 +615,14 @@ int conf_files_list_with_replacement( return r; r = conf_files_list_impl(".conf", rfd, root_abs, flags, (const char * const *) dirs_abs, - replacement, &fh, ret_inserted ? &inserted : NULL); + replacement, &fh, ret_inserted ? &c : NULL); if (r < 0) return r; - if (inserted) { - char *p; - - r = chaseat_prefix_root(inserted, root_abs, &p); + if (c) { + r = chaseat_prefix_root(c->result, root_abs, &inserted); if (r < 0) - return log_debug_errno(r, "Failed to prefix '%s' with root '%s': %m", inserted, empty_to_root(root_abs)); - - free_and_replace(inserted, p); + return log_debug_errno(r, "Failed to prefix '%s' with root '%s': %m", c->result, empty_to_root(root_abs)); } r = copy_and_sort_files_from_hashmap(fh, empty_to_root(root_abs), flags, ret_files); From 0e0b482a714c7f3ba180a9ed32440e6261bb7c69 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 29 Jun 2025 11:01:52 +0900 Subject: [PATCH 04/12] conf-files: introduce conf_files_list_full() and friends that provides results in ConfFile --- src/basic/conf-files.c | 120 +++++++++++++++++++++++++++++++++++++++++ src/basic/conf-files.h | 8 +++ 2 files changed, 128 insertions(+) diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index dc3c17d291..2e4f28ba88 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -383,6 +383,37 @@ static int files_add( return 0; } +static int dump_files(Hashmap *fh, const char *root, ConfFile ***ret_files, size_t *ret_n_files) { + ConfFile **files = NULL; + size_t n_files = 0; + int r; + + CLEANUP_ARRAY(files, n_files, conf_file_free_many); + + assert(ret_files); + assert(ret_n_files); + + /* The entries in the array given by hashmap_dump_sorted() are still owned by the hashmap. */ + r = hashmap_dump_sorted(fh, (void***) &files, &n_files); + if (r < 0) + return log_oom_debug(); + + /* Hence, we need to remove them from the hashmap. */ + FOREACH_ARRAY(i, files, n_files) + assert_se(hashmap_remove(fh, (*i)->name) == *i); + + if (root) + FOREACH_ARRAY(i, files, n_files) { + r = conf_file_prefix_root(*i, root); + if (r < 0) + return r; + } + + *ret_files = TAKE_PTR(files); + *ret_n_files = n_files; + return 0; +} + static int copy_and_sort_files_from_hashmap(Hashmap *fh, const char *root, ConfFilesFlags flags, char ***ret) { _cleanup_strv_free_ char **results = NULL; _cleanup_free_ ConfFile **files = NULL; @@ -536,6 +567,35 @@ int conf_files_list_strv( return copy_and_sort_files_from_hashmap(fh, empty_to_root(root_abs), flags, ret); } +int conf_files_list_strv_full( + const char *suffix, + const char *root, + ConfFilesFlags flags, + const char * const *dirs, + ConfFile ***ret_files, + size_t *ret_n_files) { + + _cleanup_hashmap_free_ Hashmap *fh = NULL; + _cleanup_close_ int rfd = -EBADF; + _cleanup_free_ char *root_abs = NULL; + _cleanup_strv_free_ char **dirs_abs = NULL; + int r; + + assert(ret_files); + assert(ret_n_files); + + r = prepare_dirs(root, (char**) dirs, &rfd, &root_abs, &dirs_abs); + if (r < 0) + return r; + + r = conf_files_list_impl(suffix, rfd, root_abs, flags, (const char * const *) dirs_abs, + /* replacement = */ NULL, &fh, /* ret_inserted = */ NULL); + if (r < 0) + return r; + + return dump_files(fh, empty_to_root(root_abs), ret_files, ret_n_files); +} + int conf_files_list_strv_at( char ***ret, const char *suffix, @@ -560,14 +620,48 @@ int conf_files_list_strv_at( return copy_and_sort_files_from_hashmap(fh, /* root = */ NULL, flags, ret); } +int conf_files_list_strv_at_full( + const char *suffix, + int rfd, + ConfFilesFlags flags, + const char * const *dirs, + ConfFile ***ret_files, + size_t *ret_n_files) { + + _cleanup_hashmap_free_ Hashmap *fh = NULL; + _cleanup_free_ char *root = NULL; + int r; + + assert(rfd >= 0 || rfd == AT_FDCWD); + assert(ret_files); + assert(ret_n_files); + + if (rfd >= 0 && DEBUG_LOGGING) + (void) fd_get_path(rfd, &root); /* for logging */ + + r = conf_files_list_impl(suffix, rfd, root, flags, dirs, /* replacement = */ NULL, &fh, /* ret_inserted = */ NULL); + if (r < 0) + return r; + + return dump_files(fh, /* root = */ NULL, ret_files, ret_n_files); +} + int conf_files_list(char ***ret, const char *suffix, const char *root, ConfFilesFlags flags, const char *dir) { return conf_files_list_strv(ret, suffix, root, flags, STRV_MAKE_CONST(dir)); } +int conf_files_list_full(const char *suffix, const char *root, ConfFilesFlags flags, const char *dir, ConfFile ***ret_files, size_t *ret_n_files) { + return conf_files_list_strv_full(suffix, root, flags, STRV_MAKE_CONST(dir), ret_files, ret_n_files); +} + int conf_files_list_at(char ***ret, const char *suffix, int rfd, ConfFilesFlags flags, const char *dir) { return conf_files_list_strv_at(ret, suffix, rfd, flags, STRV_MAKE_CONST(dir)); } +int conf_files_list_at_full(const char *suffix, int rfd, ConfFilesFlags flags, const char *dir, ConfFile ***ret_files, size_t *ret_n_files) { + return conf_files_list_strv_at_full(suffix, rfd, flags, STRV_MAKE_CONST(dir), ret_files, ret_n_files); +} + int conf_files_list_nulstr(char ***ret, const char *suffix, const char *root, ConfFilesFlags flags, const char *dirs) { _cleanup_strv_free_ char **d = NULL; @@ -580,6 +674,19 @@ int conf_files_list_nulstr(char ***ret, const char *suffix, const char *root, Co return conf_files_list_strv(ret, suffix, root, flags, (const char**) d); } +int conf_files_list_nulstr_full(const char *suffix, const char *root, ConfFilesFlags flags, const char *dirs, ConfFile ***ret_files, size_t *ret_n_files) { + _cleanup_strv_free_ char **d = NULL; + + assert(ret_files); + assert(ret_n_files); + + d = strv_split_nulstr(dirs); + if (!d) + return -ENOMEM; + + return conf_files_list_strv_full(suffix, root, flags, (const char**) d, ret_files, ret_n_files); +} + int conf_files_list_nulstr_at(char ***ret, const char *suffix, int rfd, ConfFilesFlags flags, const char *dirs) { _cleanup_strv_free_ char **d = NULL; @@ -592,6 +699,19 @@ int conf_files_list_nulstr_at(char ***ret, const char *suffix, int rfd, ConfFile return conf_files_list_strv_at(ret, suffix, rfd, flags, (const char**) d); } +int conf_files_list_nulstr_at_full(const char *suffix, int rfd, ConfFilesFlags flags, const char *dirs, ConfFile ***ret_files, size_t *ret_n_files) { + _cleanup_strv_free_ char **d = NULL; + + assert(ret_files); + assert(ret_n_files); + + d = strv_split_nulstr(dirs); + if (!d) + return -ENOMEM; + + return conf_files_list_strv_at_full(suffix, rfd, flags, (const char**) d, ret_files, ret_n_files); +} + int conf_files_list_with_replacement( const char *root, char **config_dirs, diff --git a/src/basic/conf-files.h b/src/basic/conf-files.h index 3941dead24..c5cad2084c 100644 --- a/src/basic/conf-files.h +++ b/src/basic/conf-files.h @@ -37,6 +37,14 @@ int conf_files_list_strv(char ***ret, const char *suffix, const char *root, Conf int conf_files_list_strv_at(char ***ret, const char *suffix, int rfd, ConfFilesFlags flags, const char * const *dirs); int conf_files_list_nulstr(char ***ret, const char *suffix, const char *root, ConfFilesFlags flags, const char *dirs); int conf_files_list_nulstr_at(char ***ret, const char *suffix, int rfd, ConfFilesFlags flags, const char *dirs); + +int conf_files_list_full(const char *suffix, const char *root, ConfFilesFlags flags, const char *dir, ConfFile ***ret_files, size_t *ret_n_files); +int conf_files_list_at_full(const char *suffix, int rfd, ConfFilesFlags flags, const char *dir, ConfFile ***ret_files, size_t *ret_n_files); +int conf_files_list_strv_full(const char *suffix, const char *root, ConfFilesFlags flags, const char * const *dirs, ConfFile ***ret_files, size_t *ret_n_files); +int conf_files_list_strv_at_full(const char *suffix, int rfd, ConfFilesFlags flags, const char * const *dirs, ConfFile ***ret_files, size_t *ret_n_files); +int conf_files_list_nulstr_full(const char *suffix, const char *root, ConfFilesFlags flags, const char *dirs, ConfFile ***ret_files, size_t *ret_n_files); +int conf_files_list_nulstr_at_full(const char *suffix, int rfd, ConfFilesFlags flags, const char *dirs, ConfFile ***ret_files, size_t *ret_n_files); + int conf_files_list_with_replacement( const char *root, char **config_dirs, From 86c4e42380a66f3adc916349f90a9c3b95415cb5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 30 Jun 2025 05:18:32 +0900 Subject: [PATCH 05/12] pretty-print: several cleanups for cat_files() - drop redundant error messages in cat_files(), as cat_file() internally logs errors, - show an empty line and filename before opening file, to make not mix any error messages with the previous file, - drop unnecessary fflush(), - use RET_GATHER() and continue to show files even if some files cannot be shown. --- src/shared/pretty-print.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/shared/pretty-print.c b/src/shared/pretty-print.c index 95d6d7051e..65f77e4cc9 100644 --- a/src/shared/pretty-print.c +++ b/src/shared/pretty-print.c @@ -10,6 +10,7 @@ #include "conf-files.h" #include "constants.h" #include "env-util.h" +#include "errno-util.h" #include "fd-util.h" #include "fileio.h" #include "log.h" @@ -183,27 +184,31 @@ int terminal_urlify_man(const char *page, const char *section, char **ret) { return terminal_urlify(url, text, ret); } -static int cat_file(const char *filename, bool newline, CatFlags flags) { +static int cat_file(const char *filename, bool *newline, CatFlags flags) { _cleanup_fclose_ FILE *f = NULL; _cleanup_free_ char *urlified = NULL, *section = NULL, *old_section = NULL; int r; assert(filename); - f = fopen(filename, "re"); - if (!f) - return log_error_errno(errno, "Failed to open \"%s\": %m", filename); + if (newline) { + if (*newline) + putc('\n', stdout); + *newline = true; + } r = terminal_urlify_path(filename, NULL, &urlified); if (r < 0) return log_error_errno(r, "Failed to urlify path \"%s\": %m", filename); - printf("%s%s# %s%s\n", - newline ? "\n" : "", + printf("%s# %s%s\n", ansi_highlight_blue(), urlified, ansi_normal()); - fflush(stdout); + + f = fopen(filename, "re"); + if (!f) + return log_error_errno(errno, "Failed to open \"%s\": %m", filename); for (bool continued = false;;) { _cleanup_free_ char *line = NULL; @@ -292,21 +297,16 @@ static int cat_file(const char *filename, bool newline, CatFlags flags) { } int cat_files(const char *file, char **dropins, CatFlags flags) { - int r; + bool newline = false; + int ret = 0; - if (file) { - r = cat_file(file, /* newline= */ false, flags); - if (r < 0) - return log_warning_errno(r, "Failed to cat %s: %m", file); - } + if (file) + ret = cat_file(file, &newline, flags); - STRV_FOREACH(path, dropins) { - r = cat_file(*path, /* newline= */ file || path != dropins, flags); - if (r < 0) - return log_warning_errno(r, "Failed to cat %s: %m", *path); - } + STRV_FOREACH(path, dropins) + RET_GATHER(ret, cat_file(*path, &newline, flags)); - return 0; + return ret; } void print_separator(void) { From 661b5bfd216e383ac7836261eea9671875e6709b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 30 Jun 2025 05:22:53 +0900 Subject: [PATCH 06/12] pretty-print: make conf_files_cat() not show files outside of the specified root. Then, make the function show the original and resolved path if they are different. With this change, procfs needs to be mounted on /proc/, hence the test code is slightly updated. --- src/shared/pretty-print.c | 80 ++++++++++++++++++++++++++--------- src/shared/pretty-print.h | 1 + test/units/TEST-65-ANALYZE.sh | 1 + 3 files changed, 62 insertions(+), 20 deletions(-) diff --git a/src/shared/pretty-print.c b/src/shared/pretty-print.c index 65f77e4cc9..cbcf2a4be2 100644 --- a/src/shared/pretty-print.c +++ b/src/shared/pretty-print.c @@ -6,6 +6,7 @@ #include #include "alloc-util.h" +#include "chase.h" #include "color-util.h" #include "conf-files.h" #include "constants.h" @@ -13,6 +14,7 @@ #include "errno-util.h" #include "fd-util.h" #include "fileio.h" +#include "fs-util.h" #include "log.h" #include "path-util.h" #include "pretty-print.h" @@ -184,12 +186,15 @@ int terminal_urlify_man(const char *page, const char *section, char **ret) { return terminal_urlify(url, text, ret); } -static int cat_file(const char *filename, bool *newline, CatFlags flags) { +static int cat_file(const ConfFile *c, bool *newline, CatFlags flags) { _cleanup_fclose_ FILE *f = NULL; _cleanup_free_ char *urlified = NULL, *section = NULL, *old_section = NULL; int r; - assert(filename); + assert(c); + assert(c->original_path); + assert(c->resolved_path); + assert(c->fd >= 0); if (newline) { if (*newline) @@ -197,25 +202,29 @@ static int cat_file(const char *filename, bool *newline, CatFlags flags) { *newline = true; } - r = terminal_urlify_path(filename, NULL, &urlified); - if (r < 0) - return log_error_errno(r, "Failed to urlify path \"%s\": %m", filename); + bool resolved = !path_equal(c->original_path, c->resolved_path); - printf("%s# %s%s\n", + r = terminal_urlify_path(c->resolved_path, NULL, &urlified); + if (r < 0) + return log_error_errno(r, "Failed to urlify path \"%s\": %m", c->resolved_path); + + printf("%s# %s%s%s%s\n", ansi_highlight_blue(), + resolved ? c->original_path : "", + resolved ? " -> " : "", urlified, ansi_normal()); - f = fopen(filename, "re"); + f = fopen(FORMAT_PROC_FD_PATH(c->fd), "re"); if (!f) - return log_error_errno(errno, "Failed to open \"%s\": %m", filename); + return log_error_errno(errno, "Failed to open \"%s\": %m", c->resolved_path); for (bool continued = false;;) { _cleanup_free_ char *line = NULL; r = read_line(f, LONG_LINE_MAX, &line); if (r < 0) - return log_error_errno(r, "Failed to read \"%s\": %m", filename); + return log_error_errno(r, "Failed to read \"%s\": %m", c->resolved_path); if (r == 0) break; @@ -296,15 +305,43 @@ static int cat_file(const char *filename, bool *newline, CatFlags flags) { return 0; } +int cat_files_full(const ConfFile *file, ConfFile * const *dropins, size_t n_dropins, CatFlags flags) { + bool newline = false; + int ret = 0; + + assert(dropins || n_dropins == 0); + + if (file) + ret = cat_file(file, &newline, flags); + + FOREACH_ARRAY(i, dropins, n_dropins) + RET_GATHER(ret, cat_file(*i, &newline, flags)); + + return ret; +} + +static int cat_file_by_path(const char *p, bool *newline, CatFlags flags) { + _cleanup_(conf_file_freep) ConfFile *c = NULL; + int r; + + assert(p); + + r = conf_file_new(p, /* root = */ NULL, CHASE_MUST_BE_REGULAR, &c); + if (r < 0) + return log_error_errno(r, "Failed to chase '%s': %m", p); + + return cat_file(c, newline, flags); +} + int cat_files(const char *file, char **dropins, CatFlags flags) { bool newline = false; int ret = 0; if (file) - ret = cat_file(file, &newline, flags); + ret = cat_file_by_path(file, &newline, flags); STRV_FOREACH(path, dropins) - RET_GATHER(ret, cat_file(*path, &newline, flags)); + RET_GATHER(ret, cat_file_by_path(*path, &newline, flags)); return ret; } @@ -384,8 +421,7 @@ static int guess_type(const char **name, char ***ret_prefixes, bool *ret_is_coll } int conf_files_cat(const char *root, const char *name, CatFlags flags) { - _cleanup_strv_free_ char **dirs = NULL, **files = NULL; - _cleanup_free_ char *path = NULL; + _cleanup_strv_free_ char **dirs = NULL; char **prefixes = NULL; /* explicit initialization to appease gcc */ bool is_collection; const char *extension; @@ -416,17 +452,18 @@ int conf_files_cat(const char *root, const char *name, CatFlags flags) { } /* First locate the main config file, if any */ + _cleanup_(conf_file_freep) ConfFile *c = NULL; if (!is_collection) { STRV_FOREACH(prefix, prefixes) { - path = path_join(root, *prefix, name); - if (!path) + _cleanup_free_ char *p = path_join(*prefix, name); + if (!p) return log_oom(); - if (access(path, F_OK) == 0) + + if (conf_file_new(p, root, CHASE_MUST_BE_REGULAR, &c) >= 0) break; - path = mfree(path); } - if (!path) + if (!c) printf("%s# Main configuration file %s not found%s\n", ansi_highlight_magenta(), name, @@ -434,7 +471,10 @@ int conf_files_cat(const char *root, const char *name, CatFlags flags) { } /* Then locate the drop-ins, if any */ - r = conf_files_list_strv(&files, extension, root, 0, (const char* const*) dirs); + ConfFile **dropins = NULL; + size_t n_dropins = 0; + CLEANUP_ARRAY(dropins, n_dropins, conf_file_free_many); + r = conf_files_list_strv_full(extension, root, CONF_FILES_REGULAR | CONF_FILES_FILTER_MASKED, (const char* const*) dirs, &dropins, &n_dropins); if (r < 0) return log_error_errno(r, "Failed to query file list: %m"); @@ -442,7 +482,7 @@ int conf_files_cat(const char *root, const char *name, CatFlags flags) { if (is_collection) flags |= CAT_FORMAT_HAS_SECTIONS; - return cat_files(path, files, flags); + return cat_files_full(c, dropins, n_dropins, flags); } int terminal_tint_color(double hue, char **ret) { diff --git a/src/shared/pretty-print.h b/src/shared/pretty-print.h index fc0ba5355f..56491886fa 100644 --- a/src/shared/pretty-print.h +++ b/src/shared/pretty-print.h @@ -30,6 +30,7 @@ typedef enum CatFlags { CAT_TLDR = 1 << 2, /* Only print comments and relevant section headers */ } CatFlags; +int cat_files_full(const ConfFile *file, ConfFile * const *dropins, size_t n_dropins, CatFlags flags); int cat_files(const char *file, char **dropins, CatFlags flags); int conf_files_cat(const char *root, const char *name, CatFlags flags); diff --git a/test/units/TEST-65-ANALYZE.sh b/test/units/TEST-65-ANALYZE.sh index 7fd4543e76..5a5a9ba206 100755 --- a/test/units/TEST-65-ANALYZE.sh +++ b/test/units/TEST-65-ANALYZE.sh @@ -313,6 +313,7 @@ if [[ ! -v ASAN_OPTIONS ]]; then # check that systemd-analyze cat-config paths work in a chroot mkdir -p /tmp/root mount --bind / /tmp/root + mount -t proc proc /tmp/root/proc if mountpoint -q /usr; then mount --bind /usr /tmp/root/usr fi From a4a6e216739506153df88cbc8ac078cba4591e5f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 1 Jul 2025 04:46:41 +0900 Subject: [PATCH 07/12] udevadm: do not read udev rules files outside of the specified root directory With this change, an invalid symlink and an empty file is silently ignored. Hence, the test code is slightly updated. --- src/udev/udevadm-cat.c | 11 +++- src/udev/udevadm-util.c | 100 +++++++++++++++++------------- src/udev/udevadm-util.h | 2 +- src/udev/udevadm-verify.c | 31 +++++---- test/units/TEST-17-UDEV.verify.sh | 6 +- 5 files changed, 89 insertions(+), 61 deletions(-) diff --git a/src/udev/udevadm-cat.c b/src/udev/udevadm-cat.c index 76811ef16f..8039a44b8c 100644 --- a/src/udev/udevadm-cat.c +++ b/src/udev/udevadm-cat.c @@ -3,6 +3,7 @@ #include #include "alloc-util.h" +#include "conf-files.h" #include "log.h" #include "parse-argument.h" #include "pretty-print.h" @@ -101,11 +102,15 @@ int cat_main(int argc, char *argv[], void *userdata) { if (arg_config) return conf_files_cat(arg_root, "udev/udev.conf", arg_cat_flags); - _cleanup_strv_free_ char **files = NULL; - r = search_rules_files(strv_skip(argv, optind), arg_root, &files); + ConfFile **files = NULL; + size_t n_files = 0; + + CLEANUP_ARRAY(files, n_files, conf_file_free_many); + + r = search_rules_files(strv_skip(argv, optind), arg_root, &files, &n_files); if (r < 0) return r; /* udev rules file does not support dropin configs. So, we can safely pass multiple files as dropins. */ - return cat_files(/* file = */ NULL, /* dropins = */ files, arg_cat_flags); + return cat_files_full(/* file = */ NULL, files, n_files, arg_cat_flags); } diff --git a/src/udev/udevadm-util.c b/src/udev/udevadm-util.c index 7952d1dc14..09c0be1565 100644 --- a/src/udev/udevadm-util.c +++ b/src/udev/udevadm-util.c @@ -221,108 +221,122 @@ int udev_ping(usec_t timeout_usec, bool ignore_connection_failure) { return 1; /* received reply */ } -static int search_rules_file_in_conf_dirs(const char *s, const char *root, char ***files) { - _cleanup_free_ char *filename = NULL; +static int search_rules_file_in_conf_dirs(const char *s, const char *root, ConfFile ***files, size_t *n_files) { + _cleanup_free_ char *with_suffix = NULL; int r; assert(s); + assert(files); + assert(n_files); - if (!endswith(s, ".rules")) - filename = strjoin(s, ".rules"); - else - filename = strdup(s); - if (!filename) - return log_oom(); + if (isempty(s) || is_path(s)) + return 0; - if (!filename_is_valid(filename)) + if (!endswith(s, ".rules")) { + with_suffix = strjoin(s, ".rules"); + if (!with_suffix) + return log_oom(); + + s = with_suffix; + } + + if (!filename_is_valid(s)) return 0; STRV_FOREACH(p, CONF_PATHS_STRV("udev/rules.d")) { - _cleanup_free_ char *path = NULL, *resolved = NULL; - path = path_join(*p, filename); + _cleanup_free_ char *path = path_join(*p, s); if (!path) return log_oom(); - r = chase(path, root, CHASE_PREFIX_ROOT | CHASE_MUST_BE_REGULAR, &resolved, /* ret_fd = */ NULL); + _cleanup_(conf_file_freep) ConfFile *c = NULL; + r = conf_file_new(path, root, CHASE_MUST_BE_REGULAR, &c); if (r == -ENOENT) continue; if (r < 0) return log_error_errno(r, "Failed to chase \"%s\": %m", path); - r = strv_consume(files, TAKE_PTR(resolved)); - if (r < 0) + if (!GREEDY_REALLOC_APPEND(*files, *n_files, &c, 1)) return log_oom(); + TAKE_PTR(c); return 1; /* found */ } return 0; } -static int search_rules_file(const char *s, const char *root, char ***files) { +static int search_rules_file(const char *s, const char *root, ConfFile ***files, size_t *n_files) { int r; assert(s); assert(files); + assert(n_files); /* If the input is a file name (e.g. 99-systemd.rules), then try to find it in udev/rules.d directories. */ - r = search_rules_file_in_conf_dirs(s, root, files); + r = search_rules_file_in_conf_dirs(s, root, files, n_files); if (r != 0) return r; /* If not found, or if it is a path, then chase it. */ - struct stat st; - _cleanup_free_ char *resolved = NULL; - r = chase_and_stat(s, root, CHASE_PREFIX_ROOT, &resolved, &st); - if (r < 0) - return log_error_errno(r, "Failed to chase \"%s\": %m", s); - - r = stat_verify_regular(&st); - if (r == -EISDIR) { - _cleanup_strv_free_ char **files_in_dir = NULL; - - r = conf_files_list_strv(&files_in_dir, ".rules", root, 0, (const char* const*) STRV_MAKE_CONST(s)); - if (r < 0) - return log_error_errno(r, "Failed to enumerate rules files in '%s': %m", resolved); - - r = strv_extend_strv_consume(files, TAKE_PTR(files_in_dir), /* filter_duplicates = */ false); - if (r < 0) + _cleanup_(conf_file_freep) ConfFile *c = NULL; + r = conf_file_new(s, root, CHASE_MUST_BE_REGULAR, &c); + if (r >= 0) { + if (!GREEDY_REALLOC_APPEND(*files, *n_files, &c, 1)) return log_oom(); + TAKE_PTR(c); return 0; } - if (r < 0) - return log_error_errno(r, "'%s' is neither a regular file nor a directory: %m", resolved); - r = strv_consume(files, TAKE_PTR(resolved)); + if (r != -EISDIR) + return log_error_errno(r, "Failed to chase \"%s\": %m", s); + + /* If a directory is specified, then find all rules file in the directory. */ + ConfFile **f = NULL; + size_t n = 0; + + CLEANUP_ARRAY(f, n, conf_file_free_many); + + r = conf_files_list_strv_full(".rules", root, CONF_FILES_REGULAR, (const char* const*) STRV_MAKE_CONST(s), &f, &n); if (r < 0) + return log_error_errno(r, "Failed to enumerate rules files in '%s': %m", s); + + if (!GREEDY_REALLOC_APPEND(*files, *n_files, f, n)) return log_oom(); + TAKE_PTR(f); + n = 0; return 0; } -int search_rules_files(char * const *a, const char *root, char ***ret) { - _cleanup_strv_free_ char **files = NULL; +int search_rules_files(char * const *a, const char *root, ConfFile ***ret_files, size_t *ret_n_files) { + ConfFile **files = NULL; + size_t n_files = 0; int r; - assert(ret); + CLEANUP_ARRAY(files, n_files, conf_file_free_many); + + assert(ret_files); + assert(ret_n_files); if (strv_isempty(a)) { - r = conf_files_list_strv(&files, ".rules", root, 0, (const char* const*) CONF_PATHS_STRV("udev/rules.d")); + r = conf_files_list_strv_full(".rules", root, CONF_FILES_REGULAR | CONF_FILES_FILTER_MASKED, + (const char* const*) CONF_PATHS_STRV("udev/rules.d"), &files, &n_files); if (r < 0) return log_error_errno(r, "Failed to enumerate rules files: %m"); - if (root && strv_isempty(files)) - return log_error_errno(SYNTHETIC_ERRNO(ENOENT), "No rules files found in %s.", root); + if (root && n_files == 0) + return log_error_errno(SYNTHETIC_ERRNO(ENOENT), "No rules files found in '%s'.", root); } else STRV_FOREACH(s, a) { - r = search_rules_file(*s, root, &files); + r = search_rules_file(*s, root, &files, &n_files); if (r < 0) return r; } - *ret = TAKE_PTR(files); + *ret_files = TAKE_PTR(files); + *ret_n_files = n_files; return 0; } diff --git a/src/udev/udevadm-util.h b/src/udev/udevadm-util.h index aca4618977..906479825c 100644 --- a/src/udev/udevadm-util.h +++ b/src/udev/udevadm-util.h @@ -12,4 +12,4 @@ int parse_device_action(const char *str, sd_device_action_t *ret); int parse_resolve_name_timing(const char *str, ResolveNameTiming *ret); int parse_key_value_argument(const char *str, bool require_value, char **key, char **value); int udev_ping(usec_t timeout, bool ignore_connection_failure); -int search_rules_files(char * const *a, const char *root, char ***ret); +int search_rules_files(char * const *a, const char *root, ConfFile ***ret_files, size_t *ret_n_files); diff --git a/src/udev/udevadm-verify.c b/src/udev/udevadm-verify.c index ac46a56463..86bb6847ce 100644 --- a/src/udev/udevadm-verify.c +++ b/src/udev/udevadm-verify.c @@ -4,6 +4,7 @@ #include #include "alloc-util.h" +#include "conf-files.h" #include "errno-util.h" #include "log.h" #include "parse-argument.h" @@ -100,13 +101,16 @@ static int parse_argv(int argc, char *argv[]) { return 1; } -static int verify_rules_file(UdevRules *rules, const char *fname) { +static int verify_rules_file(UdevRules *rules, const ConfFile *c) { UdevRuleFile *file; int r; - r = udev_rules_parse_file(rules, fname, /* extra_checks = */ true, &file); + assert(rules); + assert(c); + + r = udev_rules_parse_file(rules, c->resolved_path, /* extra_checks = */ true, &file); if (r < 0) - return log_error_errno(r, "Failed to parse rules file %s: %m", fname); + return log_error_errno(r, "Failed to parse rules file %s: %m", c->original_path); if (r == 0) /* empty file. */ return 0; @@ -114,23 +118,24 @@ static int verify_rules_file(UdevRules *rules, const char *fname) { unsigned mask = (1U << LOG_ERR) | (1U << LOG_WARNING); if (issues & mask) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "%s: udev rules check failed.", fname); + "%s: udev rules check failed.", c->original_path); if (arg_style && (issues & (1U << LOG_NOTICE))) return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), - "%s: udev rules have style issues.", fname); + "%s: udev rules have style issues.", c->original_path); return 0; } -static int verify_rules(UdevRules *rules, char **files) { +static int verify_rules(UdevRules *rules, ConfFile * const *files, size_t n_files) { size_t fail_count = 0, success_count = 0; int r, ret = 0; assert(rules); + assert(files || n_files == 0); - STRV_FOREACH(fp, files) { - r = verify_rules_file(rules, *fp); + FOREACH_ARRAY(i, files, n_files) { + r = verify_rules_file(rules, *i); if (r < 0) ++fail_count; else @@ -165,10 +170,14 @@ int verify_main(int argc, char *argv[], void *userdata) { if (!rules) return -ENOMEM; - _cleanup_strv_free_ char **files = NULL; - r = search_rules_files(strv_skip(argv, optind), arg_root, &files); + ConfFile **files = NULL; + size_t n_files = 0; + + CLEANUP_ARRAY(files, n_files, conf_file_free_many); + + r = search_rules_files(strv_skip(argv, optind), arg_root, &files, &n_files); if (r < 0) return r; - return verify_rules(rules, files); + return verify_rules(rules, files, n_files); } diff --git a/test/units/TEST-17-UDEV.verify.sh b/test/units/TEST-17-UDEV.verify.sh index 243fc00f55..9580ba4ac1 100755 --- a/test/units/TEST-17-UDEV.verify.sh +++ b/test/units/TEST-17-UDEV.verify.sh @@ -132,11 +132,11 @@ assert_0 "${rules_dir}" # Directory with a loop. ln -s . "${rules_dir}/loop.rules" -assert_1 "${rules_dir}" +assert_0 "${rules_dir}" rm "${rules_dir}/loop.rules" -# Empty rules. -touch "${rules_dir}/empty.rules" +# Effectively empty rules. +echo '#' >"${rules_dir}/empty.rules" assert_0 --root="${workdir}" : >"${exo}" assert_0 --root="${workdir}" --no-summary From bdfb88423763fc4b132f69a7a73ec68841a3c34c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 4 Jul 2025 16:54:49 +0900 Subject: [PATCH 08/12] TEST-17-UDEV: conditionalize test cases for testuser Then, we can also run the test script in our local machine. --- test/units/TEST-17-UDEV.verify.sh | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/units/TEST-17-UDEV.verify.sh b/test/units/TEST-17-UDEV.verify.sh index 9580ba4ac1..b456fdef72 100755 --- a/test/units/TEST-17-UDEV.verify.sh +++ b/test/units/TEST-17-UDEV.verify.sh @@ -315,8 +315,10 @@ if ! getent passwd 12345 >/dev/null; then test_syntax_error 'OWNER="12345"' "Unknown user '12345', ignoring." fi # regular user -test_syntax_error 'OWNER="testuser"' "User 'testuser' is not a system user, ignoring." -test_syntax_error "OWNER=\"$(id -u testuser)\"" "User '$(id -u testuser)' is not a system user, ignoring." +if getent passwd testuser >/dev/null; then + test_syntax_error 'OWNER="testuser"' "User 'testuser' is not a system user, ignoring." + test_syntax_error "OWNER=\"$(id -u testuser)\"" "User '$(id -u testuser)' is not a system user, ignoring." +fi test_syntax_error 'GROUP{a}="b"' 'Invalid attribute for GROUP.' test_syntax_error 'GROUP-="b"' 'Invalid operator for GROUP.' test_syntax_error 'GROUP!="b"' 'Invalid operator for GROUP.' @@ -345,8 +347,10 @@ if ! getent group 12345 >/dev/null; then test_syntax_error 'GROUP="12345"' "Unknown group '12345', ignoring." fi # regular group -test_syntax_error 'GROUP="testuser"' "Group 'testuser' is not a system group, ignoring." -test_syntax_error "GROUP=\"$(id -g testuser)\"" "Group '$(id -g testuser)' is not a system group, ignoring." +if getent group testuser >/dev/null; then + test_syntax_error 'GROUP="testuser"' "Group 'testuser' is not a system group, ignoring." + test_syntax_error "GROUP=\"$(id -g testuser)\"" "Group '$(id -g testuser)' is not a system group, ignoring." +fi test_syntax_error 'MODE{a}="b"' 'Invalid attribute for MODE.' test_syntax_error 'MODE-="b"' 'Invalid operator for MODE.' test_syntax_error 'MODE!="b"' 'Invalid operator for MODE.' From ab1333b2b7eefb68e3fe679c8cb60d17b81f556d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 1 Jul 2025 11:05:18 +0900 Subject: [PATCH 09/12] udev-rules: do not read udev rules files outside of specified root directory --- src/udev/fuzz-udev-rules.c | 9 ++++++- src/udev/udev-rules.c | 54 ++++++++++++++++++-------------------- src/udev/udev-rules.h | 2 +- src/udev/udevadm-verify.c | 4 +-- 4 files changed, 35 insertions(+), 34 deletions(-) diff --git a/src/udev/fuzz-udev-rules.c b/src/udev/fuzz-udev-rules.c index da94cdaf7e..34567471e8 100644 --- a/src/udev/fuzz-udev-rules.c +++ b/src/udev/fuzz-udev-rules.c @@ -2,8 +2,11 @@ #include +#include "chase.h" +#include "conf-files.h" #include "fd-util.h" #include "fuzz.h" +#include "tests.h" #include "tmpfile-util.h" #include "udev-rules.h" @@ -24,7 +27,11 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { fflush(f); assert_se(rules = udev_rules_new(RESOLVE_NAME_EARLY)); - r = udev_rules_parse_file(rules, filename, /* extra_checks = */ false, NULL); + + _cleanup_(conf_file_freep) ConfFile *c = NULL; + ASSERT_OK(conf_file_new(filename, /* root = */ NULL, CHASE_MUST_BE_REGULAR, &c)); + + r = udev_rules_parse_file(rules, c, /* extra_checks = */ false, /* ret = */ NULL); log_info_errno(r, "Parsing %s: %m", filename); assert_se(r >= 0 || /* OK */ r == -ENOBUFS); /* line length exceeded */ diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index a6da5074ac..b7fc0a675c 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -1645,46 +1645,34 @@ static void udev_check_rule_line(UdevRuleLine *line) { udev_check_conflicts_duplicates(line); } -int udev_rules_parse_file(UdevRules *rules, const char *filename, bool extra_checks, UdevRuleFile **ret) { +int udev_rules_parse_file(UdevRules *rules, const ConfFile *c, bool extra_checks, UdevRuleFile **ret) { _cleanup_(udev_rule_file_freep) UdevRuleFile *rule_file = NULL; _cleanup_free_ char *name = NULL; _cleanup_fclose_ FILE *f = NULL; - struct stat st; int r; assert(rules); - assert(filename); + assert(c); + assert(c->fd >= 0); + assert(c->original_path); - f = fopen(filename, "re"); + f = fopen(FORMAT_PROC_FD_PATH(c->fd), "re"); if (!f) { if (extra_checks) return -errno; - if (errno == ENOENT) - return 0; - - return log_warning_errno(errno, "Failed to open %s, ignoring: %m", filename); + return log_warning_errno(errno, "Failed to open %s, ignoring: %m", c->original_path); } - if (fstat(fileno(f), &st) < 0) - return log_warning_errno(errno, "Failed to stat %s, ignoring: %m", filename); - - if (null_or_empty(&st)) { - log_debug("Skipping empty file: %s", filename); - if (ret) - *ret = NULL; - return 0; - } - - r = hashmap_put_stats_by_path(&rules->stats_by_path, filename, &st); + r = hashmap_put_stats_by_path(&rules->stats_by_path, c->original_path, &c->st); if (r < 0) - return log_warning_errno(r, "Failed to save stat for %s, ignoring: %m", filename); + return log_warning_errno(r, "Failed to save stat for %s, ignoring: %m", c->original_path); - (void) fd_warn_permissions(filename, fileno(f)); + (void) stat_warn_permissions(c->original_path, &c->st); - log_debug("Reading rules file: %s", filename); + log_debug("Reading rules file: %s", c->original_path); - name = strdup(filename); + name = strdup(c->original_path); if (!name) return log_oom(); @@ -1775,7 +1763,7 @@ int udev_rules_parse_file(UdevRules *rules, const char *filename, bool extra_che *ret = rule_file; TAKE_PTR(rule_file); - return 1; + return 0; } unsigned udev_rule_file_get_issues(UdevRuleFile *rule_file) { @@ -1800,7 +1788,7 @@ UdevRules* udev_rules_new(ResolveNameTiming resolve_name_timing) { int udev_rules_load(UdevRules **ret_rules, ResolveNameTiming resolve_name_timing, char * const *extra) { _cleanup_(udev_rules_freep) UdevRules *rules = NULL; - _cleanup_strv_free_ char **files = NULL, **directories = NULL; + _cleanup_strv_free_ char **directories = NULL; int r; rules = udev_rules_new(resolve_name_timing); @@ -1817,14 +1805,22 @@ int udev_rules_load(UdevRules **ret_rules, ResolveNameTiming resolve_name_timing if (r < 0) return r; - r = conf_files_list_strv(&files, ".rules", NULL, 0, (const char* const*) directories); + ConfFile **files = NULL; + size_t n_files = 0; + + CLEANUP_ARRAY(files, n_files, conf_file_free_many); + + r = conf_files_list_strv_full(".rules", /* root = */ NULL, CONF_FILES_REGULAR | CONF_FILES_FILTER_MASKED, + (const char* const*) directories, &files, &n_files); if (r < 0) return log_debug_errno(r, "Failed to enumerate rules files: %m"); - STRV_FOREACH(f, files) { - r = udev_rules_parse_file(rules, *f, /* extra_checks = */ false, NULL); + FOREACH_ARRAY(i, files, n_files) { + ConfFile *c = *i; + + r = udev_rules_parse_file(rules, c, /* extra_checks = */ false, /* ret = */ NULL); if (r < 0) - log_debug_errno(r, "Failed to read rules file %s, ignoring: %m", *f); + log_debug_errno(r, "Failed to read rules file '%s', ignoring: %m", c->original_path); } *ret_rules = TAKE_PTR(rules); diff --git a/src/udev/udev-rules.h b/src/udev/udev-rules.h index d0ed2adf00..19d6e7e0e1 100644 --- a/src/udev/udev-rules.h +++ b/src/udev/udev-rules.h @@ -5,7 +5,7 @@ #include "udev-forward.h" int udev_rule_parse_value(char *str, char **ret_value, char **ret_endpos, bool *ret_is_case_insensitive); -int udev_rules_parse_file(UdevRules *rules, const char *filename, bool extra_checks, UdevRuleFile **ret); +int udev_rules_parse_file(UdevRules *rules, const ConfFile *c, bool extra_checks, UdevRuleFile **ret); unsigned udev_rule_file_get_issues(UdevRuleFile *rule_file); UdevRules* udev_rules_new(ResolveNameTiming resolve_name_timing); int udev_rules_load(UdevRules **ret_rules, ResolveNameTiming resolve_name_timing, char * const *extra); diff --git a/src/udev/udevadm-verify.c b/src/udev/udevadm-verify.c index 86bb6847ce..15406357b5 100644 --- a/src/udev/udevadm-verify.c +++ b/src/udev/udevadm-verify.c @@ -108,11 +108,9 @@ static int verify_rules_file(UdevRules *rules, const ConfFile *c) { assert(rules); assert(c); - r = udev_rules_parse_file(rules, c->resolved_path, /* extra_checks = */ true, &file); + r = udev_rules_parse_file(rules, c, /* extra_checks = */ true, &file); if (r < 0) return log_error_errno(r, "Failed to parse rules file %s: %m", c->original_path); - if (r == 0) /* empty file. */ - return 0; unsigned issues = udev_rule_file_get_issues(file); unsigned mask = (1U << LOG_ERR) | (1U << LOG_WARNING); From 4d000c48532ef2856cf91b8731a45138df7cf777 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 1 Jul 2025 11:12:59 +0900 Subject: [PATCH 10/12] hwdb-util: coding style update - use 'r' for storing results, - use RET_GATHER(). --- src/shared/hwdb-util.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/shared/hwdb-util.c b/src/shared/hwdb-util.c index 10d6e8ee48..3b61b6f070 100644 --- a/src/shared/hwdb-util.c +++ b/src/shared/hwdb-util.c @@ -7,6 +7,7 @@ #include "alloc-util.h" #include "conf-files.h" +#include "errno-util.h" #include "fd-util.h" #include "fileio.h" #include "fs-util.h" @@ -574,7 +575,7 @@ int hwdb_update(const char *root, const char *hwdb_bin_dir, bool strict, bool co _cleanup_(trie_freep) struct trie *trie = NULL; _cleanup_strv_free_ char **files = NULL; uint16_t file_priority = 1; - int r = 0, err; + int r, ret = 0; /* The argument 'compat' controls the format version of database. If false, then hwdb.bin will be * created with additional information such that priority, line number, and filename of database @@ -601,9 +602,9 @@ int hwdb_update(const char *root, const char *hwdb_bin_dir, bool strict, bool co trie->nodes_count++; - err = conf_files_list_strv(&files, ".hwdb", root, 0, conf_file_dirs); - if (err < 0) - return log_error_errno(err, "Failed to enumerate hwdb files: %m"); + r = conf_files_list_strv(&files, ".hwdb", root, 0, conf_file_dirs); + if (r < 0) + return log_error_errno(r, "Failed to enumerate hwdb files: %m"); if (strv_isempty(files)) { if (unlink(hwdb_bin) < 0) { @@ -619,9 +620,7 @@ int hwdb_update(const char *root, const char *hwdb_bin_dir, bool strict, bool co STRV_FOREACH(f, files) { log_debug("Reading file \"%s\"", *f); - err = import_file(trie, *f, file_priority++, compat); - if (err < 0 && strict) - r = err; + RET_GATHER(ret, import_file(trie, *f, file_priority++, compat)); } strbuf_complete(trie->strings); @@ -641,15 +640,15 @@ int hwdb_update(const char *root, const char *hwdb_bin_dir, bool strict, bool co trie->strings->dedup_len, trie->strings->dedup_count); (void) mkdir_parents_label(hwdb_bin, 0755); - err = trie_store(trie, hwdb_bin, compat); - if (err < 0) - return log_error_errno(err, "Failed to write database %s: %m", hwdb_bin); + r = trie_store(trie, hwdb_bin, compat); + if (r < 0) + return log_error_errno(r, "Failed to write database %s: %m", hwdb_bin); - err = label_fix(hwdb_bin, 0); - if (err < 0) - return err; + r = label_fix(hwdb_bin, 0); + if (r < 0) + return r; - return r; + return strict ? ret : 0; } int hwdb_query(const char *modalias, const char *root) { From 683efcf649b6c422c4a929aec921856a55022cae Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 1 Jul 2025 11:21:09 +0900 Subject: [PATCH 11/12] hwdb-util: do not read hwdb files outside of specified root directory --- src/shared/hwdb-util.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/shared/hwdb-util.c b/src/shared/hwdb-util.c index 3b61b6f070..1eab1fb881 100644 --- a/src/shared/hwdb-util.c +++ b/src/shared/hwdb-util.c @@ -458,7 +458,7 @@ static int insert_data(struct trie *trie, char **match_list, char *line, const c return 0; } -static int import_file(struct trie *trie, const char *filename, uint16_t file_priority, bool compat) { +static int import_file(struct trie *trie, int fd, const char *filename, uint16_t file_priority, bool compat) { enum { HW_NONE, HW_MATCH, @@ -469,7 +469,11 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr uint32_t line_number = 0; int r; - f = fopen(filename, "re"); + assert(trie); + assert(fd >= 0); + assert(filename); + + f = fopen(FORMAT_PROC_FD_PATH(fd), "re"); if (!f) return -errno; @@ -573,7 +577,6 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr int hwdb_update(const char *root, const char *hwdb_bin_dir, bool strict, bool compat) { _cleanup_free_ char *hwdb_bin = NULL; _cleanup_(trie_freep) struct trie *trie = NULL; - _cleanup_strv_free_ char **files = NULL; uint16_t file_priority = 1; int r, ret = 0; @@ -602,11 +605,16 @@ int hwdb_update(const char *root, const char *hwdb_bin_dir, bool strict, bool co trie->nodes_count++; - r = conf_files_list_strv(&files, ".hwdb", root, 0, conf_file_dirs); + ConfFile **files = NULL; + size_t n_files = 0; + + CLEANUP_ARRAY(files, n_files, conf_file_free_many); + + r = conf_files_list_strv_full(".hwdb", root, CONF_FILES_REGULAR | CONF_FILES_FILTER_MASKED, conf_file_dirs, &files, &n_files); if (r < 0) return log_error_errno(r, "Failed to enumerate hwdb files: %m"); - if (strv_isempty(files)) { + if (n_files == 0) { if (unlink(hwdb_bin) < 0) { if (errno != ENOENT) return log_error_errno(errno, "Failed to remove compiled hwdb database %s: %m", hwdb_bin); @@ -618,9 +626,11 @@ int hwdb_update(const char *root, const char *hwdb_bin_dir, bool strict, bool co return 0; } - STRV_FOREACH(f, files) { - log_debug("Reading file \"%s\"", *f); - RET_GATHER(ret, import_file(trie, *f, file_priority++, compat)); + FOREACH_ARRAY(i, files, n_files) { + ConfFile *c = *i; + + log_debug("Reading file \"%s\" -> \"%s\"", c->original_path, c->resolved_path); + RET_GATHER(ret, import_file(trie, c->fd, c->original_path, file_priority++, compat)); } strbuf_complete(trie->strings); From 1e29a967c76106db6980842b6bfb4b10f5b829c9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 1 Jul 2025 11:33:22 +0900 Subject: [PATCH 12/12] catalog: do not read catalog files outside of specified root directory --- src/fuzz/fuzz-catalog.c | 2 +- src/libsystemd/sd-journal/catalog.c | 23 +++++++++++++++-------- src/libsystemd/sd-journal/catalog.h | 2 +- src/libsystemd/sd-journal/test-catalog.c | 2 +- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/fuzz/fuzz-catalog.c b/src/fuzz/fuzz-catalog.c index 965828827a..3c13db2c9b 100644 --- a/src/fuzz/fuzz-catalog.c +++ b/src/fuzz/fuzz-catalog.c @@ -19,7 +19,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { assert_se(fd >= 0); assert_se(write(fd, data, size) == (ssize_t) size); - (void) catalog_import_file(&h, name); + (void) catalog_import_file(&h, fd, name); return 0; } diff --git a/src/libsystemd/sd-journal/catalog.c b/src/libsystemd/sd-journal/catalog.c index 3ca3b2dab4..ec0445f122 100644 --- a/src/libsystemd/sd-journal/catalog.c +++ b/src/libsystemd/sd-journal/catalog.c @@ -263,7 +263,7 @@ static int catalog_entry_lang( return strdup_to(ret, t); } -int catalog_import_file(OrderedHashmap **h, const char *path) { +int catalog_import_file(OrderedHashmap **h, int fd, const char *path) { _cleanup_fclose_ FILE *f = NULL; _cleanup_free_ char *payload = NULL; size_t payload_size = 0; @@ -274,9 +274,10 @@ int catalog_import_file(OrderedHashmap **h, const char *path) { int r; assert(h); + assert(fd >= 0); assert(path); - f = fopen(path, "re"); + f = fopen(FORMAT_PROC_FD_PATH(fd), "re"); if (!f) return log_error_errno(errno, "Failed to open file %s: %m", path); @@ -449,17 +450,23 @@ int catalog_update(const char *database, const char *root, const char* const *di if (!dirs) dirs = catalog_file_dirs; - _cleanup_strv_free_ char **files = NULL; - r = conf_files_list_strv(&files, ".catalog", root, 0, dirs); + ConfFile **files = NULL; + size_t n_files = 0; + + CLEANUP_ARRAY(files, n_files, conf_file_free_many); + + r = conf_files_list_strv_full(".catalog", root, CONF_FILES_REGULAR | CONF_FILES_FILTER_MASKED, dirs, &files, &n_files); if (r < 0) return log_error_errno(r, "Failed to get catalog files: %m"); _cleanup_ordered_hashmap_free_ OrderedHashmap *h = NULL; - STRV_FOREACH(f, files) { - log_debug("Reading file '%s'", *f); - r = catalog_import_file(&h, *f); + FOREACH_ARRAY(i, files, n_files) { + ConfFile *c = *i; + + log_debug("Reading file: '%s' -> '%s'", c->original_path, c->resolved_path); + r = catalog_import_file(&h, c->fd, c->original_path); if (r < 0) - return log_error_errno(r, "Failed to import file '%s': %m", *f); + return log_error_errno(r, "Failed to import file '%s': %m", c->original_path); } if (ordered_hashmap_isempty(h)) { diff --git a/src/libsystemd/sd-journal/catalog.h b/src/libsystemd/sd-journal/catalog.h index c2a20aaddb..b91bd188b5 100644 --- a/src/libsystemd/sd-journal/catalog.h +++ b/src/libsystemd/sd-journal/catalog.h @@ -3,7 +3,7 @@ #include "forward.h" -int catalog_import_file(OrderedHashmap **h, const char *path); +int catalog_import_file(OrderedHashmap **h, int fd, const char *path); int catalog_update(const char *database, const char *root, const char* const *dirs); int catalog_get(const char *database, sd_id128_t id, char **ret_text); int catalog_list(FILE *f, const char *database, bool oneline); diff --git a/src/libsystemd/sd-journal/test-catalog.c b/src/libsystemd/sd-journal/test-catalog.c index d86fd533f7..51e113b3fc 100644 --- a/src/libsystemd/sd-journal/test-catalog.c +++ b/src/libsystemd/sd-journal/test-catalog.c @@ -31,7 +31,7 @@ static OrderedHashmap* test_import(const char* contents, ssize_t size, int code) assert_se(fd >= 0); assert_se(write(fd, contents, size) == size); - assert_se(catalog_import_file(&h, name) == code); + assert_se(catalog_import_file(&h, fd, name) == code); return h; }