From 56e3d0cf5d44054e79b815068d3fa09f51dc8ba2 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sat, 20 Feb 2016 16:28:42 -0500 Subject: test-siphash24: add a test for concatenating very short buffers coverity seems to think that our siphash code can read past the end of a short buffer. Add a test which adds very short buffers with different combinations of length to the hash. Hashing is done twice, once with zeros following "data", and once with some other bytes following "data". The two results are then compared to verify that the result does not depend on bytes past the specified data length. (This test passes.) --- src/test/test-siphash24.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/test/test-siphash24.c b/src/test/test-siphash24.c index caae911f30..b74b7ad2dd 100644 --- a/src/test/test-siphash24.c +++ b/src/test/test-siphash24.c @@ -22,9 +22,9 @@ #define ITERATIONS 10000000ULL -static int do_test(const uint8_t *in, size_t len, const uint8_t *key) { +static void do_test(const uint8_t *in, size_t len, const uint8_t *key) { struct siphash state = {}; - uint64_t out = 0; + uint64_t out; unsigned i, j; out = siphash24(in, len, key); @@ -60,7 +60,46 @@ static int do_test(const uint8_t *in, size_t len, const uint8_t *key) { assert_se(out == 0xa129ca6149be45e5); } } - return 0; +} + +static void test_short_hashes(void) { + const uint8_t one[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, + 0x09, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16 }; + const uint8_t key[16] = { 0x22, 0x24, 0x41, 0x22, 0x55, 0x77, 0x88, 0x07, + 0x23, 0x09, 0x23, 0x14, 0x0c, 0x33, 0x0e, 0x0f}; + uint8_t two[sizeof one] = {}; + + struct siphash state1 = {}, state2 = {}; + unsigned i, j; + + siphash24_init(&state1, key); + siphash24_init(&state2, key); + + /* hashing 1, 2, 3, 4, 5, ..., 16 bytes, with the byte after the buffer different */ + for (i = 1; i <= sizeof one; i++) { + siphash24_compress(one, i, &state1); + + two[i-1] = one[i-1]; + siphash24_compress(two, i, &state2); + + assert_se(memcmp(&state1, &state2, sizeof state1) == 0); + } + + /* hashing n and 1, n and 2, n and 3, ..., n-1 and 1, n-2 and 2, ... */ + for (i = sizeof one; i > 0; i--) { + zero(two); + + for (j = 1; j <= sizeof one; j++) { + siphash24_compress(one, i, &state1); + siphash24_compress(one, j, &state1); + + siphash24_compress(one, i, &state2); + two[j-1] = one[j-1]; + siphash24_compress(two, j, &state2); + + assert_se(memcmp(&state1, &state2, sizeof state1) == 0); + } + } } /* see https://131002.net/siphash/siphash.pdf, Appendix A */ @@ -80,4 +119,6 @@ int main(int argc, char *argv[]) { do_test(in_buf + 2, sizeof(in), key); memcpy(in_buf + 4, in, sizeof(in)); do_test(in_buf + 4, sizeof(in), key); + + test_short_hashes(); } -- cgit v1.2.3-54-g00ecf From a2daa2f075828630059bc28ceb40e29aef815e47 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sat, 20 Feb 2016 19:10:38 -0500 Subject: time-util: check for overflow in conversion from ts to nsec_t CID #1320855. --- src/basic/time-util.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/basic/time-util.c b/src/basic/time-util.c index 130acaa9de..9bfd8f4f7a 100644 --- a/src/basic/time-util.c +++ b/src/basic/time-util.c @@ -139,8 +139,7 @@ dual_timestamp* dual_timestamp_from_boottime_or_monotonic(dual_timestamp *ts, us usec_t timespec_load(const struct timespec *ts) { assert(ts); - if (ts->tv_sec == (time_t) -1 && - ts->tv_nsec == (long) -1) + if (ts->tv_sec == (time_t) -1 && ts->tv_nsec == (long) -1) return USEC_INFINITY; if ((usec_t) ts->tv_sec > (UINT64_MAX - (ts->tv_nsec / NSEC_PER_USEC)) / USEC_PER_SEC) @@ -154,13 +153,13 @@ usec_t timespec_load(const struct timespec *ts) { static nsec_t timespec_load_nsec(const struct timespec *ts) { assert(ts); - if (ts->tv_sec == (time_t) -1 && - ts->tv_nsec == (long) -1) + if (ts->tv_sec == (time_t) -1 && ts->tv_nsec == (long) -1) return NSEC_INFINITY; - return - (nsec_t) ts->tv_sec * NSEC_PER_SEC + - (nsec_t) ts->tv_nsec; + if ((nsec_t) ts->tv_sec >= (UINT64_MAX - ts->tv_nsec) / NSEC_PER_SEC) + return NSEC_INFINITY; + + return (nsec_t) ts->tv_sec * NSEC_PER_SEC + (nsec_t) ts->tv_nsec; } struct timespec *timespec_store(struct timespec *ts, usec_t u) { -- cgit v1.2.3-54-g00ecf From bccfe92e4600632c6ef65ae8eda6ac7782a6aebf Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sat, 20 Feb 2016 19:16:01 -0500 Subject: sd-device: use (void) before set_iterate calls set_iterate sets the output argument to NULL on error, and the return value is not used in this case. CID #1306804-09. --- src/libsystemd/sd-device/sd-device.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 9633e46ce0..fdd8c05e9c 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -1397,7 +1397,7 @@ _public_ const char *sd_device_get_tag_first(sd_device *device) { device->tags_iterator_generation = device->tags_generation; device->tags_iterator = ITERATOR_FIRST; - set_iterate(device->tags, &device->tags_iterator, &v); + (void) set_iterate(device->tags, &device->tags_iterator, &v); return v; } @@ -1411,7 +1411,7 @@ _public_ const char *sd_device_get_tag_next(sd_device *device) { if (device->tags_iterator_generation != device->tags_generation) return NULL; - set_iterate(device->tags, &device->tags_iterator, &v); + (void) set_iterate(device->tags, &device->tags_iterator, &v); return v; } @@ -1425,7 +1425,7 @@ _public_ const char *sd_device_get_devlink_first(sd_device *device) { device->devlinks_iterator_generation = device->devlinks_generation; device->devlinks_iterator = ITERATOR_FIRST; - set_iterate(device->devlinks, &device->devlinks_iterator, &v); + (void) set_iterate(device->devlinks, &device->devlinks_iterator, &v); return v; } @@ -1439,7 +1439,7 @@ _public_ const char *sd_device_get_devlink_next(sd_device *device) { if (device->devlinks_iterator_generation != device->devlinks_generation) return NULL; - set_iterate(device->devlinks, &device->devlinks_iterator, &v); + (void) set_iterate(device->devlinks, &device->devlinks_iterator, &v); return v; } @@ -1606,7 +1606,7 @@ _public_ const char *sd_device_get_sysattr_first(sd_device *device) { device->sysattrs_iterator = ITERATOR_FIRST; - set_iterate(device->sysattrs, &device->sysattrs_iterator, &v); + (void) set_iterate(device->sysattrs, &device->sysattrs_iterator, &v); return v; } @@ -1618,7 +1618,7 @@ _public_ const char *sd_device_get_sysattr_next(sd_device *device) { if (!device->sysattrs_read) return NULL; - set_iterate(device->sysattrs, &device->sysattrs_iterator, &v); + (void) set_iterate(device->sysattrs, &device->sysattrs_iterator, &v); return v; } -- cgit v1.2.3-54-g00ecf From 4fd6af76c43e235d036dc55fbe1bf0c151b4e7dd Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sat, 20 Feb 2016 20:09:34 -0500 Subject: udev-rules: log_oom() on memory error and abort processing of event CID #1313566. Also, change the return value to void, because it is ignored anyway. --- src/udev/udev-rules.c | 26 ++++++++++++++------------ src/udev/udev.h | 6 +++--- 2 files changed, 17 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index c06ace09cf..02efc08d2f 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -1010,8 +1010,8 @@ static int sort_token(struct udev_rules *rules, struct rule_tmp *rule_tmp) { return 0; } -static int add_rule(struct udev_rules *rules, char *line, - const char *filename, unsigned int filename_off, unsigned int lineno) { +static void add_rule(struct udev_rules *rules, char *line, + const char *filename, unsigned int filename_off, unsigned int lineno) { char *linepos; const char *attr; struct rule_tmp rule_tmp = { @@ -1547,10 +1547,9 @@ static int add_rule(struct udev_rules *rules, char *line, if (sort_token(rules, &rule_tmp) != 0) goto invalid; - return 0; + return; invalid: log_error("invalid rule '%s:%u'", filename, lineno); - return -1; } static int parse_file(struct udev_rules *rules, const char *filename) { @@ -1849,18 +1848,18 @@ enum escape_type { ESCAPE_REPLACE, }; -int udev_rules_apply_to_event(struct udev_rules *rules, - struct udev_event *event, - usec_t timeout_usec, - usec_t timeout_warn_usec, - struct udev_list *properties_list) { +void udev_rules_apply_to_event(struct udev_rules *rules, + struct udev_event *event, + usec_t timeout_usec, + usec_t timeout_warn_usec, + struct udev_list *properties_list) { struct token *cur; struct token *rule; enum escape_type esc = ESCAPE_UNSET; bool can_set_name; if (rules->tokens == NULL) - return -1; + return; can_set_name = ((!streq(udev_device_get_action(event->dev), "remove")) && (major(udev_device_get_devnum(event->dev)) > 0 || @@ -2434,7 +2433,10 @@ int udev_rules_apply_to_event(struct udev_rules *rules, rules_str(rules, rule->rule.filename_off), rule->rule.filename_line); break; } - free_and_strdup(&event->name, name_str); + if (free_and_strdup(&event->name, name_str) < 0) { + log_oom(); + return; + } log_debug("NAME '%s' %s:%u", event->name, rules_str(rules, rule->rule.filename_off), @@ -2546,7 +2548,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules, cur = &rules->tokens[cur->key.rule_goto]; continue; case TK_END: - return 0; + return; case TK_M_PARENTS_MIN: case TK_M_PARENTS_MAX: diff --git a/src/udev/udev.h b/src/udev/udev.h index 1f9c8120c0..655880346b 100644 --- a/src/udev/udev.h +++ b/src/udev/udev.h @@ -71,9 +71,9 @@ struct udev_rules; struct udev_rules *udev_rules_new(struct udev *udev, int resolve_names); struct udev_rules *udev_rules_unref(struct udev_rules *rules); bool udev_rules_check_timestamp(struct udev_rules *rules); -int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event, - usec_t timeout_usec, usec_t timeout_warn_usec, - struct udev_list *properties_list); +void udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event, + usec_t timeout_usec, usec_t timeout_warn_usec, + struct udev_list *properties_list); int udev_rules_apply_static_dev_perms(struct udev_rules *rules); /* udev-event.c */ -- cgit v1.2.3-54-g00ecf From 99f16bb8d9adf12fbfd2fbc7018fa02ce2c97187 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sat, 20 Feb 2016 20:40:41 -0500 Subject: udev-rules: modernize syntax a bit --- src/udev/udev-rules.c | 173 ++++++++++++++++---------------------------------- 1 file changed, 54 insertions(+), 119 deletions(-) (limited to 'src') diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 02efc08d2f..abfedfb613 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -1058,55 +1058,43 @@ static void add_rule(struct udev_rules *rules, char *line, goto invalid; } rule_add_key(&rule_tmp, TK_M_ACTION, op, value, NULL); - continue; - } - if (streq(key, "DEVPATH")) { + } else if (streq(key, "DEVPATH")) { if (op > OP_MATCH_MAX) { log_error("invalid DEVPATH operation"); goto invalid; } rule_add_key(&rule_tmp, TK_M_DEVPATH, op, value, NULL); - continue; - } - if (streq(key, "KERNEL")) { + } else if (streq(key, "KERNEL")) { if (op > OP_MATCH_MAX) { log_error("invalid KERNEL operation"); goto invalid; } rule_add_key(&rule_tmp, TK_M_KERNEL, op, value, NULL); - continue; - } - if (streq(key, "SUBSYSTEM")) { + } else if (streq(key, "SUBSYSTEM")) { if (op > OP_MATCH_MAX) { log_error("invalid SUBSYSTEM operation"); goto invalid; } /* bus, class, subsystem events should all be the same */ - if (streq(value, "subsystem") || - streq(value, "bus") || - streq(value, "class")) { - if (streq(value, "bus") || streq(value, "class")) - log_error("'%s' must be specified as 'subsystem' " - "please fix it in %s:%u", value, filename, lineno); + if (STR_IN_SET(value, "subsystem", "bus", "class")) { + if (!streq(value, "subsystem")) + log_error("'%s' must be specified as 'subsystem'; please fix it in %s:%u", + value, filename, lineno); rule_add_key(&rule_tmp, TK_M_SUBSYSTEM, op, "subsystem|class|bus", NULL); } else rule_add_key(&rule_tmp, TK_M_SUBSYSTEM, op, value, NULL); - continue; - } - if (streq(key, "DRIVER")) { + } else if (streq(key, "DRIVER")) { if (op > OP_MATCH_MAX) { log_error("invalid DRIVER operation"); goto invalid; } rule_add_key(&rule_tmp, TK_M_DRIVER, op, value, NULL); - continue; - } - if (startswith(key, "ATTR{")) { + } else if (startswith(key, "ATTR{")) { attr = get_key_attribute(rules->udev, key + strlen("ATTR")); if (attr == NULL) { log_error("error parsing ATTR attribute"); @@ -1120,10 +1108,8 @@ static void add_rule(struct udev_rules *rules, char *line, rule_add_key(&rule_tmp, TK_M_ATTR, op, value, attr); else rule_add_key(&rule_tmp, TK_A_ATTR, op, value, attr); - continue; - } - if (startswith(key, "SYSCTL{")) { + } else if (startswith(key, "SYSCTL{")) { attr = get_key_attribute(rules->udev, key + strlen("SYSCTL")); if (attr == NULL) { log_error("error parsing SYSCTL attribute"); @@ -1137,10 +1123,8 @@ static void add_rule(struct udev_rules *rules, char *line, rule_add_key(&rule_tmp, TK_M_SYSCTL, op, value, attr); else rule_add_key(&rule_tmp, TK_A_SYSCTL, op, value, attr); - continue; - } - if (startswith(key, "SECLABEL{")) { + } else if (startswith(key, "SECLABEL{")) { attr = get_key_attribute(rules->udev, key + strlen("SECLABEL")); if (!attr) { log_error("error parsing SECLABEL attribute"); @@ -1152,37 +1136,29 @@ static void add_rule(struct udev_rules *rules, char *line, } rule_add_key(&rule_tmp, TK_A_SECLABEL, op, value, attr); - continue; - } - if (streq(key, "KERNELS")) { + } else if (streq(key, "KERNELS")) { if (op > OP_MATCH_MAX) { log_error("invalid KERNELS operation"); goto invalid; } rule_add_key(&rule_tmp, TK_M_KERNELS, op, value, NULL); - continue; - } - if (streq(key, "SUBSYSTEMS")) { + } else if (streq(key, "SUBSYSTEMS")) { if (op > OP_MATCH_MAX) { log_error("invalid SUBSYSTEMS operation"); goto invalid; } rule_add_key(&rule_tmp, TK_M_SUBSYSTEMS, op, value, NULL); - continue; - } - if (streq(key, "DRIVERS")) { + } else if (streq(key, "DRIVERS")) { if (op > OP_MATCH_MAX) { log_error("invalid DRIVERS operation"); goto invalid; } rule_add_key(&rule_tmp, TK_M_DRIVERS, op, value, NULL); - continue; - } - if (startswith(key, "ATTRS{")) { + } else if (startswith(key, "ATTRS{")) { if (op > OP_MATCH_MAX) { log_error("invalid ATTRS operation"); goto invalid; @@ -1199,19 +1175,15 @@ static void add_rule(struct udev_rules *rules, char *line, log_error("do not reference parent sysfs directories directly, " "it may break with a future kernel, please fix it in %s:%u", filename, lineno); rule_add_key(&rule_tmp, TK_M_ATTRS, op, value, attr); - continue; - } - if (streq(key, "TAGS")) { + } else if (streq(key, "TAGS")) { if (op > OP_MATCH_MAX) { log_error("invalid TAGS operation"); goto invalid; } rule_add_key(&rule_tmp, TK_M_TAGS, op, value, NULL); - continue; - } - if (startswith(key, "ENV{")) { + } else if (startswith(key, "ENV{")) { attr = get_key_attribute(rules->udev, key + strlen("ENV")); if (attr == NULL) { log_error("error parsing ENV attribute"); @@ -1225,60 +1197,46 @@ static void add_rule(struct udev_rules *rules, char *line, if (rule_add_key(&rule_tmp, TK_M_ENV, op, value, attr) != 0) goto invalid; } else { - static const char *blacklist[] = { - "ACTION", - "SUBSYSTEM", - "DEVTYPE", - "MAJOR", - "MINOR", - "DRIVER", - "IFINDEX", - "DEVNAME", - "DEVLINKS", - "DEVPATH", - "TAGS", - }; - unsigned int i; - - for (i = 0; i < ELEMENTSOF(blacklist); i++) { - if (!streq(attr, blacklist[i])) - continue; + if (STR_IN_SET(attr, + "ACTION", + "SUBSYSTEM", + "DEVTYPE", + "MAJOR", + "MINOR", + "DRIVER", + "IFINDEX", + "DEVNAME", + "DEVLINKS", + "DEVPATH", + "TAGS")) { log_error("invalid ENV attribute, '%s' can not be set %s:%u", attr, filename, lineno); goto invalid; } if (rule_add_key(&rule_tmp, TK_A_ENV, op, value, attr) != 0) goto invalid; } - continue; - } - if (streq(key, "TAG")) { + } else if (streq(key, "TAG")) { if (op < OP_MATCH_MAX) rule_add_key(&rule_tmp, TK_M_TAG, op, value, NULL); else rule_add_key(&rule_tmp, TK_A_TAG, op, value, NULL); - continue; - } - if (streq(key, "PROGRAM")) { + } else if (streq(key, "PROGRAM")) { if (op == OP_REMOVE) { log_error("invalid PROGRAM operation"); goto invalid; } rule_add_key(&rule_tmp, TK_M_PROGRAM, op, value, NULL); - continue; - } - if (streq(key, "RESULT")) { + } else if (streq(key, "RESULT")) { if (op > OP_MATCH_MAX) { log_error("invalid RESULT operation"); goto invalid; } rule_add_key(&rule_tmp, TK_M_RESULT, op, value, NULL); - continue; - } - if (startswith(key, "IMPORT")) { + } else if (startswith(key, "IMPORT")) { attr = get_key_attribute(rules->udev, key + strlen("IMPORT")); if (attr == NULL) { log_error("IMPORT{} type missing, ignoring IMPORT %s:%u", filename, lineno); @@ -1319,10 +1277,8 @@ static void add_rule(struct udev_rules *rules, char *line, rule_add_key(&rule_tmp, TK_M_IMPORT_PARENT, op, value, NULL); } else log_error("IMPORT{} unknown type, ignoring IMPORT %s:%u", filename, lineno); - continue; - } - if (startswith(key, "TEST")) { + } else if (startswith(key, "TEST")) { mode_t mode = 0; if (op > OP_MATCH_MAX) { @@ -1333,13 +1289,10 @@ static void add_rule(struct udev_rules *rules, char *line, if (attr != NULL) { mode = strtol(attr, NULL, 8); rule_add_key(&rule_tmp, TK_M_TEST, op, value, &mode); - } else { + } else rule_add_key(&rule_tmp, TK_M_TEST, op, value, NULL); - } - continue; - } - if (startswith(key, "RUN")) { + } else if (startswith(key, "RUN")) { attr = get_key_attribute(rules->udev, key + strlen("RUN")); if (attr == NULL) attr = "program"; @@ -1356,35 +1309,27 @@ static void add_rule(struct udev_rules *rules, char *line, else log_error("RUN{builtin}: '%s' unknown %s:%u", value, filename, lineno); } else if (streq(attr, "program")) { - enum udev_builtin_cmd cmd = UDEV_BUILTIN_MAX; + const enum udev_builtin_cmd cmd = UDEV_BUILTIN_MAX; rule_add_key(&rule_tmp, TK_A_RUN_PROGRAM, op, value, &cmd); - } else { + } else log_error("RUN{} unknown type, ignoring RUN %s:%u", filename, lineno); - } - - continue; - } - if (streq(key, "LABEL")) { + } else if (streq(key, "LABEL")) { if (op == OP_REMOVE) { log_error("invalid LABEL operation"); goto invalid; } rule_tmp.rule.rule.label_off = rules_add_string(rules, value); - continue; - } - if (streq(key, "GOTO")) { + } else if (streq(key, "GOTO")) { if (op == OP_REMOVE) { log_error("invalid GOTO operation"); goto invalid; } rule_add_key(&rule_tmp, TK_A_GOTO, 0, value, NULL); - continue; - } - if (startswith(key, "NAME")) { + } else if (startswith(key, "NAME")) { if (op == OP_REMOVE) { log_error("invalid NAME operation"); goto invalid; @@ -1405,10 +1350,8 @@ static void add_rule(struct udev_rules *rules, char *line, rule_add_key(&rule_tmp, TK_A_NAME, op, value, NULL); } rule_tmp.rule.rule.can_set_name = true; - continue; - } - if (streq(key, "SYMLINK")) { + } else if (streq(key, "SYMLINK")) { if (op == OP_REMOVE) { log_error("invalid SYMLINK operation"); goto invalid; @@ -1418,10 +1361,8 @@ static void add_rule(struct udev_rules *rules, char *line, else rule_add_key(&rule_tmp, TK_A_DEVLINK, op, value, NULL); rule_tmp.rule.rule.can_set_name = true; - continue; - } - if (streq(key, "OWNER")) { + } else if (streq(key, "OWNER")) { uid_t uid; char *endptr; @@ -1440,10 +1381,8 @@ static void add_rule(struct udev_rules *rules, char *line, rule_add_key(&rule_tmp, TK_A_OWNER, op, value, NULL); rule_tmp.rule.rule.can_set_name = true; - continue; - } - if (streq(key, "GROUP")) { + } else if (streq(key, "GROUP")) { gid_t gid; char *endptr; @@ -1462,10 +1401,8 @@ static void add_rule(struct udev_rules *rules, char *line, rule_add_key(&rule_tmp, TK_A_GROUP, op, value, NULL); rule_tmp.rule.rule.can_set_name = true; - continue; - } - if (streq(key, "MODE")) { + } else if (streq(key, "MODE")) { mode_t mode; char *endptr; @@ -1480,10 +1417,8 @@ static void add_rule(struct udev_rules *rules, char *line, else rule_add_key(&rule_tmp, TK_A_MODE, op, value, NULL); rule_tmp.rule.rule.can_set_name = true; - continue; - } - if (streq(key, "OPTIONS")) { + } else if (streq(key, "OPTIONS")) { const char *pos; if (op == OP_REMOVE) { @@ -1531,11 +1466,10 @@ static void add_rule(struct udev_rules *rules, char *line, rule_tmp.rule.rule.has_static_node = true; } - continue; + } else { + log_error("unknown key '%s' in %s:%u", key, filename, lineno); + goto invalid; } - - log_error("unknown key '%s' in %s:%u", key, filename, lineno); - goto invalid; } /* add rule token */ @@ -2427,10 +2361,11 @@ void udev_rules_apply_to_event(struct udev_rules *rules, log_debug("%i character(s) replaced", count); } if (major(udev_device_get_devnum(event->dev)) && - (!streq(name_str, udev_device_get_devnode(event->dev) + strlen("/dev/")))) { - log_error("NAME=\"%s\" ignored, kernel device nodes " - "can not be renamed; please fix it in %s:%u\n", name, - rules_str(rules, rule->rule.filename_off), rule->rule.filename_line); + !streq(name_str, udev_device_get_devnode(event->dev) + strlen("/dev/"))) { + log_error("NAME=\"%s\" ignored, kernel device nodes cannot be renamed; please fix it in %s:%u\n", + name, + rules_str(rules, rule->rule.filename_off), + rule->rule.filename_line); break; } if (free_and_strdup(&event->name, name_str) < 0) { -- cgit v1.2.3-54-g00ecf From f4850a1d9593fd0f226e507c353ebdd46c069070 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sat, 20 Feb 2016 23:00:45 -0500 Subject: udev-rules: rewrite function to avoid clobbering arguments If the attribute wasn't found, the last filename looked at was returned in the input/output argument. This just seems bad style. The return value was ignored, so change function to return void. --- src/udev/udev-rules.c | 49 ++++++++++++++++++++----------------------------- 1 file changed, 20 insertions(+), 29 deletions(-) (limited to 'src') diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index abfedfb613..689da7451c 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -36,6 +36,7 @@ #include "glob-util.h" #include "path-util.h" #include "stat-util.h" +#include "stdio-util.h" #include "strbuf.h" #include "string-util.h" #include "strv.h" @@ -686,41 +687,31 @@ static int import_parent_into_properties(struct udev_device *dev, const char *fi return 0; } -static int attr_subst_subdir(char *attr, size_t len) { - bool found = false; +static void attr_subst_subdir(char *attr, size_t len) { + const char *pos, *tail, *path; + _cleanup_closedir_ DIR *dir = NULL; + struct dirent *dent; - if (strstr(attr, "/*/")) { - char *pos; - char dirname[UTIL_PATH_SIZE]; - const char *tail; - DIR *dir; + pos = strstr(attr, "/*/"); + if (!pos) + return; - strscpy(dirname, sizeof(dirname), attr); - pos = strstr(dirname, "/*/"); - if (pos == NULL) - return -1; - pos[0] = '\0'; - tail = &pos[2]; - dir = opendir(dirname); - if (dir != NULL) { - struct dirent *dent; + tail = pos + 2; + path = strndupa(attr, pos - attr + 1); /* include slash at end */ + dir = opendir(path); + if (dir == NULL) + return; - for (dent = readdir(dir); dent != NULL; dent = readdir(dir)) { - struct stat stats; + for (dent = readdir(dir); dent != NULL; dent = readdir(dir)) + if (dent->d_name[0] != '.') { + char n[strlen(dent->d_name) + 1 + strlen(tail) + 1]; - if (dent->d_name[0] == '.') - continue; - strscpyl(attr, len, dirname, "/", dent->d_name, tail, NULL); - if (stat(attr, &stats) == 0) { - found = true; - break; - } + strscpyl(n, sizeof n, dent->d_name, "/", tail, NULL); + if (faccessat(dirfd(dir), n, F_OK, 0)) { + strscpyl(attr, len, path, n, NULL); + break; } - closedir(dir); } - } - - return found; } static int get_key(struct udev *udev, char **line, char **key, enum operation_type *op, char **value) { -- cgit v1.2.3-54-g00ecf From 19a8e656a96d63c0b0047a84db980dd57ba51df2 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 21 Feb 2016 00:26:32 -0500 Subject: udev-rules: make error messages about rules more uniform Also downgrade non-fatal warnings to log_warning. Previously rule_add_key() would check the output array and log a cryptic error and return -1. Most of the time the return value was ignored. This does not seems right, because the buffer can overflow with enough rules. It would also check if we have enough space for the *next* rule, even if there might be not next rule, i.e. off-by-one. Replace this with a check that we have enough space for a next rule before we start parsing. Normally using macros to alter flow is not allowed, but in this case I think it is worth it, because it allows lots of boilerplate code to be removed and hides repeated boring parameters, making function logic much easier to follow. --- src/udev/udev-rules.c | 353 +++++++++++++++++++++----------------------------- 1 file changed, 150 insertions(+), 203 deletions(-) (limited to 'src') diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 689da7451c..5c39c9b874 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -822,12 +822,13 @@ static const char *get_key_attribute(struct udev *udev, char *str) { return NULL; } -static int rule_add_key(struct rule_tmp *rule_tmp, enum token_type type, - enum operation_type op, - const char *value, const void *data) { - struct token *token = &rule_tmp->token[rule_tmp->token_cur]; +static void rule_add_key(struct rule_tmp *rule_tmp, enum token_type type, + enum operation_type op, + const char *value, const void *data) { + struct token *token = rule_tmp->token + rule_tmp->token_cur; const char *attr = NULL; + assert(rule_tmp->token_cur < ELEMENTSOF(rule_tmp->token)); memzero(token, sizeof(struct token)); switch (type) { @@ -910,8 +911,7 @@ static int rule_add_key(struct rule_tmp *rule_tmp, enum token_type type, case TK_M_MAX: case TK_END: case TK_UNSET: - log_error("wrong type %u", type); - return -1; + assert_not_reached("wrong type"); } if (value != NULL && type < TK_M_MAX) { @@ -960,11 +960,6 @@ static int rule_add_key(struct rule_tmp *rule_tmp, enum token_type type, token->key.type = type; token->key.op = op; rule_tmp->token_cur++; - if (rule_tmp->token_cur >= ELEMENTSOF(rule_tmp->token)) { - log_error("temporary rule array too small"); - return -1; - } - return 0; } static int sort_token(struct udev_rules *rules, struct rule_tmp *rule_tmp) { @@ -1001,6 +996,11 @@ static int sort_token(struct udev_rules *rules, struct rule_tmp *rule_tmp) { return 0; } +#define LOG_RULE_ERROR(fmt, ...) log_error("Invalid rule %s:%u: " fmt, filename, lineno, ##__VA_ARGS__) +#define LOG_RULE_WARNING(fmt, ...) log_warning("%s:%u: " fmt, filename, lineno, ##__VA_ARGS__) +#define LOG_RULE_DEBUG(fmt, ...) log_debug("%s:%u: " fmt, filename, lineno, ##__VA_ARGS__) +#define LOG_AND_RETURN(fmt, ...) { LOG_RULE_ERROR(fmt, __VA_ARGS__); return; } + static void add_rule(struct udev_rules *rules, char *line, const char *filename, unsigned int filename_off, unsigned int lineno) { char *linepos; @@ -1043,58 +1043,54 @@ static void add_rule(struct udev_rules *rules, char *line, break; } + if (rule_tmp.token_cur >= ELEMENTSOF(rule_tmp.token)) + LOG_AND_RETURN("temporary rule array too small, aborting event processing with %u items", rule_tmp.token_cur); + if (streq(key, "ACTION")) { - if (op > OP_MATCH_MAX) { - log_error("invalid ACTION operation"); - goto invalid; - } + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_ACTION, op, value, NULL); } else if (streq(key, "DEVPATH")) { - if (op > OP_MATCH_MAX) { - log_error("invalid DEVPATH operation"); - goto invalid; - } + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_DEVPATH, op, value, NULL); } else if (streq(key, "KERNEL")) { - if (op > OP_MATCH_MAX) { - log_error("invalid KERNEL operation"); - goto invalid; - } + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_KERNEL, op, value, NULL); } else if (streq(key, "SUBSYSTEM")) { - if (op > OP_MATCH_MAX) { - log_error("invalid SUBSYSTEM operation"); - goto invalid; - } + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + /* bus, class, subsystem events should all be the same */ if (STR_IN_SET(value, "subsystem", "bus", "class")) { if (!streq(value, "subsystem")) - log_error("'%s' must be specified as 'subsystem'; please fix it in %s:%u", - value, filename, lineno); + LOG_RULE_WARNING("'%s' must be specified as 'subsystem'; please fix", value); + rule_add_key(&rule_tmp, TK_M_SUBSYSTEM, op, "subsystem|class|bus", NULL); } else rule_add_key(&rule_tmp, TK_M_SUBSYSTEM, op, value, NULL); } else if (streq(key, "DRIVER")) { - if (op > OP_MATCH_MAX) { - log_error("invalid DRIVER operation"); - goto invalid; - } + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_DRIVER, op, value, NULL); } else if (startswith(key, "ATTR{")) { attr = get_key_attribute(rules->udev, key + strlen("ATTR")); - if (attr == NULL) { - log_error("error parsing ATTR attribute"); - goto invalid; - } - if (op == OP_REMOVE) { - log_error("invalid ATTR operation"); - goto invalid; - } + if (attr == NULL) + LOG_AND_RETURN("error parsing %s attribute", "ATTR"); + + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", "ATTR"); + if (op < OP_MATCH_MAX) rule_add_key(&rule_tmp, TK_M_ATTR, op, value, attr); else @@ -1102,14 +1098,12 @@ static void add_rule(struct udev_rules *rules, char *line, } else if (startswith(key, "SYSCTL{")) { attr = get_key_attribute(rules->udev, key + strlen("SYSCTL")); - if (attr == NULL) { - log_error("error parsing SYSCTL attribute"); - goto invalid; - } - if (op == OP_REMOVE) { - log_error("invalid SYSCTL operation"); - goto invalid; - } + if (attr == NULL) + LOG_AND_RETURN("error parsing %s attribute", "ATTR"); + + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", "ATTR"); + if (op < OP_MATCH_MAX) rule_add_key(&rule_tmp, TK_M_SYSCTL, op, value, attr); else @@ -1117,77 +1111,63 @@ static void add_rule(struct udev_rules *rules, char *line, } else if (startswith(key, "SECLABEL{")) { attr = get_key_attribute(rules->udev, key + strlen("SECLABEL")); - if (!attr) { - log_error("error parsing SECLABEL attribute"); - goto invalid; - } - if (op == OP_REMOVE) { - log_error("invalid SECLABEL operation"); - goto invalid; - } + if (attr == NULL) + LOG_AND_RETURN("error parsing %s attribute", "SECLABEL"); + + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", "SECLABEL"); rule_add_key(&rule_tmp, TK_A_SECLABEL, op, value, attr); } else if (streq(key, "KERNELS")) { - if (op > OP_MATCH_MAX) { - log_error("invalid KERNELS operation"); - goto invalid; - } + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_KERNELS, op, value, NULL); } else if (streq(key, "SUBSYSTEMS")) { - if (op > OP_MATCH_MAX) { - log_error("invalid SUBSYSTEMS operation"); - goto invalid; - } + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_SUBSYSTEMS, op, value, NULL); } else if (streq(key, "DRIVERS")) { - if (op > OP_MATCH_MAX) { - log_error("invalid DRIVERS operation"); - goto invalid; - } + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_DRIVERS, op, value, NULL); } else if (startswith(key, "ATTRS{")) { - if (op > OP_MATCH_MAX) { - log_error("invalid ATTRS operation"); - goto invalid; - } + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", "ATTRS"); + attr = get_key_attribute(rules->udev, key + strlen("ATTRS")); - if (attr == NULL) { - log_error("error parsing ATTRS attribute"); - goto invalid; - } + if (attr == NULL) + LOG_AND_RETURN("error parsing %s attribute", "ATTRS"); + if (startswith(attr, "device/")) - log_error("the 'device' link may not be available in a future kernel, " - "please fix it in %s:%u", filename, lineno); - else if (strstr(attr, "../") != NULL) - log_error("do not reference parent sysfs directories directly, " - "it may break with a future kernel, please fix it in %s:%u", filename, lineno); + LOG_RULE_WARNING("'device' link may not be available in future kernels; please fix"); + if (strstr(attr, "../") != NULL) + LOG_RULE_WARNING("direct reference to parent sysfs directory, may break in future kernels; please fix"); rule_add_key(&rule_tmp, TK_M_ATTRS, op, value, attr); } else if (streq(key, "TAGS")) { - if (op > OP_MATCH_MAX) { - log_error("invalid TAGS operation"); - goto invalid; - } + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_TAGS, op, value, NULL); } else if (startswith(key, "ENV{")) { attr = get_key_attribute(rules->udev, key + strlen("ENV")); - if (attr == NULL) { - log_error("error parsing ENV attribute"); - goto invalid; - } - if (op == OP_REMOVE) { - log_error("invalid ENV operation"); - goto invalid; - } - if (op < OP_MATCH_MAX) { - if (rule_add_key(&rule_tmp, TK_M_ENV, op, value, attr) != 0) - goto invalid; - } else { + if (attr == NULL) + LOG_AND_RETURN("error parsing %s attribute", "ENV"); + + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", "ENV"); + + if (op < OP_MATCH_MAX) + rule_add_key(&rule_tmp, TK_M_ENV, op, value, attr); + else { if (STR_IN_SET(attr, "ACTION", "SUBSYSTEM", @@ -1199,12 +1179,10 @@ static void add_rule(struct udev_rules *rules, char *line, "DEVNAME", "DEVLINKS", "DEVPATH", - "TAGS")) { - log_error("invalid ENV attribute, '%s' can not be set %s:%u", attr, filename, lineno); - goto invalid; - } - if (rule_add_key(&rule_tmp, TK_A_ENV, op, value, attr) != 0) - goto invalid; + "TAGS")) + LOG_AND_RETURN("invalid ENV attribute, '%s' cannot be set", attr); + + rule_add_key(&rule_tmp, TK_A_ENV, op, value, attr); } } else if (streq(key, "TAG")) { @@ -1214,68 +1192,62 @@ static void add_rule(struct udev_rules *rules, char *line, rule_add_key(&rule_tmp, TK_A_TAG, op, value, NULL); } else if (streq(key, "PROGRAM")) { - if (op == OP_REMOVE) { - log_error("invalid PROGRAM operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_PROGRAM, op, value, NULL); } else if (streq(key, "RESULT")) { - if (op > OP_MATCH_MAX) { - log_error("invalid RESULT operation"); - goto invalid; - } + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_RESULT, op, value, NULL); } else if (startswith(key, "IMPORT")) { attr = get_key_attribute(rules->udev, key + strlen("IMPORT")); if (attr == NULL) { - log_error("IMPORT{} type missing, ignoring IMPORT %s:%u", filename, lineno); + LOG_RULE_WARNING("ignoring IMPORT{} with missing type"); continue; } - if (op == OP_REMOVE) { - log_error("invalid IMPORT operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", "IMPORT"); + if (streq(attr, "program")) { /* find known built-in command */ if (value[0] != '/') { - enum udev_builtin_cmd cmd; + const enum udev_builtin_cmd cmd = udev_builtin_lookup(value); - cmd = udev_builtin_lookup(value); if (cmd < UDEV_BUILTIN_MAX) { - log_debug("IMPORT found builtin '%s', replacing %s:%u", - value, filename, lineno); + LOG_RULE_DEBUG("IMPORT found builtin '%s', replacing", value); rule_add_key(&rule_tmp, TK_M_IMPORT_BUILTIN, op, value, &cmd); continue; } } rule_add_key(&rule_tmp, TK_M_IMPORT_PROG, op, value, NULL); } else if (streq(attr, "builtin")) { - enum udev_builtin_cmd cmd = udev_builtin_lookup(value); + const enum udev_builtin_cmd cmd = udev_builtin_lookup(value); - if (cmd < UDEV_BUILTIN_MAX) - rule_add_key(&rule_tmp, TK_M_IMPORT_BUILTIN, op, value, &cmd); + if (cmd >= UDEV_BUILTIN_MAX) + LOG_RULE_WARNING("IMPORT{builtin} '%s' unknown", value); else - log_error("IMPORT{builtin}: '%s' unknown %s:%u", value, filename, lineno); - } else if (streq(attr, "file")) { + rule_add_key(&rule_tmp, TK_M_IMPORT_BUILTIN, op, value, &cmd); + } else if (streq(attr, "file")) rule_add_key(&rule_tmp, TK_M_IMPORT_FILE, op, value, NULL); - } else if (streq(attr, "db")) { + else if (streq(attr, "db")) rule_add_key(&rule_tmp, TK_M_IMPORT_DB, op, value, NULL); - } else if (streq(attr, "cmdline")) { + else if (streq(attr, "cmdline")) rule_add_key(&rule_tmp, TK_M_IMPORT_CMDLINE, op, value, NULL); - } else if (streq(attr, "parent")) { + else if (streq(attr, "parent")) rule_add_key(&rule_tmp, TK_M_IMPORT_PARENT, op, value, NULL); - } else - log_error("IMPORT{} unknown type, ignoring IMPORT %s:%u", filename, lineno); + else + LOG_RULE_ERROR("ignoring unknown %s{} type '%s'", "IMPORT", attr); } else if (startswith(key, "TEST")) { mode_t mode = 0; - if (op > OP_MATCH_MAX) { - log_error("invalid TEST operation"); - goto invalid; - } + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", "TEST"); + attr = get_key_attribute(rules->udev, key + strlen("TEST")); if (attr != NULL) { mode = strtol(attr, NULL, 8); @@ -1287,55 +1259,48 @@ static void add_rule(struct udev_rules *rules, char *line, attr = get_key_attribute(rules->udev, key + strlen("RUN")); if (attr == NULL) attr = "program"; - if (op == OP_REMOVE) { - log_error("invalid RUN operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", "RUN"); if (streq(attr, "builtin")) { - enum udev_builtin_cmd cmd = udev_builtin_lookup(value); + const enum udev_builtin_cmd cmd = udev_builtin_lookup(value); if (cmd < UDEV_BUILTIN_MAX) rule_add_key(&rule_tmp, TK_A_RUN_BUILTIN, op, value, &cmd); else - log_error("RUN{builtin}: '%s' unknown %s:%u", value, filename, lineno); + LOG_RULE_ERROR("RUN{builtin}: '%s' unknown", value); } else if (streq(attr, "program")) { const enum udev_builtin_cmd cmd = UDEV_BUILTIN_MAX; rule_add_key(&rule_tmp, TK_A_RUN_PROGRAM, op, value, &cmd); } else - log_error("RUN{} unknown type, ignoring RUN %s:%u", filename, lineno); + LOG_RULE_ERROR("ignoring unknown %s{} type '%s'", "RUN", attr); } else if (streq(key, "LABEL")) { - if (op == OP_REMOVE) { - log_error("invalid LABEL operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); + rule_tmp.rule.rule.label_off = rules_add_string(rules, value); } else if (streq(key, "GOTO")) { - if (op == OP_REMOVE) { - log_error("invalid GOTO operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_A_GOTO, 0, value, NULL); } else if (startswith(key, "NAME")) { - if (op == OP_REMOVE) { - log_error("invalid NAME operation"); - goto invalid; - } - if (op < OP_MATCH_MAX) { + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); + + if (op < OP_MATCH_MAX) rule_add_key(&rule_tmp, TK_M_NAME, op, value, NULL); - } else { + else { if (streq(value, "%k")) { - log_error("NAME=\"%%k\" is ignored, because it breaks kernel supplied names, " - "please remove it from %s:%u\n", filename, lineno); + LOG_RULE_WARNING("NAME=\"%%k\" is ignored, because it breaks kernel supplied names; please remove"); continue; } - if (value[0] == '\0') { - log_debug("NAME=\"\" is ignored, because udev will not delete any device nodes, " - "please remove it from %s:%u\n", filename, lineno); + if (isempty(value)) { + LOG_RULE_DEBUG("NAME=\"\" is ignored, because udev will not delete any device nodes; please remove"); continue; } rule_add_key(&rule_tmp, TK_A_NAME, op, value, NULL); @@ -1343,10 +1308,9 @@ static void add_rule(struct udev_rules *rules, char *line, rule_tmp.rule.rule.can_set_name = true; } else if (streq(key, "SYMLINK")) { - if (op == OP_REMOVE) { - log_error("invalid SYMLINK operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); + if (op < OP_MATCH_MAX) rule_add_key(&rule_tmp, TK_M_DEVLINK, op, value, NULL); else @@ -1357,15 +1321,13 @@ static void add_rule(struct udev_rules *rules, char *line, uid_t uid; char *endptr; - if (op == OP_REMOVE) { - log_error("invalid OWNER operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); uid = strtoul(value, &endptr, 10); - if (endptr[0] == '\0') { + if (endptr[0] == '\0') rule_add_key(&rule_tmp, TK_A_OWNER_ID, op, NULL, &uid); - } else if ((rules->resolve_names > 0) && strchr("$%", value[0]) == NULL) { + else if (rules->resolve_names > 0 && strchr("$%", value[0]) == NULL) { uid = add_uid(rules, value); rule_add_key(&rule_tmp, TK_A_OWNER_ID, op, NULL, &uid); } else if (rules->resolve_names >= 0) @@ -1377,15 +1339,13 @@ static void add_rule(struct udev_rules *rules, char *line, gid_t gid; char *endptr; - if (op == OP_REMOVE) { - log_error("invalid GROUP operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); gid = strtoul(value, &endptr, 10); - if (endptr[0] == '\0') { + if (endptr[0] == '\0') rule_add_key(&rule_tmp, TK_A_GROUP_ID, op, NULL, &gid); - } else if ((rules->resolve_names > 0) && strchr("$%", value[0]) == NULL) { + else if ((rules->resolve_names > 0) && strchr("$%", value[0]) == NULL) { gid = add_gid(rules, value); rule_add_key(&rule_tmp, TK_A_GROUP_ID, op, NULL, &gid); } else if (rules->resolve_names >= 0) @@ -1397,10 +1357,8 @@ static void add_rule(struct udev_rules *rules, char *line, mode_t mode; char *endptr; - if (op == OP_REMOVE) { - log_error("invalid MODE operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); mode = strtol(value, &endptr, 8); if (endptr[0] == '\0') @@ -1412,21 +1370,19 @@ static void add_rule(struct udev_rules *rules, char *line, } else if (streq(key, "OPTIONS")) { const char *pos; - if (op == OP_REMOVE) { - log_error("invalid OPTIONS operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); pos = strstr(value, "link_priority="); if (pos != NULL) { - int prio = atoi(&pos[strlen("link_priority=")]); + int prio = atoi(pos + strlen("link_priority=")); rule_add_key(&rule_tmp, TK_A_DEVLINK_PRIO, op, NULL, &prio); } pos = strstr(value, "string_escape="); if (pos != NULL) { - pos = &pos[strlen("string_escape=")]; + pos += strlen("string_escape="); if (startswith(pos, "none")) rule_add_key(&rule_tmp, TK_A_STRING_ESCAPE_NONE, op, NULL, NULL); else if (startswith(pos, "replace")) @@ -1453,28 +1409,19 @@ static void add_rule(struct udev_rules *rules, char *line, pos = strstr(value, "static_node="); if (pos != NULL) { - rule_add_key(&rule_tmp, TK_A_STATIC_NODE, op, &pos[strlen("static_node=")], NULL); + pos += strlen("static_node="); + rule_add_key(&rule_tmp, TK_A_STATIC_NODE, op, pos, NULL); rule_tmp.rule.rule.has_static_node = true; } - } else { - log_error("unknown key '%s' in %s:%u", key, filename, lineno); - goto invalid; - } + } else + LOG_AND_RETURN("unknown key '%s'", key); } - /* add rule token */ + /* add rule token and sort tokens */ rule_tmp.rule.rule.token_count = 1 + rule_tmp.token_cur; - if (add_token(rules, &rule_tmp.rule) != 0) - goto invalid; - - /* add tokens to list, sorted by type */ - if (sort_token(rules, &rule_tmp) != 0) - goto invalid; - - return; -invalid: - log_error("invalid rule '%s:%u'", filename, lineno); + if (add_token(rules, &rule_tmp.rule) != 0 || sort_token(rules, &rule_tmp) != 0) + LOG_RULE_ERROR("failed to add rule token"); } static int parse_file(struct udev_rules *rules, const char *filename) { -- cgit v1.2.3-54-g00ecf From fdd21be6f5f0360c4627bcf5a7957e69ad08be74 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 21 Feb 2016 10:04:36 -0500 Subject: udev-rules: use _cleanup_ for fclose --- src/udev/udev-rules.c | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 5c39c9b874..8470456d4c 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -33,6 +33,7 @@ #include "conf-files.h" #include "escape.h" #include "fd-util.h" +#include "fs-util.h" #include "glob-util.h" #include "path-util.h" #include "stat-util.h" @@ -2039,7 +2040,7 @@ void udev_rules_apply_to_event(struct udev_rules *rules, break; } case TK_M_IMPORT_CMDLINE: { - FILE *f; + _cleanup_fclose_ FILE *f = NULL; bool imported = false; f = fopen("/proc/cmdline", "re"); @@ -2052,12 +2053,12 @@ void udev_rules_apply_to_event(struct udev_rules *rules, pos = strstr(cmdline, key); if (pos != NULL) { + imported = true; pos += strlen(key); - if (pos[0] == '\0' || isspace(pos[0])) { + if (pos[0] == '\0' || isspace(pos[0])) /* we import simple flags as 'FLAG=1' */ udev_device_add_property(event->dev, key, "1"); - imported = true; - } else if (pos[0] == '=') { + else if (pos[0] == '=') { const char *value; pos++; @@ -2066,11 +2067,9 @@ void udev_rules_apply_to_event(struct udev_rules *rules, pos++; pos[0] = '\0'; udev_device_add_property(event->dev, key, value); - imported = true; } } } - fclose(f); } if (!imported && cur->key.op != OP_NOMATCH) goto nomatch; @@ -2366,7 +2365,7 @@ void udev_rules_apply_to_event(struct udev_rules *rules, const char *key_name = rules_str(rules, cur->key.attr_off); char attr[UTIL_PATH_SIZE]; char value[UTIL_NAME_SIZE]; - FILE *f; + _cleanup_fclose_ FILE *f = NULL; if (util_resolve_subsys_kernel(event->udev, key_name, attr, sizeof(attr), 0) != 0) strscpyl(attr, sizeof(attr), udev_device_get_syspath(event->dev), "/", key_name, NULL); @@ -2377,13 +2376,10 @@ void udev_rules_apply_to_event(struct udev_rules *rules, rules_str(rules, rule->rule.filename_off), rule->rule.filename_line); f = fopen(attr, "we"); - if (f != NULL) { - if (fprintf(f, "%s", value) <= 0) - log_error_errno(errno, "error writing ATTR{%s}: %m", attr); - fclose(f); - } else { + if (f == NULL) log_error_errno(errno, "error opening ATTR{%s} for writing: %m", attr); - } + else if (fprintf(f, "%s", value) <= 0) + log_error_errno(errno, "error writing ATTR{%s}: %m", attr); break; } case TK_A_SYSCTL: { @@ -2449,7 +2445,7 @@ int udev_rules_apply_static_dev_perms(struct udev_rules *rules) { char **t; FILE *f = NULL; _cleanup_free_ char *path = NULL; - int r = 0; + int r; if (rules->tokens == NULL) return 0; @@ -2520,8 +2516,6 @@ int udev_rules_apply_static_dev_perms(struct udev_rules *rules) { if (r < 0 && errno != EEXIST) return log_error_errno(errno, "failed to create symlink %s -> %s: %m", tag_symlink, device_node); - else - r = 0; } } @@ -2573,12 +2567,11 @@ finish: fflush(f); fchmod(fileno(f), 0644); if (ferror(f) || rename(path, "/run/udev/static_node-tags") < 0) { - r = -errno; - unlink("/run/udev/static_node-tags"); - unlink(path); + unlink_noerrno("/run/udev/static_node-tags"); + unlink_noerrno(path); + return -errno; } - fclose(f); } - return r; + return 0; } -- cgit v1.2.3-54-g00ecf