From 483ed8a6a00a00fbd7ef76e304ee265131207ce8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 25 Mar 2018 22:11:44 +0200 Subject: [PATCH 1/7] fuzz-unit-file: adjust check for ListenNetlink yet again The test for ListenNetlink would abort the loop if a line longer then LINE_MAX was encountered (read_line() returns -ENOBUFS in that case). Let's use the the line length limit that the unit file parses uses. https://oss-fuzz.com/v2/issue/5546208027213824/7094. --- src/fuzz/fuzz-unit-file.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/fuzz/fuzz-unit-file.c b/src/fuzz/fuzz-unit-file.c index 4865653f5b..a5cd9b4aa9 100644 --- a/src/fuzz/fuzz-unit-file.c +++ b/src/fuzz/fuzz-unit-file.c @@ -43,15 +43,17 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { _cleanup_free_ char *l = NULL; const char *ll; - if (read_line(f, LINE_MAX, &l) <= 0) + if (read_line(f, LONG_LINE_MAX, &l) <= 0) break; ll = l + strspn(l, WHITESPACE); - if (startswith(ll, "ListenNetlink=")) + if (startswith(ll, "ListenNetlink=")) { /* ListenNetlink causes a false positive in msan, * let's skip this for now. */ + log_notice("Skipping test because ListenNetlink= is present"); return 0; + } } assert_se(fseek(f, offset, SEEK_SET) == 0); From 989290dbf192b24609fa36379241f2f1edf19d8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 25 Mar 2018 22:26:13 +0200 Subject: [PATCH 2/7] fuzz-unit-file: add __has_feature(memory_sanitizer) when skipping ListenNetlink= https://clang.llvm.org/docs/MemorySanitizer.html#id5 documents this check as the way to detect MemorySanitizer at compilation time. We only need to skip the test if MemorySanitizer is used. Also, use this condition in cg_slice_to_path(). There, the code that is conditionalized is not harmful in any way (it's just unnecessary), so remove the FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION condition. Fixes #8482. --- src/basic/cgroup-util.c | 2 +- src/basic/macro.h | 11 +++++++++++ src/fuzz/fuzz-unit-file.c | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 5934ee6e8a..67e690d6f5 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -1976,7 +1976,7 @@ int cg_slice_to_path(const char *unit, char **ret) { _cleanup_free_ char *escaped = NULL; char n[dash - p + sizeof(".slice")]; -#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION +#if HAS_FEATURE_MEMORY_SANITIZER /* msan doesn't instrument stpncpy, so it thinks * n is later used unitialized: * https://github.com/google/sanitizers/issues/926 diff --git a/src/basic/macro.h b/src/basic/macro.h index 8911edfc4b..8e61ff0685 100644 --- a/src/basic/macro.h +++ b/src/basic/macro.h @@ -63,6 +63,17 @@ #endif #endif +#if !defined(HAS_FEATURE_MEMORY_SANITIZER) +# if defined(__has_feature) +# if __has_feature(memory_sanitizer) +# define HAS_FEATURE_MEMORY_SANITIZER 1 +# endif +# endif +# if !defined(HAS_FEATURE_MEMORY_SANITIZER) +# define HAS_FEATURE_MEMORY_SANITIZER 0 +# endif +#endif + /* Temporarily disable some warnings */ #define DISABLE_WARNING_DECLARATION_AFTER_STATEMENT \ _Pragma("GCC diagnostic push"); \ diff --git a/src/fuzz/fuzz-unit-file.c b/src/fuzz/fuzz-unit-file.c index a5cd9b4aa9..17da776ae3 100644 --- a/src/fuzz/fuzz-unit-file.c +++ b/src/fuzz/fuzz-unit-file.c @@ -48,7 +48,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { ll = l + strspn(l, WHITESPACE); - if (startswith(ll, "ListenNetlink=")) { + if (HAS_FEATURE_MEMORY_SANITIZER && startswith(ll, "ListenNetlink=")) { /* ListenNetlink causes a false positive in msan, * let's skip this for now. */ log_notice("Skipping test because ListenNetlink= is present"); From d6c5d19b66d61d9ca8293b31aaefe310ae691d30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 25 Mar 2018 22:43:43 +0200 Subject: [PATCH 3/7] tree-wide: add bsearch_safe and use where appropriate Should fix #8557. --- src/basic/strbuf.c | 12 ++++-------- src/basic/util.h | 13 +++++++++++++ src/hwdb/hwdb.c | 2 +- src/udev/udevadm-hwdb.c | 4 +++- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/basic/strbuf.c b/src/basic/strbuf.c index bc3e56cf71..b1b1af9cd4 100644 --- a/src/basic/strbuf.c +++ b/src/basic/strbuf.c @@ -24,6 +24,7 @@ #include "alloc-util.h" #include "strbuf.h" +#include "util.h" /* * Strbuf stores given strings in a single continuous allocated memory @@ -144,7 +145,6 @@ ssize_t strbuf_add_string(struct strbuf *str, const char *s, size_t len) { str->in_len += len; node = str->root; - c = s[len-1]; for (depth = 0; depth <= len; depth++) { struct strbuf_child_entry search; @@ -158,15 +158,11 @@ ssize_t strbuf_add_string(struct strbuf *str, const char *s, size_t len) { c = s[len - 1 - depth]; - /* bsearch is not allowed on a NULL sequence */ - if (node->children_count == 0) - break; - /* lookup child node */ search.c = c; - child = bsearch(&search, node->children, node->children_count, - sizeof(struct strbuf_child_entry), - (__compar_fn_t) strbuf_children_cmp); + child = bsearch_safe(&search, node->children, node->children_count, + sizeof(struct strbuf_child_entry), + (__compar_fn_t) strbuf_children_cmp); if (!child) break; node = child->child; diff --git a/src/basic/util.h b/src/basic/util.h index 6f8d8bef34..19e9eae126 100644 --- a/src/basic/util.h +++ b/src/basic/util.h @@ -91,6 +91,19 @@ void *xbsearch_r(const void *key, const void *base, size_t nmemb, size_t size, int (*compar) (const void *, const void *, void *), void *arg); +/** + * Normal bsearch requires base to be nonnull. Here were require + * that only if nmemb > 0. + */ +static inline void* bsearch_safe(const void *key, const void *base, + size_t nmemb, size_t size, comparison_fn_t compar) { + if (nmemb <= 0) + return NULL; + + assert(base); + return bsearch(key, base, nmemb, size, compar); +} + /** * Normal qsort requires base to be nonnull. Here were require * that only if nmemb > 0. diff --git a/src/hwdb/hwdb.c b/src/hwdb/hwdb.c index f579ef737e..6b450b4bb1 100644 --- a/src/hwdb/hwdb.c +++ b/src/hwdb/hwdb.c @@ -123,7 +123,7 @@ static struct trie_node *node_lookup(const struct trie_node *node, uint8_t c) { struct trie_child_entry search; search.c = c; - child = bsearch(&search, node->children, node->children_count, sizeof(struct trie_child_entry), trie_children_cmp); + child = bsearch_safe(&search, node->children, node->children_count, sizeof(struct trie_child_entry), trie_children_cmp); if (child) return child->child; return NULL; diff --git a/src/udev/udevadm-hwdb.c b/src/udev/udevadm-hwdb.c index dc3ae7484d..caa0a92335 100644 --- a/src/udev/udevadm-hwdb.c +++ b/src/udev/udevadm-hwdb.c @@ -114,7 +114,9 @@ static struct trie_node *node_lookup(const struct trie_node *node, uint8_t c) { struct trie_child_entry search; search.c = c; - child = bsearch(&search, node->children, node->children_count, sizeof(struct trie_child_entry), trie_children_cmp); + child = bsearch_safe(&search, + node->children, node->children_count, sizeof(struct trie_child_entry), + trie_children_cmp); if (child) return child->child; return NULL; From 2e38df53494178022aeee885e339273fecf80e40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 25 Mar 2018 22:44:05 +0200 Subject: [PATCH 4/7] journal: use structured initalizer --- src/journal/catalog.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/journal/catalog.c b/src/journal/catalog.c index 6775535b17..03d386cc64 100644 --- a/src/journal/catalog.c +++ b/src/journal/catalog.c @@ -592,13 +592,10 @@ static int open_mmap(const char *database, int *_fd, struct stat *_st, void **_p } static const char *find_id(void *p, sd_id128_t id) { - CatalogItem key, *f = NULL; + CatalogItem *f = NULL, key = { .id = id }; const CatalogHeader *h = p; const char *loc; - zero(key); - key.id = id; - loc = setlocale(LC_MESSAGES, NULL); if (loc && loc[0] && !streq(loc, "C") && !streq(loc, "POSIX")) { strncpy(key.language, loc, sizeof(key.language)); From 2fb076ad43772b808813f88dd26b300336b63671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 26 Mar 2018 10:26:39 +0200 Subject: [PATCH 5/7] basic/strbuf: use structured initalizers to shorten code --- src/basic/strbuf.c | 46 +++++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/src/basic/strbuf.c b/src/basic/strbuf.c index b1b1af9cd4..5a416ce286 100644 --- a/src/basic/strbuf.c +++ b/src/basic/strbuf.c @@ -47,33 +47,31 @@ struct strbuf *strbuf_new(void) { struct strbuf *str; - str = new0(struct strbuf, 1); + str = new(struct strbuf, 1); if (!str) return NULL; + *str = (struct strbuf) { + .buf = new0(char, 1), + .root = new0(struct strbuf_node, 1), + .len = 1, + .nodes_count = 1, + }; + if (!str->buf || !str->root) { + free(str->buf); + free(str->root); + return mfree(str); + } - str->buf = new0(char, 1); - if (!str->buf) - goto err; - str->len = 1; - - str->root = new0(struct strbuf_node, 1); - if (!str->root) - goto err; - str->nodes_count = 1; return str; -err: - free(str->buf); - free(str->root); - return mfree(str); } -static void strbuf_node_cleanup(struct strbuf_node *node) { +static struct strbuf_node* strbuf_node_cleanup(struct strbuf_node *node) { size_t i; for (i = 0; i < node->children_count; i++) strbuf_node_cleanup(node->children[i].child); free(node->children); - free(node); + return mfree(node); } /* clean up trie data, leave only the string buffer */ @@ -81,16 +79,12 @@ void strbuf_complete(struct strbuf *str) { if (!str) return; if (str->root) - strbuf_node_cleanup(str->root); - str->root = NULL; + str->root = strbuf_node_cleanup(str->root); } /* clean up everything */ void strbuf_cleanup(struct strbuf *str) { - if (!str) - return; - if (str->root) - strbuf_node_cleanup(str->root); + strbuf_complete(str); free(str->buf); free(str); } @@ -179,11 +173,13 @@ ssize_t strbuf_add_string(struct strbuf *str, const char *s, size_t len) { str->buf[str->len++] = '\0'; /* new node */ - node_child = new0(struct strbuf_node, 1); + node_child = new(struct strbuf_node, 1); if (!node_child) return -ENOMEM; - node_child->value_off = off; - node_child->value_len = len; + *node_child = (struct strbuf_node) { + .value_off = off, + .value_len = len, + }; /* extend array, add new entry, sort for bisection */ child = reallocarray(node->children, node->children_count + 1, sizeof(struct strbuf_child_entry)); From 435ce146ac6d7ea3c9ed5318b2694179db887cee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 26 Mar 2018 10:32:42 +0200 Subject: [PATCH 6/7] basic/strbuf: include empty strings in count Not that it matters much, but it seems cleaner to also count those inputs, even if they do not consume extra storage space. The test is extended to include an empty input and counts in the test are adjusted to include it. --- src/basic/strbuf.c | 7 +++++-- src/test/test-strbuf.c | 10 +++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/basic/strbuf.c b/src/basic/strbuf.c index 5a416ce286..8b8281bb3b 100644 --- a/src/basic/strbuf.c +++ b/src/basic/strbuf.c @@ -133,9 +133,12 @@ ssize_t strbuf_add_string(struct strbuf *str, const char *s, size_t len) { return -EINVAL; /* search string; start from last character to find possibly matching tails */ - if (len == 0) - return 0; + str->in_count++; + if (len == 0) { + str->dedup_count++; + return 0; + } str->in_len += len; node = str->root; diff --git a/src/test/test-strbuf.c b/src/test/test-strbuf.c index 891d7b1d42..e7395ff96c 100644 --- a/src/test/test-strbuf.c +++ b/src/test/test-strbuf.c @@ -33,7 +33,7 @@ static ssize_t add_string(struct strbuf *sb, const char *s) { static void test_strbuf(void) { struct strbuf *sb; _cleanup_strv_free_ char **l; - ssize_t a, b, c, d, e, f, g; + ssize_t a, b, c, d, e, f, g, h; sb = strbuf_new(); @@ -44,6 +44,7 @@ static void test_strbuf(void) { e = add_string(sb, "aldo"); /* duplicate */ f = add_string(sb, "do"); /* duplicate */ g = add_string(sb, "waldorf"); /* not a duplicate: matches from tail */ + h = add_string(sb, ""); /* check the content of the buffer directly */ l = strv_parse_nulstr(sb->buf, sb->len); @@ -53,10 +54,11 @@ static void test_strbuf(void) { assert_se(streq(l[2], "foo")); assert_se(streq(l[3], "bar")); assert_se(streq(l[4], "waldorf")); + assert_se(l[5] == NULL); assert_se(sb->nodes_count == 5); /* root + 4 non-duplicates */ - assert_se(sb->dedup_count == 3); - assert_se(sb->in_count == 7); + assert_se(sb->dedup_count == 4); + assert_se(sb->in_count == 8); assert_se(sb->in_len == 29); /* length of all strings added */ assert_se(sb->dedup_len == 11); /* length of all strings duplicated */ @@ -70,6 +72,7 @@ static void test_strbuf(void) { assert_se(e == 2); assert_se(f == 4); assert_se(g == 15); + assert_se(h == 0); assert_se(streq(sb->buf + a, "waldo")); assert_se(streq(sb->buf + b, "foo")); @@ -78,6 +81,7 @@ static void test_strbuf(void) { assert_se(streq(sb->buf + e, "aldo")); assert_se(streq(sb->buf + f, "do")); assert_se(streq(sb->buf + g, "waldorf")); + assert_se(streq(sb->buf + h, "")); strbuf_complete(sb); assert_se(sb->root == NULL); From 94ac201ac2dd8228038f0451d81aee897aa007f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 26 Mar 2018 10:40:45 +0200 Subject: [PATCH 7/7] README: mention systemd-stable Fixes #8564. https://www.freedesktop.org/wiki/Software/systemd/Backports/ has already been adjusted. --- README | 11 +++++++++++ README.md | 2 ++ 2 files changed, 13 insertions(+) diff --git a/README b/README index 8807e5cfe4..b735214e2e 100644 --- a/README +++ b/README @@ -314,6 +314,17 @@ WARNINGS: nice output only on exit(), hence on shutdown we don't execve() systemd-shutdown. +STABLE BRANCHES AND BACKPORTS + + Stable branches with backported patches are available in the + systemd-stable repo at https://github.com/systemd/systemd-stable. + + Stable branches are started for certain releases of systemd and named + after them, e.g. v238-stable. Stable branches are managed by + distribution maintainers on an as needed basis. See + https://www.freedesktop.org/wiki/Software/systemd/Backports/ for some + more information and examples. + ENGINEERING AND CONSULTING SERVICES: Kinvolk (https://kinvolk.io) offers professional engineering and consulting services for systemd. Please contact Chris Kühl diff --git a/README.md b/README.md index 4b017faf1b..c122d01731 100644 --- a/README.md +++ b/README.md @@ -21,3 +21,5 @@ Please see our [Contribution Guidelines](../master/.github/CONTRIBUTING.md) for When preparing patches for systemd, please follow our [Coding Style Guidelines](../master/doc/CODING_STYLE). If you are looking for support, please contact our [mailing list](https://lists.freedesktop.org/mailman/listinfo/systemd-devel) or join our [IRC channel](irc://irc.freenode.org/%23systemd). + +Stable branches with backported patches are available in the [stable repo](https://github.com/systemd/systemd-stable).