diff --git a/src/shared/udev-util.c b/src/shared/udev-util.c index a4b17be097..646eaf9135 100644 --- a/src/shared/udev-util.c +++ b/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; diff --git a/src/test/test-udev-util.c b/src/test/test-udev-util.c index 1db2dad4ff..4be3694e9e 100644 --- a/src/test/test-udev-util.c +++ b/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'); } } diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 71c0949220..f708f79c9a 100644 --- a/src/udev/udev-rules.c +++ b/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); } diff --git a/test/units/testsuite-17.11.sh b/test/units/testsuite-17.11.sh index 50e1820957..5f067f02b2 100755 --- a/test/units/testsuite-17.11.sh +++ 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}" <