Merge pull request #26886 from ldv-alt/udevadm-verify

udev-rules: fix matching of token types that support alternative patterns

src/shared/udev-util.c

@@ -347,11 +347,10 @@ int udev_rule_parse_value(char *str, char **ret_value, char **ret_endpos) {
         str += is_escaped;
         if (str[0] != '"')
                 return -EINVAL;
-        str++;
 
         if (!is_escaped) {
                 /* unescape double quotation '\"'->'"' */
-                for (i = j = str; *i != '"'; i++, j++) {
+                for (j = str, i = str + 1; *i != '"'; i++, j++) {
                         if (*i == '\0')
                                 return -EINVAL;
                         if (i[0] == '\\' && i[1] == '"')
@@ -359,12 +358,17 @@ int udev_rule_parse_value(char *str, char **ret_value, char **ret_endpos) {
                         *j = *i;
                 }
                 j[0] = '\0';
+                /*
+                 * The return value must be terminated by two subsequent NULs
+                 * so it could be safely interpreted as nulstr.
+                 */
+                j[1] = '\0';
         } else {
                 _cleanup_free_ char *unescaped = NULL;
                 ssize_t l;
 
                 /* find the end position of value */
-                for (i = str; *i != '"'; i++) {
+                for (i = str + 1; *i != '"'; i++) {
                         if (i[0] == '\\')
                                 i++;
                         if (*i == '\0')
@@ -372,12 +376,17 @@ int udev_rule_parse_value(char *str, char **ret_value, char **ret_endpos) {
                 }
                 i[0] = '\0';
 
-                l = cunescape_length(str, i - str, 0, &unescaped);
+                l = cunescape_length(str + 1, i - (str + 1), 0, &unescaped);
                 if (l < 0)
                         return l;
 
-                assert(l <= i - str);
+                assert(l <= i - (str + 1));
                 memcpy(str, unescaped, l + 1);
+                /*
+                 * The return value must be terminated by two subsequent NULs
+                 * so it could be safely interpreted as nulstr.
+                 */
+                str[l + 1] = '\0';
         }
 
         *ret_value = str;

src/test/test-udev-util.c

@@ -24,6 +24,11 @@ static void test_udev_rule_parse_value_one(const char *in, const char *expected_
         } else {
                 assert_se(streq_ptr(value, expected_value));
                 assert_se(endpos == str + strlen(in));
+                /*
+                 * The return value must be terminated by two subsequent NULs
+                 * so it could be safely interpreted as nulstr.
+                 */
+                assert_se(value[strlen(value) + 1] == '\0');
         }
 }
 

src/udev/udev-rules.c

@@ -477,6 +477,10 @@ static UdevRuleSubstituteType rule_get_substitution_type(const char *str) {
         return SUBST_TYPE_PLAIN;
 }
 
+static bool type_has_nulstr_value(UdevRuleTokenType type) {
+        return type < TK_M_TEST || type == TK_M_RESULT;
+}
+
 static int rule_line_add_token(UdevRuleLine *rule_line, UdevRuleTokenType type, UdevRuleOperatorType op, char *value, void *data) {
         _cleanup_(udev_rule_token_freep) UdevRuleToken *token = NULL;
         UdevRuleMatchType match_type = _MATCH_TYPE_INVALID;
@@ -505,7 +509,7 @@ static int rule_line_add_token(UdevRuleLine *rule_line, UdevRuleTokenType type,
                 else
                         match_type = MATCH_TYPE_PLAIN;
 
-                if (type < TK_M_TEST || type == TK_M_RESULT) {
+                if (type_has_nulstr_value(type)) {
                         /* Convert value string to nulstr. */
                         bool bar = true, empty = false;
                         char *a, *b;
@@ -1148,9 +1152,7 @@ static int rule_add_line(UdevRuleFile *rule_file, const char *line_str, unsigned
         if (isempty(line_str))
                 return 0;
 
-        /* We use memdup_suffix0() here, since we want to add a second NUL byte to the end, since possibly
-         * some parsers might turn this into a "nulstr", which requires an extra NUL at the end. */
-        line = memdup_suffix0(line_str, strlen(line_str) + 1);
+        line = strdup(line_str);
         if (!line)
                 return log_oom();
 
@@ -1363,18 +1365,32 @@ static bool token_type_and_data_eq(const UdevRuleToken *a, const UdevRuleToken *
                (token_data_is_string(a->type) ? streq_ptr(a->data, b->data) : (a->data == b->data));
 }
 
-static bool token_value_eq(const UdevRuleToken *a, const UdevRuleToken *b) {
+static bool nulstr_eq(const char *a, const char *b) {
+        NULSTR_FOREACH(i, a)
+                if (!nulstr_contains(b, i))
+                        return false;
+
+        NULSTR_FOREACH(i, b)
+                if (!nulstr_contains(a, i))
+                        return false;
+
+        return true;
+}
+
+static bool token_type_and_value_eq(const UdevRuleToken *a, const UdevRuleToken *b) {
         assert(a);
         assert(b);
 
-        if (a->match_type != b->match_type)
+        if (a->type != b->type ||
+            a->match_type != b->match_type)
                 return false;
 
         /* token value is ignored for certain match types */
         if (IN_SET(a->match_type, MATCH_TYPE_EMPTY, MATCH_TYPE_SUBSYSTEM))
                 return true;
 
-        return streq_ptr(a->value, b->value);
+        return type_has_nulstr_value(a->type) ? nulstr_eq(a->value, b->value) :
+                                                streq_ptr(a->value, b->value);
 }
 
 static bool conflicting_op(UdevRuleOperatorType a, UdevRuleOperatorType b) {
@@ -1389,7 +1405,7 @@ static bool tokens_eq(const UdevRuleToken *a, const UdevRuleToken *b) {
 
         return a->attr_subst_type == b->attr_subst_type &&
                a->attr_match_remove_trailing_whitespace == b->attr_match_remove_trailing_whitespace &&
-               token_value_eq(a, b) &&
+               token_type_and_value_eq(a, b) &&
                token_type_and_data_eq(a, b);
 }
 

test/units/testsuite-17.11.sh

@@ -277,14 +277,22 @@ test_syntax_error 'LABEL="b"' 'LABEL="b" is unused.'
 test_syntax_error 'a="b"' "Invalid key 'a'"
 test_syntax_error 'KERNEL=="", KERNEL=="?*", NAME="a"' 'conflicting match expressions, the line takes no effect'
 test_syntax_error 'KERNEL=="abc", KERNEL!="abc", NAME="b"' 'conflicting match expressions, the line takes no effect'
+test_syntax_error 'KERNEL=="|a|b", KERNEL!="b|a|", NAME="c"' 'conflicting match expressions, the line takes no effect'
 # shellcheck disable=SC2016
 test_syntax_error 'ENV{DISKSEQ}=="?*", ENV{DEVTYPE}!="partition", ENV{DISKSEQ}!="?*" ENV{ID_IGNORE_DISKSEQ}!="1", SYMLINK+="disk/by-diskseq/$env{DISKSEQ}"' \
                   'conflicting match expressions, the line takes no effect'
 test_syntax_error 'KERNEL!="", KERNEL=="?*", NAME="a"' 'duplicate expressions'
+test_syntax_error 'KERNEL=="|a|b", KERNEL=="b|a|", NAME="c"' 'duplicate expressions'
 # shellcheck disable=SC2016
 test_syntax_error 'ENV{DISKSEQ}=="?*", ENV{DEVTYPE}!="partition", ENV{DISKSEQ}=="?*" ENV{ID_IGNORE_DISKSEQ}!="1", SYMLINK+="disk/by-diskseq/$env{DISKSEQ}"' \
                   'duplicate expressions'
 
+cat >"${rules}" <<'EOF'
+KERNEL=="a|b", KERNEL=="a|c", NAME="d"
+KERNEL=="a|b", KERNEL!="a|c", NAME="d"
+EOF
+assert_0 "${rules}"
+
 echo 'GOTO="a"' >"${rules}"
 cat >"${exp}" <<EOF
 ${rules}:1 GOTO="a" has no matching label, ignoring