From 0b1ee3e31159086d1e51b8d12f6626601c2578ca Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Thu, 9 Mar 2023 08:00:00 +0000 Subject: [PATCH 1/7] testsuite-17.11.sh: robustify unknown user/group checks Use certainly invalid user/group names in the tests that check unknown user/group diagnostics. --- test/units/testsuite-17.11.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/units/testsuite-17.11.sh b/test/units/testsuite-17.11.sh index ed5da16c07..6a6d873bde 100755 --- a/test/units/testsuite-17.11.sh +++ b/test/units/testsuite-17.11.sh @@ -147,12 +147,12 @@ test_syntax_error 'OWNER{a}="b"' 'Invalid attribute for OWNER.' test_syntax_error 'OWNER-="b"' 'Invalid operator for OWNER.' test_syntax_error 'OWNER!="b"' 'Invalid operator for OWNER.' test_syntax_error 'OWNER+="0"' "OWNER key takes '=' or ':=' operator, assuming '='." -test_syntax_error 'OWNER="nosuchuser"' "Unknown user 'nosuchuser', ignoring" +test_syntax_error 'OWNER=":nosuchuser:"' "Unknown user ':nosuchuser:', ignoring" 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.' test_syntax_error 'GROUP+="0"' "GROUP key takes '=' or ':=' operator, assuming '='." -test_syntax_error 'GROUP="nosuchuser"' "Unknown group 'nosuchuser', ignoring" +test_syntax_error 'GROUP=":nosuchgroup:"' "Unknown group ':nosuchgroup:', ignoring" 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 99f24451710ed86021c331dd2e90ca35497faa93 Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Thu, 9 Mar 2023 08:00:00 +0000 Subject: [PATCH 2/7] testsuite-17.11.sh: create all files in a temporary directory Make sure the test would not collide with anything else by moving all files it created into a temporary directory. --- test/units/testsuite-17.11.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/units/testsuite-17.11.sh b/test/units/testsuite-17.11.sh index 6a6d873bde..ad0158921c 100755 --- a/test/units/testsuite-17.11.sh +++ b/test/units/testsuite-17.11.sh @@ -5,6 +5,16 @@ set -o pipefail # Test for udevadm verify. +cleanup() { + cd / + rm -rf "${workdir}" + workdir= +} + +workdir="$(mktemp -d)" +trap cleanup EXIT +cd "${workdir}" + udevadm verify -h udevadm verify --help udevadm verify -V From 197f695a75d842eb1f9b0d42ee86f71038f200a0 Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Thu, 9 Mar 2023 08:00:00 +0000 Subject: [PATCH 3/7] testsuite-17.11.sh: prepare to test udevadm verify --root --- test/units/testsuite-17.11.sh | 153 +++++++++++++++++++++------------- 1 file changed, 94 insertions(+), 59 deletions(-) diff --git a/test/units/testsuite-17.11.sh b/test/units/testsuite-17.11.sh index ad0158921c..4abcae60a5 100755 --- a/test/units/testsuite-17.11.sh +++ b/test/units/testsuite-17.11.sh @@ -5,6 +5,9 @@ set -o pipefail # Test for udevadm verify. +# shellcheck source=test/units/assert.sh +. "$(dirname "$0")"/assert.sh + cleanup() { cd / rm -rf "${workdir}" @@ -15,70 +18,104 @@ workdir="$(mktemp -d)" trap cleanup EXIT cd "${workdir}" -udevadm verify -h -udevadm verify --help -udevadm verify -V -udevadm verify --version -udevadm verify /dev/null +test_number=0 +rules= +exp= +err= +next_test_number() { + : $((++test_number)) + + local num_str + num_str=$(printf %05d "${test_number}") + + rules="sample-${num_str}.rules" + exp="sample-${num_str}.exp" + err="sample-${num_str}.err" +} + +assert_0() { + assert_rc 0 udevadm verify "$@" + next_test_number +} + +assert_1() { + if [ -f "${exp}" ]; then + set +e + udevadm verify "$@" 2>"${err}" + assert_eq "$?" 1 + set -e + diff "${exp}" "${err}" + else + set +e + udevadm verify "$@" + assert_eq "$?" 1 + set -e + fi + next_test_number +} + +assert_0 -h +assert_0 --help +assert_0 -V +assert_0 --version +assert_0 /dev/null # No rules file specified. -(! udevadm verify) +assert_1 # unrecognized option '--unknown' -(! udevadm verify --unknown) +assert_1 --unknown # option requires an argument -- 'N' -(! udevadm verify -N) +assert_1 -N # --resolve-names= takes "early" or "never" -(! udevadm verify -N now) +assert_1 -N now # option '--resolve-names' requires an argument -(! udevadm verify --resolve-names) +assert_1 --resolve-names # --resolve-names= takes "early" or "never" -(! udevadm verify --resolve-names=now) +assert_1 --resolve-names=now # Failed to parse rules file .: Is a directory -(! udevadm verify .) +assert_1 . # Failed to parse rules file .: Is a directory -(! udevadm verify /dev/null . /dev/null) +assert_1 /dev/null . /dev/null # UDEV_LINE_SIZE 16384 -printf '%16383s\n' ' ' >sample.rules -udevadm verify sample.rules +printf '%16383s\n' ' ' >"${rules}" +assert_0 "${rules}" -# Failed to parse rules file sample.rules: No buffer space available -printf '%16384s\n' ' ' >sample.rules -(! udevadm verify sample.rules) +# Failed to parse rules file ${rules}: No buffer space available +printf '%16384s\n' ' ' >"${rules}" +echo "Failed to parse rules file ${rules}: No buffer space available" >"${exp}" +assert_1 "${rules}" -printf 'RUN+="/bin/true"%8175s\\\n' ' ' ' ' >sample.rules -echo >>sample.rules -udevadm verify sample.rules +printf 'RUN+="/bin/true"%8175s\\\n' ' ' ' ' >"${rules}" +echo >>"${rules}" +assert_0 "${rules}" -printf 'RUN+="/bin/true"%8176s\\\n #\n' ' ' ' ' >sample.rules -echo >>sample.rules -cat >exp <<'EOF' -sample.rules:5 Line is too long, ignored -sample.rules: udev rules check failed +printf 'RUN+="/bin/true"%8176s\\\n #\n' ' ' ' ' >"${rules}" +echo >>"${rules}" +cat >"${exp}" <err) -diff exp err +assert_1 "${rules}" -printf '\\\n' >sample.rules -cat >exp <<'EOF' -sample.rules:1 Unexpected EOF after line continuation, line ignored -sample.rules: udev rules check failed +printf '\\\n' >"${rules}" +cat >"${exp}" <err) -diff exp err +assert_1 "${rules}" test_syntax_error() { - local rule msg - rule="$1"; shift - msg="$1"; shift + local rule msg + rule="$1"; shift + msg="$1"; shift - printf '%s\n' "$rule" > sample.rules - cat >exp <<-EOF - sample.rules:1 $msg - sample.rules: udev rules check failed - EOF - (! udevadm verify sample.rules 2>err) - diff exp err + printf '%s\n' "${rule}" >"${rules}" + cat >"${exp}" <sample.rules -cat >exp <<'EOF' -sample.rules:1 GOTO="a" has no matching label, ignoring -sample.rules:1 The line takes no effect any more, dropping -sample.rules: udev rules check failed +echo 'GOTO="a"' >"${rules}" +cat >"${exp}" <err) -diff exp err +assert_1 "${rules}" -cat >sample.rules <<'EOF' +cat >"${rules}" <<'EOF' GOTO="a" LABEL="a" EOF -udevadm verify sample.rules +assert_0 "${rules}" -cat >sample.rules <<'EOF' +cat >"${rules}" <<'EOF' GOTO="b" LABEL="b" LABEL="b" EOF -cat >exp <<'EOF' -sample.rules:3 LABEL="b" is unused. -sample.rules: udev rules check failed +cat >"${exp}" <err) -diff exp err +assert_1 "${rules}" exit 0 From e8c53080c41e0e651bc33185148925b7f6be4441 Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Thu, 9 Mar 2023 08:00:00 +0000 Subject: [PATCH 4/7] udevadm verify: load all rules from the system if no rules were given When udevadm verify is invoked without positional arguments, that is, when no udev rules files are specified, load all rules files from the system like the udev daemon does, and verify them. --- man/udevadm.xml | 8 +++++--- src/udev/udevadm-verify.c | 18 ++++++++++++++---- test/units/testsuite-17.11.sh | 2 -- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/man/udevadm.xml b/man/udevadm.xml index 95b8da2b0a..bafd5af0fb 100644 --- a/man/udevadm.xml +++ b/man/udevadm.xml @@ -51,7 +51,7 @@ udevadm verify options - file + file udevadm wait options device|syspath @@ -745,13 +745,15 @@ udevadm verify <arg choice="opt"><replaceable>options</replaceable></arg> - <arg choice="plain" rep="repeat"><replaceable>file</replaceable></arg> + <arg choice="opt" 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. + Positional arguments could be used to specify one or more files to check. + If no files are specified, the udev rules are read from the files located in + the same udev/rules.d directories that are processed by the udev daemon. The exit status is 0 if all specified udev rules files are syntactically and semantically correct, and a non-zero error code otherwise. diff --git a/src/udev/udevadm-verify.c b/src/udev/udevadm-verify.c index 8819b07238..f472892731 100644 --- a/src/udev/udevadm-verify.c +++ b/src/udev/udevadm-verify.c @@ -7,6 +7,8 @@ #include #include +#include "conf-files.h" +#include "constants.h" #include "log.h" #include "pretty-print.h" #include "strv.h" @@ -23,7 +25,7 @@ static int help(void) { if (r < 0) return log_oom(); - printf("%s verify [OPTIONS] FILE...\n" + 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" @@ -75,9 +77,6 @@ static int parse_argv(int argc, char *argv[]) { assert_not_reached(); } - if (optind == argc) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No rules file specified."); - return 1; } @@ -121,5 +120,16 @@ int verify_main(int argc, char *argv[], void *userdata) { if (!rules) return -ENOMEM; + if (optind == argc) { + const char* const* rules_dirs = STRV_MAKE_CONST(CONF_PATHS("udev/rules.d")); + _cleanup_strv_free_ char **files = NULL; + + r = conf_files_list_strv(&files, ".rules", NULL, 0, rules_dirs); + if (r < 0) + return log_error_errno(r, "Failed to enumerate rules files: %m"); + + return verify_rules(rules, files); + } + return verify_rules(rules, strv_skip(argv, optind)); } diff --git a/test/units/testsuite-17.11.sh b/test/units/testsuite-17.11.sh index 4abcae60a5..eb6a00dfb7 100755 --- a/test/units/testsuite-17.11.sh +++ b/test/units/testsuite-17.11.sh @@ -60,8 +60,6 @@ assert_0 -V assert_0 --version assert_0 /dev/null -# No rules file specified. -assert_1 # unrecognized option '--unknown' assert_1 --unknown # option requires an argument -- 'N' From 0a7eda348ccf2e4591ac5db039482785fdd40021 Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Thu, 9 Mar 2023 08:00:00 +0000 Subject: [PATCH 5/7] udevadm verify: introduce --root option When udevadm verify is invoked without positional arguments and loads all rules files from the system like the udev daemon does, this option can be used to operate on files underneath the specified root path. --- man/udevadm.xml | 5 +++++ shell-completion/bash/udevadm | 6 +++++- shell-completion/zsh/_udevadm | 1 + src/udev/udevadm-verify.c | 33 ++++++++++++++++++++++++++++----- 4 files changed, 39 insertions(+), 6 deletions(-) diff --git a/man/udevadm.xml b/man/udevadm.xml index bafd5af0fb..503ec81091 100644 --- a/man/udevadm.xml +++ b/man/udevadm.xml @@ -769,6 +769,11 @@ parsed. When set to never, names will never be resolved. + + + When looking for udev rules files located in udev/rules.d directories, + operate on files underneath the specified root path PATH. + diff --git a/shell-completion/bash/udevadm b/shell-completion/bash/udevadm index b6e14e1d36..3c3c403153 100644 --- a/shell-completion/bash/udevadm +++ b/shell-completion/bash/udevadm @@ -64,7 +64,7 @@ _udevadm() { [MONITOR_ARG]='-s --subsystem-match -t --tag-match' [TEST]='-a --action -N --resolve-names' [TEST_BUILTIN]='-a --action' - [VERIFY]='-N --resolve-names' + [VERIFY]='-N --resolve-names --root' [WAIT]='-t --timeout --initialized=no --removed --settle' [LOCK]='-t --timeout -d --device -b --backing -p --print' ) @@ -254,6 +254,10 @@ _udevadm() { -N|--resolve-names) comps='early never' ;; + --root) + comps=$(compgen -A directory -- "$cur" ) + compopt -o dirnames + ;; *) comps='' ;; diff --git a/shell-completion/zsh/_udevadm b/shell-completion/zsh/_udevadm index 074d367a9d..e8414eef67 100644 --- a/shell-completion/zsh/_udevadm +++ b/shell-completion/zsh/_udevadm @@ -108,6 +108,7 @@ _udevadm_test-builtin(){ _udevadm_verify(){ _arguments \ {-N+,--resolve-names=}'[When to resolve names.]:resolve:(early never)' \ + '--root=[Operate on catalog hierarchy under specified directory]:directories:_directories' \ {-h,--help}'[Print help text.]' \ '*::files:_files' } diff --git a/src/udev/udevadm-verify.c b/src/udev/udevadm-verify.c index f472892731..26f317711e 100644 --- a/src/udev/udevadm-verify.c +++ b/src/udev/udevadm-verify.c @@ -10,12 +10,17 @@ #include "conf-files.h" #include "constants.h" #include "log.h" +#include "parse-argument.h" #include "pretty-print.h" +#include "static-destruct.h" #include "strv.h" #include "udev-rules.h" #include "udevadm.h" static ResolveNameTiming arg_resolve_name_timing = RESOLVE_NAME_EARLY; +static char *arg_root = NULL; + +STATIC_DESTRUCTOR_REGISTER(arg_root, freep); static int help(void) { _cleanup_free_ char *link = NULL; @@ -30,6 +35,7 @@ static int help(void) { " -h --help Show this help\n" " -V --version Show package version\n" " -N --resolve-names=early|never When to resolve names\n" + " --root=PATH Operate on an alternate filesystem root\n" "\nSee the %s for details.\n", program_invocation_short_name, ansi_highlight(), @@ -40,14 +46,18 @@ static int help(void) { } static int parse_argv(int argc, char *argv[]) { + enum { + ARG_ROOT = 0x100, + }; static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "version", no_argument, NULL, 'V' }, - { "resolve-names", required_argument, NULL, 'N' }, + { "help", no_argument, NULL, 'h' }, + { "version", no_argument, NULL, 'V' }, + { "resolve-names", required_argument, NULL, 'N' }, + { "root", required_argument, NULL, ARG_ROOT }, {} }; - int c; + int r, c; assert(argc >= 0); assert(argv); @@ -71,12 +81,22 @@ static int parse_argv(int argc, char *argv[]) { if (arg_resolve_name_timing == RESOLVE_NAME_NEVER) arg_resolve_name_timing = RESOLVE_NAME_LATE; break; + case ARG_ROOT: + r = parse_path_argument(optarg, /* suppress_root= */ true, &arg_root); + if (r < 0) + return r; + break; + case '?': return -EINVAL; default: assert_not_reached(); } + if (arg_root && optind < argc) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Combination of --root= and FILEs is not supported."); + return 1; } @@ -124,9 +144,12 @@ int verify_main(int argc, char *argv[], void *userdata) { const char* const* rules_dirs = STRV_MAKE_CONST(CONF_PATHS("udev/rules.d")); _cleanup_strv_free_ char **files = NULL; - r = conf_files_list_strv(&files, ".rules", NULL, 0, rules_dirs); + r = conf_files_list_strv(&files, ".rules", arg_root, 0, rules_dirs); if (r < 0) return log_error_errno(r, "Failed to enumerate rules files: %m"); + if (arg_root && strv_isempty(files)) + return log_error_errno(SYNTHETIC_ERRNO(ENOENT), + "No rules files found in %s", arg_root); return verify_rules(rules, files); } From c8f39ddcf6b0e8560dcecfc21f428114ff69bfb0 Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Thu, 9 Mar 2023 08:00:00 +0000 Subject: [PATCH 6/7] testsuite-17.11.sh: check udevadm verify --root --- test/units/testsuite-17.11.sh | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/units/testsuite-17.11.sh b/test/units/testsuite-17.11.sh index eb6a00dfb7..9a404781e7 100755 --- a/test/units/testsuite-17.11.sh +++ b/test/units/testsuite-17.11.sh @@ -75,6 +75,21 @@ assert_1 . # Failed to parse rules file .: Is a directory assert_1 /dev/null . /dev/null +rules_dir='etc/udev/rules.d' +mkdir -p "${rules_dir}" +# No rules files found in $PWD +assert_1 --root="${workdir}" + +touch "${rules_dir}/empty.rules" +assert_0 --root="${workdir}" + +# Combination of --root= and FILEs is not supported. +assert_1 --root="${workdir}" /dev/null +# No rules files found in nosuchdir +assert_1 --root=nosuchdir + +cd "${rules_dir}" + # UDEV_LINE_SIZE 16384 printf '%16383s\n' ' ' >"${rules}" assert_0 "${rules}" @@ -247,4 +262,9 @@ ${rules}: udev rules check failed EOF assert_1 "${rules}" +# udevadm verify --root +sed "s|sample-[0-9]*.rules|${workdir}/${rules_dir}/&|" sample-*.exp >"${workdir}/${exp}" +cd - +assert_1 --root="${workdir}" + exit 0 From b2f7bb760ac75820ee82a7dbaa23b7c21c08e2de Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Fri, 10 Mar 2023 08:00:00 +0000 Subject: [PATCH 7/7] udev_rules_parse_file: issue diagnostics about duplicate LABEL tokens When a rules contains several LABEL tokens, the parser used to silently discard all of them besides the last one without any diagnostics at all. It's time to break the vow of silence and let the parser issue a warning. --- src/udev/udev-rules.c | 3 +++ test/units/testsuite-17.11.sh | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 9d831a6004..5e7d8d390e 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -1014,6 +1014,9 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp return log_token_invalid_attr(rules, key); if (op != OP_ASSIGN) return log_token_invalid_op(rules, key); + if (FLAGS_SET(rule_line->type, LINE_HAS_LABEL)) + log_token_warning(rules, "Contains multiple LABEL keys, ignoring LABEL=\"%s\".", + rule_line->label); rule_line->label = value; SET_FLAG(rule_line->type, LINE_HAS_LABEL, true); diff --git a/test/units/testsuite-17.11.sh b/test/units/testsuite-17.11.sh index 9a404781e7..363040c28c 100755 --- a/test/units/testsuite-17.11.sh +++ b/test/units/testsuite-17.11.sh @@ -262,6 +262,19 @@ ${rules}: udev rules check failed EOF assert_1 "${rules}" +cat >"${rules}" <<'EOF' +GOTO="a" +LABEL="a", LABEL="b" +EOF +cat >"${exp}" <"${workdir}/${exp}" cd -