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.

diff --git a/src/udev/udevadm-cat.c b/src/udev/udevadm-cat.c
index 76811ef16fb89fd4ab8f6954b8be58670f07a271..8039a44b8cb89804fb56fa7af508ef87f0948d32 100644 (file)
--- a/src/udev/udevadm-cat.c
+++ b/src/udev/udevadm-cat.c
@@ -3,6 +3,7 @@
 #include <getopt.h>
 
 #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 7952d1dc149c156e1557cea038a014bd5d71f75c..09c0be1565e57194366977f16a6e9cd47b9a5249 100644 (file)
--- 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 (!endswith(s, ".rules")) {
+                with_suffix = strjoin(s, ".rules");
+                if (!with_suffix)
+                        return log_oom();
+
+                s = with_suffix;
+        }
 
-        if (!filename_is_valid(filename))
+        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 aca461897756677dfed685b3287f50c07c524f33..906479825ca3fc3468a1c44f15b35d2fffc04f60 100644 (file)
--- 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 ac46a56463d0c76973cd19b5f5235e665f9be161..86bb6847ce6927c90f4868af498f32250509e4bc 100644 (file)
--- a/src/udev/udevadm-verify.c
+++ b/src/udev/udevadm-verify.c
@@ -4,6 +4,7 @@
 #include <stdio.h>
 
 #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 243fc00f55039941f487da9ef78eb4f28c682ecc..9580ba4ac1ee21c2f671601e9acd1f76622b80ac 100755 (executable)
--- 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