From acdba85e0e2bf229f356ad09a2b880e89224715d Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Thu, 2 Mar 2023 08:00:00 +0000 Subject: [PATCH] udevadm: introduce new 'verify' command We seem to have no tool to verify udev rule files. There is a simple udev rules syntax checker in the tree, test/rule-syntax-check.py, but it is too simple to detect less trivial issues not detected by udev, e.g. redundant comparisons (#26593) or labels without references. Such a tool would be beneficial not only for maintaining udev rules distributed along with udev, but also and even more so for maintaining third party udev rules that are more likely to have issues with syntax and semantic correctness. Implement a udev rules syntax and semantics checker in the form of 'udevadm verify [OPTIONS] FILE...' command that is based on udev_rules_parse_file() interface and would apply further checks on top of it in subsequent commits. Resolves: #26606 --- man/udevadm.xml | 36 ++++++++++ shell-completion/bash/udevadm | 24 ++++++- shell-completion/zsh/_udevadm | 9 +++ src/udev/meson.build | 1 + src/udev/udev-rules.c | 10 +++ src/udev/udev-rules.h | 1 + src/udev/udevadm-verify.c | 125 ++++++++++++++++++++++++++++++++++ src/udev/udevadm.c | 2 + src/udev/udevadm.h | 1 + 9 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 src/udev/udevadm-verify.c diff --git a/man/udevadm.xml b/man/udevadm.xml index 33af1550213..95b8da2b0a4 100644 --- a/man/udevadm.xml +++ b/man/udevadm.xml @@ -48,6 +48,11 @@ udevadm test-builtin options command devpath + + udevadm verify + options + file + udevadm wait options device|syspath @@ -737,6 +742,37 @@ + + udevadm verify + <arg choice="opt"><replaceable>options</replaceable></arg> + <arg choice="plain" rep="repeat"><replaceable>file</replaceable></arg> + … + + + Verify syntactic and semantic correctness of udev rules files. + + Positional arguments should be used to specify one or more files to check. + + The exit status is 0 if all specified udev rules files + are syntactically and semantically correct, and a non-zero error code otherwise. + + + + + + + Specify when udevadm should resolve names of users + and groups. When set to early (the + default), names will be resolved when the rules are + parsed. When set to never, names will + never be resolved. + + + + + + + udevadm wait <arg choice="opt"><replaceable>options</replaceable></arg> diff --git a/shell-completion/bash/udevadm b/shell-completion/bash/udevadm index 99d4aa55c98..b6e14e1d36e 100644 --- a/shell-completion/bash/udevadm +++ b/shell-completion/bash/udevadm @@ -64,11 +64,12 @@ _udevadm() { [MONITOR_ARG]='-s --subsystem-match -t --tag-match' [TEST]='-a --action -N --resolve-names' [TEST_BUILTIN]='-a --action' + [VERIFY]='-N --resolve-names' [WAIT]='-t --timeout --initialized=no --removed --settle' [LOCK]='-t --timeout -d --device -b --backing -p --print' ) - local verbs=(info trigger settle control monitor test-builtin test wait lock) + local verbs=(info trigger settle control monitor test-builtin test verify wait lock) local builtins=(blkid btrfs hwdb input_id keyboard kmod net_id net_setup_link path_id usb_id uaccess) for ((i=0; i < COMP_CWORD; i++)); do @@ -247,6 +248,27 @@ _udevadm() { fi ;; + 'verify') + if __contains_word "$prev" ${OPTS[VERIFY]}; then + case $prev in + -N|--resolve-names) + comps='early never' + ;; + *) + comps='' + ;; + esac + COMPREPLY=( $(compgen -W '$comps' -- "$cur") ) + return 0 + fi + + if [[ $cur = -* ]]; then + comps="${OPTS[COMMON]} ${OPTS[VERIFY]}" + else + comps=$( compgen -A file -- "$cur" ) + fi + ;; + 'wait') if __contains_word "$prev" ${OPTS[WAIT]}; then case $prev in diff --git a/shell-completion/zsh/_udevadm b/shell-completion/zsh/_udevadm index f7c3384eae2..074d367a9de 100644 --- a/shell-completion/zsh/_udevadm +++ b/shell-completion/zsh/_udevadm @@ -104,6 +104,14 @@ _udevadm_test-builtin(){ fi } +(( $+functions[_udevadm_verify] )) || +_udevadm_verify(){ + _arguments \ + {-N+,--resolve-names=}'[When to resolve names.]:resolve:(early never)' \ + {-h,--help}'[Print help text.]' \ + '*::files:_files' +} + (( $+functions[_udevadm_wait] )) || _udevadm_wait(){ _arguments \ @@ -154,6 +162,7 @@ _udevadm_commands(){ 'monitor:listen to kernel and udev events' 'test:test an event run' 'test-builtin:test a built-in command' + 'verify:verify udev rules files' 'wait:wait for devices or device symlinks being created' 'lock:lock a block device and run a comand' ) diff --git a/src/udev/meson.build b/src/udev/meson.build index 3f4ab00ac95..1cac581e7fe 100644 --- a/src/udev/meson.build +++ b/src/udev/meson.build @@ -12,6 +12,7 @@ udevadm_sources = files( 'udevadm-test-builtin.c', 'udevadm-trigger.c', 'udevadm-util.c', + 'udevadm-verify.c', 'udevadm-wait.c', 'udevd.c', ) diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 840448d65c3..c2e98e9a9e8 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -1321,6 +1321,16 @@ int udev_rules_parse_file(UdevRules *rules, const char *filename) { return 0; } +unsigned udev_check_current_rule_file(UdevRules *rules) { + assert(rules); + + UdevRuleFile *rule_file = rules->current_file; + if (!rule_file) + return 0; + + return rule_file->issues; +} + UdevRules* udev_rules_new(ResolveNameTiming resolve_name_timing) { assert(resolve_name_timing >= 0 && resolve_name_timing < _RESOLVE_NAME_TIMING_MAX); diff --git a/src/udev/udev-rules.h b/src/udev/udev-rules.h index 860fe7c8e4e..44f30e1c137 100644 --- a/src/udev/udev-rules.h +++ b/src/udev/udev-rules.h @@ -18,6 +18,7 @@ typedef enum { } UdevRuleEscapeType; int udev_rules_parse_file(UdevRules *rules, const char *filename); +unsigned udev_check_current_rule_file(UdevRules *rules); UdevRules* udev_rules_new(ResolveNameTiming resolve_name_timing); int udev_rules_load(UdevRules **ret_rules, ResolveNameTiming resolve_name_timing); UdevRules *udev_rules_free(UdevRules *rules); diff --git a/src/udev/udevadm-verify.c b/src/udev/udevadm-verify.c new file mode 100644 index 00000000000..8819b07238e --- /dev/null +++ b/src/udev/udevadm-verify.c @@ -0,0 +1,125 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <errno.h> +#include <getopt.h> +#include <stddef.h> +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> + +#include "log.h" +#include "pretty-print.h" +#include "strv.h" +#include "udev-rules.h" +#include "udevadm.h" + +static ResolveNameTiming arg_resolve_name_timing = RESOLVE_NAME_EARLY; + +static int help(void) { + _cleanup_free_ char *link = NULL; + int r; + + r = terminal_urlify_man("udevadm", "8", &link); + if (r < 0) + return log_oom(); + + printf("%s verify [OPTIONS] FILE...\n" + "\n%sVerify udev rules files.%s\n\n" + " -h --help Show this help\n" + " -V --version Show package version\n" + " -N --resolve-names=early|never When to resolve names\n" + "\nSee the %s for details.\n", + program_invocation_short_name, + ansi_highlight(), + ansi_normal(), + link); + + return 0; +} + +static int parse_argv(int argc, char *argv[]) { + static const struct option options[] = { + { "help", no_argument, NULL, 'h' }, + { "version", no_argument, NULL, 'V' }, + { "resolve-names", required_argument, NULL, 'N' }, + {} + }; + + int c; + + assert(argc >= 0); + assert(argv); + + while ((c = getopt_long(argc, argv, "hVN:", options, NULL)) >= 0) + switch (c) { + case 'h': + return help(); + case 'V': + return print_version(); + case 'N': + arg_resolve_name_timing = resolve_name_timing_from_string(optarg); + if (arg_resolve_name_timing < 0) + return log_error_errno(arg_resolve_name_timing, + "--resolve-names= takes \"early\" or \"never\""); + /* + * In the verifier "late" has the effect of "never", + * and "never" would generate irrelevant diagnostics, + * so map "never" to "late". + */ + if (arg_resolve_name_timing == RESOLVE_NAME_NEVER) + arg_resolve_name_timing = RESOLVE_NAME_LATE; + break; + case '?': + return -EINVAL; + default: + assert_not_reached(); + } + + if (optind == argc) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No rules file specified."); + + return 1; +} + +static int verify_rules_file(UdevRules *rules, const char *fname) { + int r; + + r = udev_rules_parse_file(rules, fname); + if (r < 0) + return log_error_errno(r, "Failed to parse rules file %s: %m", fname); + + unsigned issues = udev_check_current_rule_file(rules); + unsigned mask = (1U << LOG_ERR) | (1U << LOG_WARNING); + if (issues & mask) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: udev rules check failed", fname); + + return 0; +} + +static int verify_rules(UdevRules *rules, char **files) { + int r, rv = 0; + + STRV_FOREACH(fp, files) { + r = verify_rules_file(rules, *fp); + if (r < 0 && rv >= 0) + rv = r; + } + + return rv; +} + +int verify_main(int argc, char *argv[], void *userdata) { + _cleanup_(udev_rules_freep) UdevRules *rules = NULL; + int r; + + r = parse_argv(argc, argv); + if (r <= 0) + return r; + + rules = udev_rules_new(arg_resolve_name_timing); + if (!rules) + return -ENOMEM; + + return verify_rules(rules, strv_skip(argv, optind)); +} diff --git a/src/udev/udevadm.c b/src/udev/udevadm.c index b742c1a2873..273d3c5b294 100644 --- a/src/udev/udevadm.c +++ b/src/udev/udevadm.c @@ -25,6 +25,7 @@ static int help(void) { { "monitor", "Listen to kernel and udev events" }, { "test", "Test an event run" }, { "test-builtin", "Test a built-in command" }, + { "verify", "Verify udev rules files" }, { "wait", "Wait for device or device symlink" }, { "lock", "Lock a block device" }, }; @@ -104,6 +105,7 @@ static int udevadm_main(int argc, char *argv[]) { { "test-builtin", VERB_ANY, VERB_ANY, 0, builtin_main }, { "wait", VERB_ANY, VERB_ANY, 0, wait_main }, { "lock", VERB_ANY, VERB_ANY, 0, lock_main }, + { "verify", VERB_ANY, VERB_ANY, 0, verify_main }, { "version", VERB_ANY, VERB_ANY, 0, version_main }, { "help", VERB_ANY, VERB_ANY, 0, help_main }, {} diff --git a/src/udev/udevadm.h b/src/udev/udevadm.h index 417611affe3..7920a70d5b1 100644 --- a/src/udev/udevadm.h +++ b/src/udev/udevadm.h @@ -13,6 +13,7 @@ int monitor_main(int argc, char *argv[], void *userdata); int hwdb_main(int argc, char *argv[], void *userdata); int test_main(int argc, char *argv[], void *userdata); int builtin_main(int argc, char *argv[], void *userdata); +int verify_main(int argc, char *argv[], void *userdata); int wait_main(int argc, char *argv[], void *userdata); int lock_main(int argc, char *argv[], void *userdata);