From 521ff2fcd057aed893932e09a78eea8088871518 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 28 May 2019 14:35:20 +0200 Subject: [PATCH 1/8] test-user-util: add simple test for make_salt() --- src/test/test-user-util.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/test/test-user-util.c b/src/test/test-user-util.c index 1a507bc00a..fd7878fde2 100644 --- a/src/test/test-user-util.c +++ b/src/test/test-user-util.c @@ -191,6 +191,20 @@ static void test_get_group_creds_one(const char *id, const char *name, gid_t gid assert_se(rgid == gid); } +static void test_make_salt(void) { + log_info("/* %s */", __func__); + + _cleanup_free_ char *s, *t; + + assert_se(make_salt(&s) == 0); + log_info("got %s", s); + + assert_se(make_salt(&t) == 0); + log_info("got %s", t); + + assert(!streq(s, t)); +} + int main(int argc, char *argv[]) { test_uid_to_name_one(0, "root"); test_uid_to_name_one(UID_NOBODY, NOBODY_USER_NAME); @@ -221,5 +235,7 @@ int main(int argc, char *argv[]) { test_valid_gecos(); test_valid_home(); + test_make_salt(); + return 0; } From c2babfc2f5240019ca09a9c2ef026755b997adb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 28 May 2019 16:11:55 +0200 Subject: [PATCH 2/8] networkd: rewrite condition to make it easier to understand --- src/network/networkd-link.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 9486329b9a..6613992a09 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -965,7 +965,8 @@ void link_check_ready(Link *link) { return; if ((link_dhcp4_enabled(link) || link_dhcp6_enabled(link)) && - !(link->dhcp4_configured || link->dhcp6_configured) && + !link->dhcp4_configured && + !link->dhcp6_configured && !(link_ipv4ll_enabled(link, ADDRESS_FAMILY_FALLBACK_IPV4) && link->ipv4ll_address && link->ipv4ll_route)) /* When DHCP is enabled, at least one protocol must provide an address, or * an IPv4ll fallback address must be configured. */ From 2a04712c19556517b47215d49b3faea14490a4f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 May 2019 18:35:22 +0200 Subject: [PATCH 3/8] shared/varlink: add missing terminator in json strings Should finally fix oss-fuzz-14688. 8688c29b5aece49805a244676cba5bba0196f509 wasn't enough. The buffer retrieved from memstream has the size that the same as the written data. When we write do write(f, s, strlen(s)), then no terminating NUL is written, and the buffer is not (necessarilly) a proper C string. --- src/shared/json.c | 10 ++++++++-- src/shared/varlink.c | 1 + src/test/test-json.c | 4 ++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/shared/json.c b/src/shared/json.c index 782d80af16..baea31fed9 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -1555,6 +1555,9 @@ int json_variant_format(JsonVariant *v, JsonFormatFlags flags, char **ret) { size_t sz = 0; int r; + /* Returns the length of the generated string (without the terminating NUL), + * or negative on error. */ + assert_return(v, -EINVAL); assert_return(ret, -EINVAL); @@ -1567,6 +1570,9 @@ int json_variant_format(JsonVariant *v, JsonFormatFlags flags, char **ret) { json_variant_dump(v, flags, f, NULL); + /* Add terminating 0, so that the output buffer is a valid string. */ + fputc('\0', f); + r = fflush_and_check(f); } if (r < 0) @@ -1574,8 +1580,8 @@ int json_variant_format(JsonVariant *v, JsonFormatFlags flags, char **ret) { assert(s); *ret = TAKE_PTR(s); - - return (int) sz; + assert(sz > 0); + return (int) sz - 1; } void json_variant_dump(JsonVariant *v, JsonFormatFlags flags, FILE *f, const char *prefix) { diff --git a/src/shared/varlink.c b/src/shared/varlink.c index 3256a93490..45442848bd 100644 --- a/src/shared/varlink.c +++ b/src/shared/varlink.c @@ -1212,6 +1212,7 @@ static int varlink_enqueue_json(Varlink *v, JsonVariant *m) { r = json_variant_format(m, 0, &text); if (r < 0) return r; + assert(text[r] == '\0'); if (v->output_buffer_size + r + 1 > VARLINK_BUFFER_MAX) return -ENOBUFS; diff --git a/src/test/test-json.c b/src/test/test-json.c index 8bd6a7221c..a6613043b9 100644 --- a/src/test/test-json.c +++ b/src/test/test-json.c @@ -89,6 +89,7 @@ static void test_variant(const char *data, Test test) { r = json_variant_format(v, 0, &s); assert_se(r >= 0); assert_se(s); + assert_se((size_t) r == strlen(s)); log_info("formatted normally: %s\n", s); @@ -105,6 +106,7 @@ static void test_variant(const char *data, Test test) { r = json_variant_format(v, JSON_FORMAT_PRETTY, &s); assert_se(r >= 0); assert_se(s); + assert_se((size_t) r == strlen(s)); log_info("formatted prettily:\n%s", s); @@ -120,12 +122,14 @@ static void test_variant(const char *data, Test test) { r = json_variant_format(v, JSON_FORMAT_COLOR, &s); assert_se(r >= 0); assert_se(s); + assert_se((size_t) r == strlen(s)); printf("Normal with color: %s\n", s); s = mfree(s); r = json_variant_format(v, JSON_FORMAT_COLOR|JSON_FORMAT_PRETTY, &s); assert_se(r >= 0); assert_se(s); + assert_se((size_t) r == strlen(s)); printf("Pretty with color:\n%s\n", s); if (test) From a48481dc7748a33632acbd5f0bffc073cda74f4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 May 2019 19:39:05 +0200 Subject: [PATCH 4/8] Revert "varlink: initialize Varlink with 0" This reverts commit 8688c29b5aece49805a244676cba5bba0196f509, but leaves the reproducer. Structured assignment should be enough to fully initialize the variable and new0 is not necessary. --- src/shared/varlink.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/shared/varlink.c b/src/shared/varlink.c index 45442848bd..48e18b0987 100644 --- a/src/shared/varlink.c +++ b/src/shared/varlink.c @@ -246,8 +246,7 @@ static int varlink_new(Varlink **ret) { assert(ret); - /* Here use new0 as the below structured initializer is nested. */ - v = new0(Varlink, 1); + v = new(Varlink, 1); if (!v) return -ENOMEM; From e7b93f97ed65878772132bf16f33720cdddfb51d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 May 2019 20:36:54 +0200 Subject: [PATCH 5/8] shared/varlink: add missing va_end() Coverity CID#1401347. --- src/shared/varlink.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/shared/varlink.c b/src/shared/varlink.c index 48e18b0987..dea8e4363f 100644 --- a/src/shared/varlink.c +++ b/src/shared/varlink.c @@ -2304,7 +2304,7 @@ int varlink_server_bind_method(VarlinkServer *s, const char *method, VarlinkMeth int varlink_server_bind_method_many_internal(VarlinkServer *s, ...) { va_list ap; - int r; + int r = 0; assert_return(s, -EINVAL); @@ -2321,10 +2321,11 @@ int varlink_server_bind_method_many_internal(VarlinkServer *s, ...) { r = varlink_server_bind_method(s, method, callback); if (r < 0) - return r; + break; } + va_end(ap); - return 0; + return r; } int varlink_server_bind_connect(VarlinkServer *s, VarlinkConnect callback) { From 3135369cffd78e31b795697ba3167e282f429a19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 May 2019 22:49:00 +0200 Subject: [PATCH 6/8] basic/memory-util: do not "return" anything from memzero() macro The macro is not used in expressions, so we don't need the ternary statement. --- src/basic/memory-util.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/basic/memory-util.h b/src/basic/memory-util.h index 915c24a5dd..0e8957b783 100644 --- a/src/basic/memory-util.h +++ b/src/basic/memory-util.h @@ -37,8 +37,8 @@ static inline int memcmp_nn(const void *s1, size_t n1, const void *s2, size_t n2 #define memzero(x,l) \ ({ \ size_t _l_ = (l); \ - void *_x_ = (x); \ - _l_ == 0 ? _x_ : memset(_x_, 0, _l_); \ + if (_l_ > 0) \ + memset(x, 0, _l_); \ }) #define zero(x) (memzero(&(x), sizeof(x))) From 5043ec64766de6d97b692e94874a4658ff46df00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 May 2019 22:56:03 +0200 Subject: [PATCH 7/8] networkd: shorter two error statements a bit Also let's not use a cast if we don't have to. --- src/network/netdev/macsec.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/network/netdev/macsec.c b/src/network/netdev/macsec.c index 740d885339..a4967e817b 100644 --- a/src/network/netdev/macsec.c +++ b/src/network/netdev/macsec.c @@ -852,15 +852,13 @@ int config_parse_macsec_key_id( r = unhexmem(rvalue, strlen(rvalue), &p, &l); if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse key id. Ignoring assignment: %s", rvalue); - return 0; - } - if (l > MACSEC_KEYID_LEN) { - log_syntax(unit, LOG_ERR, filename, line, 0, - "The size of key id is too large (%zu), maximum of %zu permitted. " - "Ignoring assignment: %s", l, (size_t) MACSEC_KEYID_LEN, rvalue); + log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse KeyId \"%s\": %m", rvalue); return 0; } + if (l > MACSEC_KEYID_LEN) + return log_syntax(unit, LOG_ERR, filename, line, 0, + "Specified KeyId is larger then the allowed maximum (%zu > %u), ignoring: %s", + l, MACSEC_KEYID_LEN, rvalue); dest = a ? a->sa.key_id : b->sa.key_id; memcpy_safe(dest, p, l); From 98d4a4dfb3534e2f9c1dbfacd94057b311ba6870 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 May 2019 23:05:31 +0200 Subject: [PATCH 8/8] sd-journal: voidify call To appease coverity, CID#1400674. --- src/journal/sd-journal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c index 82e7d59fbb..40baf2b374 100644 --- a/src/journal/sd-journal.c +++ b/src/journal/sd-journal.c @@ -1734,7 +1734,7 @@ static void remove_directory(sd_journal *j, Directory *d) { hashmap_remove(j->directories_by_wd, INT_TO_PTR(d->wd)); if (j->inotify_fd >= 0) - inotify_rm_watch(j->inotify_fd, d->wd); + (void) inotify_rm_watch(j->inotify_fd, d->wd); } hashmap_remove(j->directories_by_path, d->path);