From: Zbigniew Jędrzejewski-Szmek Date: Thu, 27 Jun 2019 08:47:56 +0000 (+0200) Subject: udev-rules: add precise information to rule failure logs X-Git-Tag: v243-rc1~218^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=f85cc54c4bbeca11783efdae4332a2759c8ac64c;p=thirdparty%2Fsystemd.git udev-rules: add precise information to rule failure logs It is pretty hard to figure out what the problem actually is, esp. when the rule is long. On my machine: systemd[1]: Starting udev Kernel Device Manager... systemd-udevd[217399]: /usr/lib/udev/rules.d/11-dm-lvm.rules:40 Invalid value for OPTIONS key, ignoring: 'event_timeout=180' systemd-udevd[217399]: /usr/lib/udev/rules.d/11-dm-lvm.rules:40 The line takes no effect, ignoring. systemd-udevd[217399]: /etc/udev/rules.d/60-ipath.rules:4 Invalid value "kcopy/%02n" for NAME (char 7: invalid substitution type), ignoring, but please fix it. systemd-udevd[217399]: /usr/lib/udev/rules.d/65-md-incremental.rules:28 Invalid value "/sbin/mdadm -I $env{DEVNAME} --export $devnode --offroot ${DEVLINKS}" for IMPORT (char 58: invalid substitution type), ignoring, but please fix it. systemd-udevd[217399]: /etc/udev/rules.d/73-special-net-names.rules:14 Invalid value "/bin/sh -ec 'D=${DEVPATH#*/vio/}; D=${D%%%%/*}; D=${D#????}; D=${D#0}; D=${D#0}; D=${D#0}; D=${D#0}; echo ${D:-0}'" for PROGRAM (char 16: invalid substitution type), ignoring, but please fix it. systemd-udevd[217399]: /usr/lib/udev/rules.d/84-nm-drivers.rules:10 Invalid value "/bin/sh -c 'ethtool -i $1 | sed -n s/^driver:\ //p' -- $env{INTERFACE}" for PROGRAM (char 24: invalid substitution type), ignoring, but please fix it. systemd-udevd[217399]: /usr/lib/udev/rules.d/90-libgpod.rules:19 IMPORT key takes '==' or '!=' operator, assuming '==', but please fix it. systemd-udevd[217399]: /usr/lib/udev/rules.d/90-libgpod.rules:23 IMPORT key takes '==' or '!=' operator, assuming '==', but please fix it. systemd-udevd[217399]: /usr/lib/udev/rules.d/99-vmware-scsi-udev.rules:5 Invalid value "/bin/sh -c 'echo 180 >/sys$DEVPATH/device/timeout'" for RUN (char 27: invalid substitution type), ignoring, but please fix it. systemd-udevd[217399]: /usr/lib/udev/rules.d/99-vmware-scsi-udev.rules:6 Invalid value "/bin/sh -c 'echo 180 >/sys$DEVPATH/device/timeout'" for RUN (char 27: invalid substitution type), ignoring, but please fix it. systemd[1]: Started udev Kernel Device Manager. --- diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 0f74fe340a7..025a37bc26b 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -484,29 +484,44 @@ ssize_t udev_event_apply_format(UdevEvent *event, return size; } -int udev_check_format(const char *s) { +int udev_check_format(const char *value, size_t *offset, const char **hint) { FormatSubstitutionType type; + const char *s = value; char attr[UTIL_PATH_SIZE]; int r; while (*s) { r = get_subst_type(&s, true, &type, attr); - if (r < 0) + if (r < 0) { + if (offset) + *offset = s - value; + if (hint) + *hint = "invalid substitution type"; return r; - if (r == 0) { + } else if (r == 0) { s++; continue; } - if (IN_SET(type, FORMAT_SUBST_ATTR, FORMAT_SUBST_ENV) && isempty(attr)) + if (IN_SET(type, FORMAT_SUBST_ATTR, FORMAT_SUBST_ENV) && isempty(attr)) { + if (offset) + *offset = s - value; + if (hint) + *hint = "attribute value missing"; return -EINVAL; + } if (type == FORMAT_SUBST_RESULT && !isempty(attr)) { unsigned i; r = safe_atou_optional_plus(attr, &i); - if (r < 0) + if (r < 0) { + if (offset) + *offset = s - value; + if (hint) + *hint = "attribute value not a valid number"; return r; + } } } diff --git a/src/udev/udev-event.h b/src/udev/udev-event.h index eca4ec8a9d0..a05e1252595 100644 --- a/src/udev/udev-event.h +++ b/src/udev/udev-event.h @@ -51,7 +51,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(UdevEvent*, udev_event_free); ssize_t udev_event_apply_format(UdevEvent *event, const char *src, char *dest, size_t size, bool replace_whitespace); -int udev_check_format(const char *s); +int udev_check_format(const char *value, size_t *offset, const char **hint); int udev_event_spawn(UdevEvent *event, usec_t timeout_usec, bool accept_failure, diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 40f743a033c..66208dc2862 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -228,10 +228,10 @@ struct UdevRules { log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL), \ "Invalid attribute \"%s\" for %s, ignoring, but please fix it.", \ attr, key) -#define log_token_invalid_value(rules, key, value) \ +#define log_token_invalid_value(rules, key, value, offset, hint) \ log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL), \ - "Invalid value \"%s\" for %s, ignoring, but please fix it.", \ - value, key) + "Invalid value \"%s\" for %s (char %zu: %s), ignoring, but please fix it.", \ + value, key, offset, hint) static void log_unknown_owner(sd_device *dev, UdevRules *rules, int error, const char *entity, const char *name) { if (IN_SET(abs(error), ENOENT, ESRCH)) @@ -510,6 +510,16 @@ static int rule_line_add_token(UdevRuleLine *rule_line, UdevRuleTokenType type, return 0; } +static void check_value_format_and_warn(UdevRules *rules, const char *key, const char *value, bool nonempty) { + size_t offset; + const char *hint; + + if (nonempty && isempty(value)) + log_token_invalid_value(rules, key, value, (size_t) 0, "empty value"); + else if (udev_check_format(value, &offset, &hint) < 0) + log_token_invalid_value(rules, key, value, offset + 1, hint); +} + static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOperatorType op, char *value) { bool is_match = IN_SET(op, OP_MATCH, OP_NOMATCH); UdevRuleLine *rule_line; @@ -551,9 +561,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp return log_token_invalid_op(rules, key); if (!is_match) { - if (udev_check_format(value) < 0) - log_token_invalid_value(rules, key, value); - + check_value_format_and_warn(rules, key, value, false); r = rule_line_add_token(rule_line, TK_A_DEVLINK, op, value, NULL); } else r = rule_line_add_token(rule_line, TK_M_DEVLINK, op, value, NULL); @@ -574,8 +582,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp if (isempty(value)) return log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL), "Ignoring NAME=\"\", as udev will not delete any device nodes."); - if (udev_check_format(value) < 0) - log_token_invalid_value(rules, key, value); + check_value_format_and_warn(rules, key, value, false); r = rule_line_add_token(rule_line, TK_A_NAME, op, value, NULL); } else @@ -597,8 +604,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp return log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL), "Invalid ENV attribute. '%s' cannot be set.", attr); - if (udev_check_format(value) < 0) - log_token_invalid_value(rules, key, value); + check_value_format_and_warn(rules, key, value, false); + r = rule_line_add_token(rule_line, TK_A_ENV, op, value, attr); } else r = rule_line_add_token(rule_line, TK_M_ENV, op, value, attr); @@ -611,8 +618,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp } if (!is_match) { - if (isempty(value) || udev_check_format(value) < 0) - log_token_invalid_value(rules, key, value); + check_value_format_and_warn(rules, key, value, true); + r = rule_line_add_token(rule_line, TK_A_TAG, op, value, NULL); } else r = rule_line_add_token(rule_line, TK_M_TAG, op, value, NULL); @@ -636,7 +643,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp } else if (streq(key, "ATTR")) { if (isempty(attr)) return log_token_invalid_attr(rules, key); - if (udev_check_format(attr) < 0) + if (udev_check_format(attr, NULL, NULL) < 0) log_token_invalid_attr_format(rules, key, attr); if (op == OP_REMOVE) return log_token_invalid_op(rules, key); @@ -646,16 +653,14 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp } if (!is_match) { - if (udev_check_format(value) < 0) - log_token_invalid_value(rules, key, value); - + check_value_format_and_warn(rules, key, value, false); r = rule_line_add_token(rule_line, TK_A_ATTR, op, value, attr); } else r = rule_line_add_token(rule_line, TK_M_ATTR, op, value, attr); } else if (streq(key, "SYSCTL")) { if (isempty(attr)) return log_token_invalid_attr(rules, key); - if (udev_check_format(attr) < 0) + if (udev_check_format(attr, NULL, NULL) < 0) log_token_invalid_attr_format(rules, key, attr); if (op == OP_REMOVE) return log_token_invalid_op(rules, key); @@ -665,9 +670,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp } if (!is_match) { - if (udev_check_format(value) < 0) - log_token_invalid_value(rules, key, value); - + check_value_format_and_warn(rules, key, value, false); r = rule_line_add_token(rule_line, TK_A_SYSCTL, op, value, attr); } else r = rule_line_add_token(rule_line, TK_M_SYSCTL, op, value, attr); @@ -695,7 +698,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp } else if (streq(key, "ATTRS")) { if (isempty(attr)) return log_token_invalid_attr(rules, key); - if (udev_check_format(attr) < 0) + if (udev_check_format(attr, NULL, NULL) < 0) log_token_invalid_attr_format(rules, key, attr); if (!is_match) return log_token_invalid_op(rules, key); @@ -721,8 +724,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp if (r < 0) return log_token_error_errno(rules, r, "Failed to parse mode '%s': %m", attr); } - if (isempty(value) || udev_check_format(value) < 0) - log_token_invalid_value(rules, key, value); + check_value_format_and_warn(rules, key, value, true); if (!is_match) return log_token_invalid_op(rules, key); @@ -730,8 +732,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp } else if (streq(key, "PROGRAM")) { if (attr) return log_token_invalid_attr(rules, key); - if (isempty(value) || udev_check_format(value) < 0) - log_token_invalid_value(rules, key, value); + check_value_format_and_warn(rules, key, value, true); if (op == OP_REMOVE) return log_token_invalid_op(rules, key); if (!is_match) { @@ -746,8 +747,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp } else if (streq(key, "IMPORT")) { if (isempty(attr)) return log_token_invalid_attr(rules, key); - if (isempty(value) || udev_check_format(value) < 0) - log_token_invalid_value(rules, key, value); + check_value_format_and_warn(rules, key, value, true); if (op == OP_REMOVE) return log_token_invalid_op(rules, key); if (!is_match) { @@ -849,8 +849,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp r = rule_line_add_token(rule_line, TK_A_OWNER_ID, op, NULL, UID_TO_PTR(uid)); } else if (rules->resolve_name_timing != RESOLVE_NAME_NEVER) { - if (isempty(value) || udev_check_format(value) < 0) - log_token_invalid_value(rules, key, value); + check_value_format_and_warn(rules, key, value, true); r = rule_line_add_token(rule_line, TK_A_OWNER, op, value, NULL); } else { log_token_debug(rules, "Resolving user name is disabled, ignoring %s=%s", key, value); @@ -878,8 +877,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp r = rule_line_add_token(rule_line, TK_A_GROUP_ID, op, NULL, GID_TO_PTR(gid)); } else if (rules->resolve_name_timing != RESOLVE_NAME_NEVER) { - if (isempty(value) || udev_check_format(value) < 0) - log_token_invalid_value(rules, key, value); + check_value_format_and_warn(rules, key, value, true); r = rule_line_add_token(rule_line, TK_A_GROUP, op, value, NULL); } else { log_token_debug(rules, "Resolving group name is disabled, ignoring %s=%s", key, value); @@ -900,15 +898,13 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp if (parse_mode(value, &mode) >= 0) r = rule_line_add_token(rule_line, TK_A_MODE_ID, op, NULL, MODE_TO_PTR(mode)); else { - if (isempty(value) || udev_check_format(value) < 0) - log_token_invalid_value(rules, key, value); + check_value_format_and_warn(rules, key, value, true); r = rule_line_add_token(rule_line, TK_A_MODE, op, value, NULL); } } else if (streq(key, "SECLABEL")) { if (isempty(attr)) return log_token_invalid_attr(rules, key); - if (isempty(value) || udev_check_format(value) < 0) - log_token_invalid_value(rules, key, value); + check_value_format_and_warn(rules, key, value, true); if (is_match || op == OP_REMOVE) return log_token_invalid_op(rules, key); if (op == OP_ASSIGN_FINAL) { @@ -920,8 +916,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp } else if (streq(key, "RUN")) { if (is_match || op == OP_REMOVE) return log_token_invalid_op(rules, key); - if (isempty(value) || udev_check_format(value) < 0) - log_token_invalid_value(rules, key, value); + check_value_format_and_warn(rules, key, value, true); if (!attr || streq(attr, "program")) r = rule_line_add_token(rule_line, TK_A_RUN_PROGRAM, op, value, NULL); else if (streq(attr, "builtin")) {