From d4114f704f71ba309e316e66499cd454b96d630c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 27 Jun 2019 08:36:20 +0200 Subject: [PATCH 1/5] udev: move rules logging functions into the .c file They are (and can) only be used there, no need to expose them. --- src/udev/udev-rules.c | 73 ++++++++++++++++++++++++++++++++++++++----- src/udev/udev-rules.h | 54 -------------------------------- 2 files changed, 66 insertions(+), 61 deletions(-) diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 6811a5ce4b..8ad346b7ac 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -30,6 +30,72 @@ #define RULES_DIRS (const char* const*) CONF_PATHS_STRV("udev/rules.d") + +/*** Logging helpers ***/ + +#define log_rule_full(device, rules, level, error, fmt, ...) \ + ({ \ + UdevRules *_r = (rules); \ + UdevRuleFile *_f = _r ? _r->current_file : NULL; \ + UdevRuleLine *_l = _f ? _f->current_line : NULL; \ + const char *_n = _f ? _f->filename : NULL; \ + \ + log_device_full(device, level, error, "%s:%u " fmt, \ + strna(_n), _l ? _l->line_number : 0, \ + ##__VA_ARGS__); \ + }) + +#define log_rule_debug(device, rules, ...) log_rule_full(device, rules, LOG_DEBUG, 0, ##__VA_ARGS__) +#define log_rule_info(device, rules, ...) log_rule_full(device, rules, LOG_INFO, 0, ##__VA_ARGS__) +#define log_rule_notice(device, rules, ...) log_rule_full(device, rules, LOG_NOTICE, 0, ##__VA_ARGS__) +#define log_rule_warning(device, rules, ...) log_rule_full(device, rules, LOG_WARNING, 0, ##__VA_ARGS__) +#define log_rule_error(device, rules, ...) log_rule_full(device, rules, LOG_ERR, 0, ##__VA_ARGS__) + +#define log_rule_debug_errno(device, rules, error, ...) log_rule_full(device, rules, LOG_DEBUG, error, ##__VA_ARGS__) +#define log_rule_info_errno(device, rules, error, ...) log_rule_full(device, rules, LOG_INFO, error, ##__VA_ARGS__) +#define log_rule_notice_errno(device, rules, error, ...) log_rule_full(device, rules, LOG_NOTICE, error, ##__VA_ARGS__) +#define log_rule_warning_errno(device, rules, error, ...) log_rule_full(device, rules, LOG_WARNING, error, ##__VA_ARGS__) +#define log_rule_error_errno(device, rules, error, ...) log_rule_full(device, rules, LOG_ERR, error, ##__VA_ARGS__) + +#define log_token_full(rules, ...) log_rule_full(NULL, rules, ##__VA_ARGS__) + +#define log_token_debug(rules, ...) log_token_full(rules, LOG_DEBUG, 0, ##__VA_ARGS__) +#define log_token_info(rules, ...) log_token_full(rules, LOG_INFO, 0, ##__VA_ARGS__) +#define log_token_notice(rules, ...) log_token_full(rules, LOG_NOTICE, 0, ##__VA_ARGS__) +#define log_token_warning(rules, ...) log_token_full(rules, LOG_WARNING, 0, ##__VA_ARGS__) +#define log_token_error(rules, ...) log_token_full(rules, LOG_ERR, 0, ##__VA_ARGS__) + +#define log_token_debug_errno(rules, error, ...) log_token_full(rules, LOG_DEBUG, error, ##__VA_ARGS__) +#define log_token_info_errno(rules, error, ...) log_token_full(rules, LOG_INFO, error, ##__VA_ARGS__) +#define log_token_notice_errno(rules, error, ...) log_token_full(rules, LOG_NOTICE, error, ##__VA_ARGS__) +#define log_token_warning_errno(rules, error, ...) log_token_full(rules, LOG_WARNING, error, ##__VA_ARGS__) +#define log_token_error_errno(rules, error, ...) log_token_full(rules, LOG_ERR, error, ##__VA_ARGS__) + +#define _log_token_invalid(rules, key, type) \ + log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL), \ + "Invalid %s for %s.", type, key) + +#define log_token_invalid_op(rules, key) _log_token_invalid(rules, key, "operator") +#define log_token_invalid_attr(rules, key) _log_token_invalid(rules, key, "attribute") + +#define log_token_invalid_attr_format(rules, key, attr) \ + 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) \ + log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL), \ + "Invalid value \"%s\" for %s, ignoring, but please fix it.", \ + value, key) + +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)) + log_rule_error(dev, rules, "Unknown %s '%s', ignoring", entity, name); + else + log_rule_error_errno(dev, rules, error, "Failed to resolve %s '%s', ignoring: %m", entity, name); +} + +/*** Other functions ***/ + static void udev_rule_token_free(UdevRuleToken *token) { free(token); } @@ -91,13 +157,6 @@ UdevRules *udev_rules_free(UdevRules *rules) { return mfree(rules); } -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)) - log_rule_error(dev, rules, "Unknown %s '%s', ignoring", entity, name); - else - log_rule_error_errno(dev, rules, error, "Failed to resolve %s '%s', ignoring: %m", entity, name); -} - static int rule_resolve_user(UdevRules *rules, const char *name, uid_t *ret) { _cleanup_free_ char *n = NULL; uid_t uid; diff --git a/src/udev/udev-rules.h b/src/udev/udev-rules.h index f5f5bdb060..52bf03b076 100644 --- a/src/udev/udev-rules.h +++ b/src/udev/udev-rules.h @@ -170,57 +170,3 @@ int udev_rules_apply_to_event(UdevRules *rules, UdevEvent *event, usec_t timeout_usec, Hashmap *properties_list); int udev_rules_apply_static_dev_perms(UdevRules *rules); - -#define log_rule_full(device, rules, level, error, fmt, ...) \ - ({ \ - UdevRules *_r = (rules); \ - UdevRuleFile *_f = _r ? _r->current_file : NULL; \ - UdevRuleLine *_l = _f ? _f->current_line : NULL; \ - const char *_n = _f ? _f->filename : NULL; \ - \ - log_device_full(device, level, error, "%s:%u " fmt, \ - strna(_n), _l ? _l->line_number : 0, \ - ##__VA_ARGS__); \ - }) - -#define log_rule_debug(device, rules, ...) log_rule_full(device, rules, LOG_DEBUG, 0, ##__VA_ARGS__) -#define log_rule_info(device, rules, ...) log_rule_full(device, rules, LOG_INFO, 0, ##__VA_ARGS__) -#define log_rule_notice(device, rules, ...) log_rule_full(device, rules, LOG_NOTICE, 0, ##__VA_ARGS__) -#define log_rule_warning(device, rules, ...) log_rule_full(device, rules, LOG_WARNING, 0, ##__VA_ARGS__) -#define log_rule_error(device, rules, ...) log_rule_full(device, rules, LOG_ERR, 0, ##__VA_ARGS__) - -#define log_rule_debug_errno(device, rules, error, ...) log_rule_full(device, rules, LOG_DEBUG, error, ##__VA_ARGS__) -#define log_rule_info_errno(device, rules, error, ...) log_rule_full(device, rules, LOG_INFO, error, ##__VA_ARGS__) -#define log_rule_notice_errno(device, rules, error, ...) log_rule_full(device, rules, LOG_NOTICE, error, ##__VA_ARGS__) -#define log_rule_warning_errno(device, rules, error, ...) log_rule_full(device, rules, LOG_WARNING, error, ##__VA_ARGS__) -#define log_rule_error_errno(device, rules, error, ...) log_rule_full(device, rules, LOG_ERR, error, ##__VA_ARGS__) - -#define log_token_full(rules, ...) log_rule_full(NULL, rules, ##__VA_ARGS__) - -#define log_token_debug(rules, ...) log_token_full(rules, LOG_DEBUG, 0, ##__VA_ARGS__) -#define log_token_info(rules, ...) log_token_full(rules, LOG_INFO, 0, ##__VA_ARGS__) -#define log_token_notice(rules, ...) log_token_full(rules, LOG_NOTICE, 0, ##__VA_ARGS__) -#define log_token_warning(rules, ...) log_token_full(rules, LOG_WARNING, 0, ##__VA_ARGS__) -#define log_token_error(rules, ...) log_token_full(rules, LOG_ERR, 0, ##__VA_ARGS__) - -#define log_token_debug_errno(rules, error, ...) log_token_full(rules, LOG_DEBUG, error, ##__VA_ARGS__) -#define log_token_info_errno(rules, error, ...) log_token_full(rules, LOG_INFO, error, ##__VA_ARGS__) -#define log_token_notice_errno(rules, error, ...) log_token_full(rules, LOG_NOTICE, error, ##__VA_ARGS__) -#define log_token_warning_errno(rules, error, ...) log_token_full(rules, LOG_WARNING, error, ##__VA_ARGS__) -#define log_token_error_errno(rules, error, ...) log_token_full(rules, LOG_ERR, error, ##__VA_ARGS__) - -#define _log_token_invalid(rules, key, type) \ - log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL), \ - "Invalid %s for %s.", type, key) - -#define log_token_invalid_op(rules, key) _log_token_invalid(rules, key, "operator") -#define log_token_invalid_attr(rules, key) _log_token_invalid(rules, key, "attribute") - -#define log_token_invalid_attr_format(rules, key, attr) \ - 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) \ - log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL), \ - "Invalid value \"%s\" for %s, ignoring, but please fix it.", \ - value, key) From e79c228b527cf5fd45a32edb0f8fff124a680cc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 27 Jun 2019 08:51:53 +0200 Subject: [PATCH 2/5] udev: move rule structure definitions into the .c file They are (and should) only be used there, no need to expose them. --- src/udev/udev-rules.c | 146 ++++++++++++++++++++++++++++++++++++++++++ src/udev/udev-rules.h | 145 ----------------------------------------- 2 files changed, 146 insertions(+), 145 deletions(-) diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 8ad346b7ac..40f743a033 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -14,6 +14,7 @@ #include "fs-util.h" #include "glob-util.h" #include "libudev-util.h" +#include "list.h" #include "mkdir.h" #include "nulstr-util.h" #include "parse-util.h" @@ -31,6 +32,151 @@ #define RULES_DIRS (const char* const*) CONF_PATHS_STRV("udev/rules.d") +typedef enum { + OP_MATCH, /* == */ + OP_NOMATCH, /* != */ + OP_ADD, /* += */ + OP_REMOVE, /* -= */ + OP_ASSIGN, /* = */ + OP_ASSIGN_FINAL, /* := */ + _OP_TYPE_MAX, + _OP_TYPE_INVALID = -1 +} UdevRuleOperatorType; + +typedef enum { + MATCH_TYPE_EMPTY, /* empty string */ + MATCH_TYPE_PLAIN, /* no special characters */ + MATCH_TYPE_GLOB, /* shell globs ?,*,[] */ + MATCH_TYPE_SUBSYSTEM, /* "subsystem", "bus", or "class" */ + _MATCH_TYPE_MAX, + _MATCH_TYPE_INVALID = -1 +} UdevRuleMatchType; + +typedef enum { + SUBST_TYPE_PLAIN, /* no substitution */ + SUBST_TYPE_FORMAT, /* % or $ */ + SUBST_TYPE_SUBSYS, /* "[/]" format */ + _SUBST_TYPE_MAX, + _SUBST_TYPE_INVALID = -1 +} UdevRuleSubstituteType; + +typedef enum { + /* lvalues which take match or nomatch operator */ + TK_M_ACTION, /* string, device_get_action() */ + TK_M_DEVPATH, /* path, sd_device_get_devpath() */ + TK_M_KERNEL, /* string, sd_device_get_sysname() */ + TK_M_DEVLINK, /* strv, sd_device_get_devlink_first(), sd_device_get_devlink_next() */ + TK_M_NAME, /* string, name of network interface */ + TK_M_ENV, /* string, device property, takes key through attribute */ + TK_M_TAG, /* strv, sd_device_get_tag_first(), sd_device_get_tag_next() */ + TK_M_SUBSYSTEM, /* string, sd_device_get_subsystem() */ + TK_M_DRIVER, /* string, sd_device_get_driver() */ + TK_M_ATTR, /* string, takes filename through attribute, sd_device_get_sysattr_value(), util_resolve_subsys_kernel(), etc. */ + TK_M_SYSCTL, /* string, takes kernel parameter through attribute */ + + /* matches parent paramters */ + TK_M_PARENTS_KERNEL, /* string */ + TK_M_PARENTS_SUBSYSTEM, /* string */ + TK_M_PARENTS_DRIVER, /* string */ + TK_M_PARENTS_ATTR, /* string */ + TK_M_PARENTS_TAG, /* strv */ + + TK_M_TEST, /* path, optionally mode_t can be specified by attribute, test the existence of a file */ + TK_M_PROGRAM, /* string, execute a program */ + TK_M_IMPORT_FILE, /* path */ + TK_M_IMPORT_PROGRAM, /* string, import properties from the result of program */ + TK_M_IMPORT_BUILTIN, /* string, import properties from the result of built-in command */ + TK_M_IMPORT_DB, /* string, import properties from database */ + TK_M_IMPORT_CMDLINE, /* string, kernel command line */ + TK_M_IMPORT_PARENT, /* string, parent property */ + TK_M_RESULT, /* string, result of TK_M_PROGRAM */ + +#define _TK_M_MAX (TK_M_RESULT + 1) +#define _TK_A_MIN _TK_M_MAX + + /* lvalues which take one of assign operators */ + TK_A_OPTIONS_STRING_ESCAPE_NONE, /* no argument */ + TK_A_OPTIONS_STRING_ESCAPE_REPLACE, /* no argument */ + TK_A_OPTIONS_DB_PERSIST, /* no argument */ + TK_A_OPTIONS_INOTIFY_WATCH, /* boolean */ + TK_A_OPTIONS_DEVLINK_PRIORITY, /* int */ + TK_A_OWNER, /* user name */ + TK_A_GROUP, /* group name */ + TK_A_MODE, /* mode string */ + TK_A_OWNER_ID, /* uid_t */ + TK_A_GROUP_ID, /* gid_t */ + TK_A_MODE_ID, /* mode_t */ + TK_A_TAG, /* string */ + TK_A_OPTIONS_STATIC_NODE, /* device path, /dev/... */ + TK_A_SECLABEL, /* string with attribute */ + TK_A_ENV, /* string with attribute */ + TK_A_NAME, /* ifname */ + TK_A_DEVLINK, /* string */ + TK_A_ATTR, /* string with attribute */ + TK_A_SYSCTL, /* string with attribute */ + TK_A_RUN_BUILTIN, /* string */ + TK_A_RUN_PROGRAM, /* string */ + + _TK_TYPE_MAX, + _TK_TYPE_INVALID = -1, +} UdevRuleTokenType; + +typedef enum { + LINE_HAS_NAME = 1 << 0, /* has NAME= */ + LINE_HAS_DEVLINK = 1 << 1, /* has SYMLINK=, OWNER=, GROUP= or MODE= */ + LINE_HAS_STATIC_NODE = 1 << 2, /* has OPTIONS=static_node */ + LINE_HAS_GOTO = 1 << 3, /* has GOTO= */ + LINE_HAS_LABEL = 1 << 4, /* has LABEL= */ + LINE_UPDATE_SOMETHING = 1 << 5, /* has other TK_A_* or TK_M_IMPORT tokens */ +} UdevRuleLineType; + +typedef struct UdevRuleFile UdevRuleFile; +typedef struct UdevRuleLine UdevRuleLine; +typedef struct UdevRuleToken UdevRuleToken; + +struct UdevRuleToken { + UdevRuleTokenType type:8; + UdevRuleOperatorType op:8; + UdevRuleMatchType match_type:8; + UdevRuleSubstituteType attr_subst_type:7; + bool attr_match_remove_trailing_whitespace:1; + const char *value; + void *data; + LIST_FIELDS(UdevRuleToken, tokens); +}; + +struct UdevRuleLine { + char *line; + unsigned line_number; + UdevRuleLineType type; + + const char *label; + const char *goto_label; + UdevRuleLine *goto_line; + + UdevRuleFile *rule_file; + UdevRuleToken *current_token; + LIST_HEAD(UdevRuleToken, tokens); + LIST_FIELDS(UdevRuleLine, rule_lines); +}; + +struct UdevRuleFile { + char *filename; + UdevRuleLine *current_line; + LIST_HEAD(UdevRuleLine, rule_lines); + LIST_FIELDS(UdevRuleFile, rule_files); +}; + +struct UdevRules { + usec_t dirs_ts_usec; + ResolveNameTiming resolve_name_timing; + Hashmap *known_users; + Hashmap *known_groups; + UdevRuleFile *current_file; + LIST_HEAD(UdevRuleFile, rule_files); +}; + + /*** Logging helpers ***/ #define log_rule_full(device, rules, level, error, fmt, ...) \ diff --git a/src/udev/udev-rules.h b/src/udev/udev-rules.h index 52bf03b076..9fff5da7b5 100644 --- a/src/udev/udev-rules.h +++ b/src/udev/udev-rules.h @@ -1,46 +1,13 @@ /* SPDX-License-Identifier: GPL-2.0+ */ #pragma once -#include "device-util.h" #include "hashmap.h" -#include "list.h" #include "time-util.h" #include "udev-util.h" typedef struct UdevRules UdevRules; -typedef struct UdevRuleFile UdevRuleFile; -typedef struct UdevRuleLine UdevRuleLine; -typedef struct UdevRuleToken UdevRuleToken; typedef struct UdevEvent UdevEvent; -typedef enum { - OP_MATCH, /* == */ - OP_NOMATCH, /* != */ - OP_ADD, /* += */ - OP_REMOVE, /* -= */ - OP_ASSIGN, /* = */ - OP_ASSIGN_FINAL, /* := */ - _OP_TYPE_MAX, - _OP_TYPE_INVALID = -1 -} UdevRuleOperatorType; - -typedef enum { - MATCH_TYPE_EMPTY, /* empty string */ - MATCH_TYPE_PLAIN, /* no special characters */ - MATCH_TYPE_GLOB, /* shell globs ?,*,[] */ - MATCH_TYPE_SUBSYSTEM, /* "subsystem", "bus", or "class" */ - _MATCH_TYPE_MAX, - _MATCH_TYPE_INVALID = -1 -} UdevRuleMatchType; - -typedef enum { - SUBST_TYPE_PLAIN, /* no substitution */ - SUBST_TYPE_FORMAT, /* % or $ */ - SUBST_TYPE_SUBSYS, /* "[/]" format */ - _SUBST_TYPE_MAX, - _SUBST_TYPE_INVALID = -1 -} UdevRuleSubstituteType; - typedef enum { ESCAPE_UNSET, ESCAPE_NONE, /* OPTIONS="string_escape=none" */ @@ -49,118 +16,6 @@ typedef enum { _ESCAPE_TYPE_INVALID = -1 } UdevRuleEscapeType; -typedef enum { - /* lvalues which take match or nomatch operator */ - TK_M_ACTION, /* string, device_get_action() */ - TK_M_DEVPATH, /* path, sd_device_get_devpath() */ - TK_M_KERNEL, /* string, sd_device_get_sysname() */ - TK_M_DEVLINK, /* strv, sd_device_get_devlink_first(), sd_device_get_devlink_next() */ - TK_M_NAME, /* string, name of network interface */ - TK_M_ENV, /* string, device property, takes key through attribute */ - TK_M_TAG, /* strv, sd_device_get_tag_first(), sd_device_get_tag_next() */ - TK_M_SUBSYSTEM, /* string, sd_device_get_subsystem() */ - TK_M_DRIVER, /* string, sd_device_get_driver() */ - TK_M_ATTR, /* string, takes filename through attribute, sd_device_get_sysattr_value(), util_resolve_subsys_kernel(), etc. */ - TK_M_SYSCTL, /* string, takes kernel parameter through attribute */ - - /* matches parent paramters */ - TK_M_PARENTS_KERNEL, /* string */ - TK_M_PARENTS_SUBSYSTEM, /* string */ - TK_M_PARENTS_DRIVER, /* string */ - TK_M_PARENTS_ATTR, /* string */ - TK_M_PARENTS_TAG, /* strv */ - - TK_M_TEST, /* path, optionally mode_t can be specified by attribute, test the existence of a file */ - TK_M_PROGRAM, /* string, execute a program */ - TK_M_IMPORT_FILE, /* path */ - TK_M_IMPORT_PROGRAM, /* string, import properties from the result of program */ - TK_M_IMPORT_BUILTIN, /* string, import properties from the result of built-in command */ - TK_M_IMPORT_DB, /* string, import properties from database */ - TK_M_IMPORT_CMDLINE, /* string, kernel command line */ - TK_M_IMPORT_PARENT, /* string, parent property */ - TK_M_RESULT, /* string, result of TK_M_PROGRAM */ - -#define _TK_M_MAX (TK_M_RESULT + 1) -#define _TK_A_MIN _TK_M_MAX - - /* lvalues which take one of assign operators */ - TK_A_OPTIONS_STRING_ESCAPE_NONE, /* no argument */ - TK_A_OPTIONS_STRING_ESCAPE_REPLACE, /* no argument */ - TK_A_OPTIONS_DB_PERSIST, /* no argument */ - TK_A_OPTIONS_INOTIFY_WATCH, /* boolean */ - TK_A_OPTIONS_DEVLINK_PRIORITY, /* int */ - TK_A_OWNER, /* user name */ - TK_A_GROUP, /* group name */ - TK_A_MODE, /* mode string */ - TK_A_OWNER_ID, /* uid_t */ - TK_A_GROUP_ID, /* gid_t */ - TK_A_MODE_ID, /* mode_t */ - TK_A_TAG, /* string */ - TK_A_OPTIONS_STATIC_NODE, /* device path, /dev/... */ - TK_A_SECLABEL, /* string with attribute */ - TK_A_ENV, /* string with attribute */ - TK_A_NAME, /* ifname */ - TK_A_DEVLINK, /* string */ - TK_A_ATTR, /* string with attribute */ - TK_A_SYSCTL, /* string with attribute */ - TK_A_RUN_BUILTIN, /* string */ - TK_A_RUN_PROGRAM, /* string */ - - _TK_TYPE_MAX, - _TK_TYPE_INVALID = -1, -} UdevRuleTokenType; - -typedef enum { - LINE_HAS_NAME = 1 << 0, /* has NAME= */ - LINE_HAS_DEVLINK = 1 << 1, /* has SYMLINK=, OWNER=, GROUP= or MODE= */ - LINE_HAS_STATIC_NODE = 1 << 2, /* has OPTIONS=static_node */ - LINE_HAS_GOTO = 1 << 3, /* has GOTO= */ - LINE_HAS_LABEL = 1 << 4, /* has LABEL= */ - LINE_UPDATE_SOMETHING = 1 << 5, /* has other TK_A_* or TK_M_IMPORT tokens */ -} UdevRuleLineType; - -struct UdevRuleToken { - UdevRuleTokenType type:8; - UdevRuleOperatorType op:8; - UdevRuleMatchType match_type:8; - UdevRuleSubstituteType attr_subst_type:7; - bool attr_match_remove_trailing_whitespace:1; - const char *value; - void *data; - LIST_FIELDS(UdevRuleToken, tokens); -}; - -struct UdevRuleLine { - char *line; - unsigned line_number; - UdevRuleLineType type; - - const char *label; - const char *goto_label; - UdevRuleLine *goto_line; - - UdevRuleFile *rule_file; - UdevRuleToken *current_token; - LIST_HEAD(UdevRuleToken, tokens); - LIST_FIELDS(UdevRuleLine, rule_lines); -}; - -struct UdevRuleFile { - char *filename; - UdevRuleLine *current_line; - LIST_HEAD(UdevRuleLine, rule_lines); - LIST_FIELDS(UdevRuleFile, rule_files); -}; - -struct UdevRules { - usec_t dirs_ts_usec; - ResolveNameTiming resolve_name_timing; - Hashmap *known_users; - Hashmap *known_groups; - UdevRuleFile *current_file; - LIST_HEAD(UdevRuleFile, rule_files); -}; - int udev_rules_new(UdevRules **ret_rules, ResolveNameTiming resolve_name_timing); UdevRules *udev_rules_free(UdevRules *rules); DEFINE_TRIVIAL_CLEANUP_FUNC(UdevRules*, udev_rules_free); From 1824300add747ebe8dd9ab5d1a3b3eabeca9dd21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 27 Jun 2019 08:58:00 +0200 Subject: [PATCH 3/5] udev-event: use normal pointer ops where appropriate --- src/udev/udev-event.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 09eb56b97d..0f74fe340a 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -157,24 +157,24 @@ static int get_subst_type(const char **str, bool strict, FormatSubstitutionType assert(*str); assert(ret_type); - if (p[0] == '$') { + if (*p == '$') { p++; - if (p[0] == '$') { + if (*p == '$') { *str = p; return 0; } for (i = 0; i < ELEMENTSOF(map); i++) if ((q = startswith(p, map[i].name))) break; - } else if (p[0] == '%') { + } else if (*p == '%') { p++; - if (p[0] == '%') { + if (*p == '%') { *str = p; return 0; } for (i = 0; i < ELEMENTSOF(map); i++) - if (p[0] == map[i].fmt) { + if (*p == map[i].fmt) { q = p + 1; break; } @@ -184,7 +184,7 @@ static int get_subst_type(const char **str, bool strict, FormatSubstitutionType /* When 'strict' flag is set, then '$' and '%' must be escaped. */ return strict ? -EINVAL : 0; - if (q[0] == '{') { + if (*q == '{') { const char *start, *end; size_t len; @@ -200,7 +200,7 @@ static int get_subst_type(const char **str, bool strict, FormatSubstitutionType strnscpy(ret_attr, UTIL_PATH_SIZE, start, len); q = end + 1; } else - ret_attr[0] = '\0'; + *ret_attr = '\0'; *str = q; *ret_type = map[i].type; @@ -311,7 +311,7 @@ static ssize_t udev_event_subst_format( p = skip_leading_chars(event->program_result, NULL); for (i = 1; i < index; i++) { - while (p[0] != '\0' && !strchr(WHITESPACE, p[0])) + while (*p && !strchr(WHITESPACE, *p)) p++; p = skip_leading_chars(p, NULL); if (*p == '\0') @@ -327,7 +327,7 @@ static ssize_t udev_event_subst_format( if (has_plus) l = strpcpy(&s, l, start); else { - while (p[0] != '\0' && !strchr(WHITESPACE, p[0])) + while (*p && !strchr(WHITESPACE, *p)) p++; l = strnpcpy(&s, l, start, p - start); } @@ -447,7 +447,7 @@ ssize_t udev_event_apply_format(UdevEvent *event, assert(dest); assert(size > 0); - while (s[0] != '\0') { + while (*s) { FormatSubstitutionType type; char attr[UTIL_PATH_SIZE]; ssize_t subst_len; @@ -480,7 +480,7 @@ ssize_t udev_event_apply_format(UdevEvent *event, } assert(size >= 1); - dest[0] = '\0'; + *dest = '\0'; return size; } @@ -489,7 +489,7 @@ int udev_check_format(const char *s) { char attr[UTIL_PATH_SIZE]; int r; - while (s[0] != '\0') { + while (*s) { r = get_subst_type(&s, true, &type, attr); if (r < 0) return r; From f85cc54c4bbeca11783efdae4332a2759c8ac64c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 27 Jun 2019 10:47:56 +0200 Subject: [PATCH 4/5] 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. --- src/udev/udev-event.c | 25 ++++++++++++---- src/udev/udev-event.h | 2 +- src/udev/udev-rules.c | 69 ++++++++++++++++++++----------------------- 3 files changed, 53 insertions(+), 43 deletions(-) diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 0f74fe340a..025a37bc26 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 eca4ec8a9d..a05e125259 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 40f743a033..66208dc286 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")) { From 7504610ba08c412cc2349770d6caeaa668ccbd85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 28 Jun 2019 16:17:30 +0200 Subject: [PATCH 5/5] udev-rules: add more info to errors about attr formats --- src/udev/udev-rules.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 66208dc286..bbba2b49d2 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -224,10 +224,10 @@ struct UdevRules { #define log_token_invalid_op(rules, key) _log_token_invalid(rules, key, "operator") #define log_token_invalid_attr(rules, key) _log_token_invalid(rules, key, "attribute") -#define log_token_invalid_attr_format(rules, key, attr) \ +#define log_token_invalid_attr_format(rules, key, attr, offset, hint) \ log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL), \ - "Invalid attribute \"%s\" for %s, ignoring, but please fix it.", \ - attr, key) + "Invalid attribute \"%s\" for %s (char %zu: %s), ignoring, but please fix it.", \ + attr, key, offset, hint) #define log_token_invalid_value(rules, key, value, offset, hint) \ log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL), \ "Invalid value \"%s\" for %s (char %zu: %s), ignoring, but please fix it.", \ @@ -520,6 +520,17 @@ static void check_value_format_and_warn(UdevRules *rules, const char *key, const log_token_invalid_value(rules, key, value, offset + 1, hint); } +static int check_attr_format_and_warn(UdevRules *rules, const char *key, const char *value) { + size_t offset; + const char *hint; + + if (isempty(value)) + return log_token_invalid_attr(rules, key); + if (udev_check_format(value, &offset, &hint) < 0) + log_token_invalid_attr_format(rules, key, value, offset + 1, hint); + return 0; +} + 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; @@ -641,10 +652,9 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp r = rule_line_add_token(rule_line, TK_M_DRIVER, op, value, NULL); } else if (streq(key, "ATTR")) { - if (isempty(attr)) - return log_token_invalid_attr(rules, key); - if (udev_check_format(attr, NULL, NULL) < 0) - log_token_invalid_attr_format(rules, key, attr); + r = check_attr_format_and_warn(rules, key, attr); + if (r < 0) + return r; if (op == OP_REMOVE) return log_token_invalid_op(rules, key); if (IN_SET(op, OP_ADD, OP_ASSIGN_FINAL)) { @@ -658,10 +668,9 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp } 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, NULL, NULL) < 0) - log_token_invalid_attr_format(rules, key, attr); + r = check_attr_format_and_warn(rules, key, attr); + if (r < 0) + return r; if (op == OP_REMOVE) return log_token_invalid_op(rules, key); if (IN_SET(op, OP_ADD, OP_ASSIGN_FINAL)) { @@ -696,10 +705,9 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp r = rule_line_add_token(rule_line, TK_M_PARENTS_DRIVER, op, value, NULL); } else if (streq(key, "ATTRS")) { - if (isempty(attr)) - return log_token_invalid_attr(rules, key); - if (udev_check_format(attr, NULL, NULL) < 0) - log_token_invalid_attr_format(rules, key, attr); + r = check_attr_format_and_warn(rules, key, attr); + if (r < 0) + return r; if (!is_match) return log_token_invalid_op(rules, key);