Merge pull request #18892 from poettering/cname-tweaks

resolved: properly handle stub replies for chains of multiple CNAMEs

docs/ENVIRONMENT.md

@@ -301,3 +301,15 @@ installed systemd tests:
 
 * `$SYSTEMD_SYSVRCND_PATH` — Controls where `systemd-sysv-generator` looks for
   SysV init script runlevel link farms.
+
+fuzzers:
+
+* `$SYSTEMD_FUZZ_OUTPUT` — A boolean that specifies whether to write output to
+  stdout. Setting to true is useful in manual invocations, since all output is
+  suppressed by default.
+
+* `$SYSTEMD_FUZZ_RUNS` — The number of times execution should be repeated in
+  manual invocations.
+
+Note that is may be also useful to set `$SYSTEMD_LOG_LEVEL`, since all logging
+is suppressed by default.

man/sd_bus_add_match.xml

@@ -94,13 +94,13 @@
     connection object <parameter>bus</parameter>. The syntax of the match rule expression passed in
     <parameter>match</parameter> is described in the <ulink
     url="https://dbus.freedesktop.org/doc/dbus-specification.html">D-Bus Specification</ulink>. The specified handler
-    function <parameter>callback</parameter> is called for eaching incoming message matching the specified expression,
+    function <parameter>callback</parameter> is called for each incoming message matching the specified expression,
     the <parameter>userdata</parameter> parameter is passed as-is to the callback function. The match is installed
     synchronously when connected to a bus broker, i.e. the call sends a control message requested the match to be added
     to the broker and waits until the broker confirms the match has been installed successfully.</para>
 
     <para><function>sd_bus_add_match_async()</function> operates very similar to
-    <function>sd_bus_match_signal()</function>, however it installs the match asynchronously, in a non-blocking
+    <function>sd_bus_add_match()</function>, however it installs the match asynchronously, in a non-blocking
     fashion: a request is sent to the broker, but the call does not wait for a response. The
     <parameter>install_callback</parameter> function is called when the response is later received, with the response
     message from the broker as parameter. If this function is specified as <constant>NULL</constant> a default

man/sd_bus_request_name.xml

@@ -100,7 +100,7 @@
     is sent to the bus broker, and the call waits until the broker responds.</para>
 
     <para><function>sd_bus_request_name_async()</function> is an asynchronous version of
-    <function>sd_bus_release_name()</function>. Instead of waiting for the request to complete, the request message is
+    <function>sd_bus_request_name()</function>. Instead of waiting for the request to complete, the request message is
     enqueued. The specified <parameter>callback</parameter> will be called when the broker's response is received. If
     the parameter is specified as <constant>NULL</constant> a default implementation is used instead which will
     terminate the connection when the name cannot be acquired. The function returns a slot object in its

src/basic/audit-util.h

@@ -5,7 +5,7 @@
 #include <stdint.h>
 #include <sys/types.h>
 
-#define AUDIT_SESSION_INVALID (UINT32_MAX)
+#define AUDIT_SESSION_INVALID UINT32_MAX
 
 int audit_session_from_pid(pid_t pid, uint32_t *id);
 int audit_loginuid_from_pid(pid_t pid, uid_t *uid);

src/basic/cgroup-util.h

@@ -75,13 +75,13 @@ CGroupMask get_cpu_accounting_mask(void);
 bool cpu_accounting_is_cheap(void);
 
 /* Special values for all weight knobs on unified hierarchy */
-#define CGROUP_WEIGHT_INVALID (UINT64_MAX)
+#define CGROUP_WEIGHT_INVALID UINT64_MAX
 #define CGROUP_WEIGHT_MIN UINT64_C(1)
 #define CGROUP_WEIGHT_MAX UINT64_C(10000)
 #define CGROUP_WEIGHT_DEFAULT UINT64_C(100)
 
 #define CGROUP_LIMIT_MIN UINT64_C(0)
-#define CGROUP_LIMIT_MAX (UINT64_MAX)
+#define CGROUP_LIMIT_MAX UINT64_MAX
 
 static inline bool CGROUP_WEIGHT_IS_OK(uint64_t x) {
         return
@@ -106,7 +106,7 @@ const char* cgroup_io_limit_type_to_string(CGroupIOLimitType t) _const_;
 CGroupIOLimitType cgroup_io_limit_type_from_string(const char *s) _pure_;
 
 /* Special values for the cpu.shares attribute */
-#define CGROUP_CPU_SHARES_INVALID (UINT64_MAX)
+#define CGROUP_CPU_SHARES_INVALID UINT64_MAX
 #define CGROUP_CPU_SHARES_MIN UINT64_C(2)
 #define CGROUP_CPU_SHARES_MAX UINT64_C(262144)
 #define CGROUP_CPU_SHARES_DEFAULT UINT64_C(1024)
@@ -118,7 +118,7 @@ static inline bool CGROUP_CPU_SHARES_IS_OK(uint64_t x) {
 }
 
 /* Special values for the blkio.weight attribute */
-#define CGROUP_BLKIO_WEIGHT_INVALID (UINT64_MAX)
+#define CGROUP_BLKIO_WEIGHT_INVALID UINT64_MAX
 #define CGROUP_BLKIO_WEIGHT_MIN UINT64_C(10)
 #define CGROUP_BLKIO_WEIGHT_MAX UINT64_C(1000)
 #define CGROUP_BLKIO_WEIGHT_DEFAULT UINT64_C(500)

src/basic/string-util.c

@@ -71,7 +71,7 @@ char *strnappend(const char *s, const char *suffix, size_t b) {
         assert(suffix);
 
         a = strlen(s);
-        if (b > (SIZE_MAX) - a)
+        if (b > SIZE_MAX - a)
                 return NULL;
 
         r = new(char, a+b+1);

src/fuzz/fuzz-main.c

@@ -1,9 +1,11 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
 #include "alloc-util.h"
-#include "log.h"
 #include "fileio.h"
 #include "fuzz.h"
+#include "log.h"
+#include "parse-util.h"
+#include "string-util.h"
 #include "tests.h"
 
 /* This is a test driver for the systemd fuzzers that provides main function
@@ -15,13 +17,22 @@
 /* This one was borrowed from
  * https://github.com/google/oss-fuzz/blob/646fca1b506b056db3a60d32c4a1a7398f171c94/infra/base-images/base-runner/bad_build_check#L19
  */
-#define MIN_NUMBER_OF_RUNS 4
+#define NUMBER_OF_RUNS 4
 
 int main(int argc, char **argv) {
         int r;
 
         test_setup_logging(LOG_DEBUG);
 
+        unsigned number_of_runs = NUMBER_OF_RUNS;
+
+        const char *v = getenv("SYSTEMD_FUZZ_RUNS");
+        if (!isempty(v)) {
+                r = safe_atou(v, &number_of_runs);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to parse SYSTEMD_FUZZ_RUNS=%s: %m", v);
+        }
+
         for (int i = 1; i < argc; i++) {
                 _cleanup_free_ char *buf = NULL;
                 size_t size;
@@ -35,7 +46,7 @@ int main(int argc, char **argv) {
                 }
                 printf("%s... ", name);
                 fflush(stdout);
-                for (int j = 0; j < MIN_NUMBER_OF_RUNS; j++)
+                for (unsigned j = 0; j < number_of_runs; j++)
                         if (LLVMFuzzerTestOneInput((uint8_t*)buf, size) == EXIT_TEST_SKIP)
                                 return EXIT_TEST_SKIP;
                 printf("ok\n");

src/fuzz/meson.build

@@ -1,8 +1,6 @@
 # SPDX-License-Identifier: LGPL-2.1-or-later
 
 fuzzers += [
-        [['src/fuzz/fuzz-bus-message.c']],
-
         [['src/fuzz/fuzz-catalog.c']],
 
         [['src/fuzz/fuzz-json.c']],

src/libsystemd/meson.build

@@ -315,3 +315,11 @@ if cxx_cmd != ''
                 [['src/libsystemd/sd-bus/test-bus-vtable-cc.cc']],
         ]
 endif
+
+############################################################
+
+fuzzers += [
+        [['src/libsystemd/sd-bus/fuzz-bus-message.c']],
+
+        [['src/libsystemd/sd-bus/fuzz-bus-match.c']],
+]

src/libsystemd/sd-bus/bus-internal.h

@@ -44,8 +44,8 @@ struct match_callback {
         unsigned last_iteration;
 
         /* Don't dispatch this slot with messages that arrived in any iteration before or at the this
-         * one. We use this to ensure that matches don't apply "retroactively" and thus can confuse the
-         * caller: matches will only match incoming messages from the moment on the match was installed. */
+         * one. We use this to ensure that matches don't apply "retroactively" and confuse the caller:
+         * only messages received after the match was installed will be considered. */
         uint64_t after;
 
         char *match_string;

src/libsystemd/sd-bus/bus-match.c

@@ -408,12 +408,9 @@ int bus_match_run(
                         if (r != 0)
                                 return r;
                 }
-        } else {
-                struct bus_match_node *c;
-
+        } else
                 /* No hash table, so let's iterate manually... */
-
-                for (c = node->child; c; c = c->next) {
+                for (struct bus_match_node *c = node->child; c; c = c->next) {
                         if (!value_node_test(c, node->type, test_u8, test_str, test_strv, m))
                                 continue;
 
@@ -424,7 +421,6 @@ int bus_match_run(
                         if (bus && bus->match_callbacks_modified)
                                 return 0;
                 }
-        }
 
         if (bus && bus->match_callbacks_modified)
                 return 0;
@@ -440,7 +436,7 @@ static int bus_match_add_compare_value(
                 const char *value_str,
                 struct bus_match_node **ret) {
 
-        struct bus_match_node *c = NULL, *n = NULL;
+        struct bus_match_node *c, *n = NULL;
         int r;
 
         assert(where);
@@ -452,25 +448,22 @@ static int bus_match_add_compare_value(
                 ;
 
         if (c) {
-                /* Comparison node already exists? Then let's see if
-                 * the value node exists too. */
+                /* Comparison node already exists? Then let's see if the value node exists too. */
 
                 if (t == BUS_MATCH_MESSAGE_TYPE)
                         n = hashmap_get(c->compare.children, UINT_TO_PTR(value_u8));
                 else if (BUS_MATCH_CAN_HASH(t))
                         n = hashmap_get(c->compare.children, value_str);
-                else {
+                else
                         for (n = c->child; n && !value_node_same(n, t, value_u8, value_str); n = n->next)
                                 ;
-                }
 
                 if (n) {
                         *ret = n;
                         return 0;
                 }
         } else {
-                /* Comparison node, doesn't exist yet? Then let's
-                 * create it. */
+                /* Comparison node, doesn't exist yet? Then let's create it. */
 
                 c = new0(struct bus_match_node, 1);
                 if (!c) {
@@ -706,9 +699,7 @@ static int match_component_compare(const struct bus_match_component *a, const st
 }
 
 void bus_match_parse_free(struct bus_match_component *components, unsigned n_components) {
-        unsigned i;
-
-        for (i = 0; i < n_components; i++)
+        for (unsigned i = 0; i < n_components; i++)
                 free(components[i].value_str);
 
         free(components);
@@ -716,51 +707,54 @@ void bus_match_parse_free(struct bus_match_component *components, unsigned n_com
 
 int bus_match_parse(
                 const char *match,
-                struct bus_match_component **_components,
-                unsigned *_n_components) {
+                struct bus_match_component **ret_components,
+                unsigned *ret_n_components) {
 
-        const char *p = match;
         struct bus_match_component *components = NULL;
         size_t components_allocated = 0;
-        unsigned n_components = 0, i;
-        _cleanup_free_ char *value = NULL;
+        unsigned n_components = 0;
         int r;
 
         assert(match);
-        assert(_components);
-        assert(_n_components);
+        assert(ret_components);
+        assert(ret_n_components);
 
-        while (*p != 0) {
+        while (*match != '\0') {
                 const char *eq, *q;
                 enum bus_match_node_type t;
                 unsigned j = 0;
+                _cleanup_free_ char *value = NULL;
                 size_t value_allocated = 0;
                 bool escaped = false, quoted;
                 uint8_t u;
 
                 /* Avahi's match rules appear to include whitespace, skip over it */
-                p += strspn(p, " ");
+                match += strspn(match, " ");
 
-                eq = strchr(p, '=');
-                if (!eq)
-                        return -EINVAL;
+                eq = strchr(match, '=');
+                if (!eq) {
+                        r = -EINVAL;
+                        goto fail;
+                }
 
-                t = bus_match_node_type_from_string(p, eq - p);
-                if (t < 0)
-                        return -EINVAL;
+                t = bus_match_node_type_from_string(match, eq - match);
+                if (t < 0) {
+                        r = -EINVAL;
+                        goto fail;
+                }
 
                 quoted = eq[1] == '\'';
 
                 for (q = eq + 1 + quoted;; q++) {
 
-                        if (*q == 0) {
+                        if (*q == '\0') {
 
                                 if (quoted) {
                                         r = -EINVAL;
                                         goto fail;
                                 } else {
                                         if (value)
-                                                value[j] = 0;
+                                                value[j] = '\0';
                                         break;
                                 }
                         }
@@ -774,14 +768,13 @@ int bus_match_parse(
                                 if (quoted) {
                                         if (*q == '\'') {
                                                 if (value)
-                                                        value[j] = 0;
+                                                        value[j] = '\0';
                                                 break;
                                         }
                                 } else {
                                         if (*q == ',') {
                                                 if (value)
-                                                        value[j] = 0;
-
+                                                        value[j] = '\0';
                                                 break;
                                         }
                                 }
@@ -818,10 +811,11 @@ int bus_match_parse(
                         goto fail;
                 }
 
-                components[n_components].type = t;
-                components[n_components].value_str = TAKE_PTR(value);
-                components[n_components].value_u8 = u;
-                n_components++;
+                components[n_components++] = (struct bus_match_component) {
+                        .type = t,
+                        .value_str = TAKE_PTR(value),
+                        .value_u8 = u,
+                };
 
                 if (q[quoted] == 0)
                         break;
@@ -831,21 +825,21 @@ int bus_match_parse(
                         goto fail;
                 }
 
-                p = q + 1 + quoted;
+                match = q + 1 + quoted;
         }
 
         /* Order the whole thing, so that we always generate the same tree */
         typesafe_qsort(components, n_components, match_component_compare);
 
         /* Check for duplicates */
-        for (i = 0; i+1 < n_components; i++)
+        for (unsigned i = 0; i+1 < n_components; i++)
                 if (components[i].type == components[i+1].type) {
                         r = -EINVAL;
                         goto fail;
                 }
 
-        *_components = components;
-        *_n_components = n_components;
+        *ret_components = components;
+        *ret_n_components = n_components;
 
         return 0;
 
@@ -855,10 +849,8 @@ fail:
 }
 
 char *bus_match_to_string(struct bus_match_component *components, unsigned n_components) {
-        _cleanup_fclose_ FILE *f = NULL;
-        char *buffer = NULL;
+        _cleanup_free_ char *buffer = NULL;
         size_t size = 0;
-        unsigned i;
         int r;
 
         if (n_components <= 0)
@@ -866,11 +858,11 @@ char *bus_match_to_string(struct bus_match_component *components, unsigned n_com
 
         assert(components);
 
-        f = open_memstream_unlocked(&buffer, &size);
+        FILE *f = open_memstream_unlocked(&buffer, &size);
         if (!f)
                 return NULL;
 
-        for (i = 0; i < n_components; i++) {
+        for (unsigned i = 0; i < n_components; i++) {
                 char buf[32];
 
                 if (i != 0)
@@ -889,10 +881,10 @@ char *bus_match_to_string(struct bus_match_component *components, unsigned n_com
         }
 
         r = fflush_and_check(f);
+        safe_fclose(f);
         if (r < 0)
                 return NULL;
-
-        return buffer;
+        return TAKE_PTR(buffer);
 }
 
 int bus_match_add(
@@ -901,23 +893,22 @@ int bus_match_add(
                 unsigned n_components,
                 struct match_callback *callback) {
 
-        unsigned i;
-        struct bus_match_node *n;
         int r;
 
         assert(root);
         assert(callback);
 
-        n = root;
-        for (i = 0; i < n_components; i++) {
-                r = bus_match_add_compare_value(
-                                n, components[i].type,
-                                components[i].value_u8, components[i].value_str, &n);
+        for (unsigned i = 0; i < n_components; i++) {
+                r = bus_match_add_compare_value(root,
+                                                components[i].type,
+                                                components[i].value_u8,
+                                                components[i].value_str,
+                                                &root);
                 if (r < 0)
                         return r;
         }
 
-        return bus_match_add_leaf(n, callback);
+        return bus_match_add_leaf(root, callback);
 }
 
 int bus_match_remove(
@@ -1028,42 +1019,39 @@ const char* bus_match_node_type_to_string(enum bus_match_node_type t, char buf[]
         }
 }
 
-void bus_match_dump(struct bus_match_node *node, unsigned level) {
-        struct bus_match_node *c;
-        _cleanup_free_ char *pfx = NULL;
+void bus_match_dump(FILE *out, struct bus_match_node *node, unsigned level) {
         char buf[32];
 
         if (!node)
                 return;
 
-        pfx = strrep("  ", level);
-        printf("%s[%s]", strempty(pfx), bus_match_node_type_to_string(node->type, buf, sizeof(buf)));
+        fprintf(out, "%*s[%s]", 2 * level, "", bus_match_node_type_to_string(node->type, buf, sizeof(buf)));
 
         if (node->type == BUS_MATCH_VALUE) {
                 if (node->parent->type == BUS_MATCH_MESSAGE_TYPE)
-                        printf(" <%u>\n", node->value.u8);
+                        fprintf(out, " <%u>\n", node->value.u8);
                 else
-                        printf(" <%s>\n", node->value.str);
+                        fprintf(out, " <%s>\n", node->value.str);
         } else if (node->type == BUS_MATCH_ROOT)
-                puts(" root");
+                fputs(" root\n", out);
         else if (node->type == BUS_MATCH_LEAF)
-                printf(" %p/%p\n", node->leaf.callback->callback, container_of(node->leaf.callback, sd_bus_slot, match_callback)->userdata);
+                fprintf(out, " %p/%p\n", node->leaf.callback->callback,
+                        container_of(node->leaf.callback, sd_bus_slot, match_callback)->userdata);
         else
-                putchar('\n');
+                putc('\n', out);
 
         if (BUS_MATCH_CAN_HASH(node->type)) {
-
+                struct bus_match_node *c;
                 HASHMAP_FOREACH(c, node->compare.children)
-                        bus_match_dump(c, level + 1);
+                        bus_match_dump(out, c, level + 1);
         }
 
-        for (c = node->child; c; c = c->next)
-                bus_match_dump(c, level + 1);
+        for (struct bus_match_node *c = node->child; c; c = c->next)
+                bus_match_dump(out, c, level + 1);
 }
 
 enum bus_match_scope bus_match_get_scope(const struct bus_match_component *components, unsigned n_components) {
         bool found_driver = false;
-        unsigned i;
 
         if (n_components <= 0)
                 return BUS_MATCH_GENERIC;
@@ -1076,7 +1064,7 @@ enum bus_match_scope bus_match_get_scope(const struct bus_match_component *compo
          * local messages, then we check if it only matches on the
          * driver. */
 
-        for (i = 0; i < n_components; i++) {
+        for (unsigned i = 0; i < n_components; i++) {
                 const struct bus_match_component *c = components + i;
 
                 if (c->type == BUS_MATCH_SENDER) {
@@ -1095,5 +1083,4 @@ enum bus_match_scope bus_match_get_scope(const struct bus_match_component *compo
         }
 
         return found_driver ? BUS_MATCH_DRIVER : BUS_MATCH_GENERIC;
-
 }

src/libsystemd/sd-bus/bus-match.h

@@ -1,6 +1,8 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 #pragma once
 
+#include <stdio.h>
+
 #include "sd-bus.h"
 
 #include "hashmap.h"
@@ -68,12 +70,12 @@ int bus_match_remove(struct bus_match_node *root, struct match_callback *callbac
 
 void bus_match_free(struct bus_match_node *node);
 
-void bus_match_dump(struct bus_match_node *node, unsigned level);
+void bus_match_dump(FILE *out, struct bus_match_node *node, unsigned level);
 
 const char* bus_match_node_type_to_string(enum bus_match_node_type t, char buf[], size_t l);
 enum bus_match_node_type bus_match_node_type_from_string(const char *k, size_t n);
 
-int bus_match_parse(const char *match, struct bus_match_component **_components, unsigned *_n_components);
+int bus_match_parse(const char *match, struct bus_match_component **ret_components, unsigned *ret_n_components);
 void bus_match_parse_free(struct bus_match_component *components, unsigned n_components);
 char *bus_match_to_string(struct bus_match_component *components, unsigned n_components);
 

src/libsystemd/sd-bus/bus-message.c

@@ -161,8 +161,7 @@ static void *message_extend_fields(sd_bus_message *m, size_t align, size_t sz, b
         start = ALIGN_TO(old_size, align);
         new_size = start + sz;
 
-        if (new_size < start ||
-            new_size > (size_t) (UINT32_MAX))
+        if (new_size < start || new_size > UINT32_MAX)
                 goto poison;
 
         if (old_size == new_size)
@@ -1337,8 +1336,7 @@ static void *message_extend_body(
         added = padding + sz;
 
         /* Check for 32bit overflows */
-        if (end_body > (size_t) (UINT32_MAX) ||
-            end_body < start_body) {
+        if (end_body < start_body || end_body > UINT32_MAX) {
                 m->poisoned = true;
                 return NULL;
         }

src/libsystemd/sd-bus/fuzz-bus-match.c

@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include "alloc-util.h"
+#include "bus-internal.h"
+#include "bus-match.h"
+#include "env-util.h"
+#include "fd-util.h"
+#include "fileio.h"
+#include "fuzz.h"
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
+        _cleanup_free_ char *out = NULL; /* out should be freed after g */
+        size_t out_size;
+        _cleanup_fclose_ FILE *g = NULL;
+        _cleanup_(sd_bus_unrefp) sd_bus *bus = NULL;
+        int r;
+
+        /* We don't want to fill the logs with messages about parse errors.
+         * Disable most logging if not running standalone */
+        if (!getenv("SYSTEMD_LOG_LEVEL"))
+                log_set_max_level(LOG_CRIT);
+
+        r = sd_bus_new(&bus);
+        assert_se(r >= 0);
+
+        struct bus_match_node root = {
+                .type = BUS_MATCH_ROOT,
+        };
+
+        /* Note that we use the pointer to match_callback substructure, but the code
+         * uses container_of() to access outside of the passed-in type. */
+        sd_bus_slot slot = {
+                .type = BUS_MATCH_CALLBACK,
+                .match_callback = {},
+        };
+
+        if (getenv_bool("SYSTEMD_FUZZ_OUTPUT") <= 0)
+                assert_se(g = open_memstream_unlocked(&out, &out_size));
+
+        for (size_t offset = 0; offset < size; ) {
+                _cleanup_free_ char *line = NULL;
+                char *end;
+
+                end = memchr((char*) data + offset, '\n', size - offset);
+
+                line = memdup_suffix0((char*) data + offset,
+                                      end ? end - (char*) data - offset : size - offset);
+                if (!line)
+                        return log_oom_debug();
+
+                offset = end ? (size_t) (end - (char*) data + 1) : size;
+
+                struct bus_match_component *components;
+                unsigned n_components;
+                r = bus_match_parse(line, &components, &n_components);
+                if (IN_SET(r, -EINVAL, -ENOMEM)) {
+                        log_debug_errno(r, "Failed to parse line: %m");
+                        continue;
+                }
+                assert_se(r >= 0); /* We only expect EINVAL and ENOMEM errors, or success. */
+
+                log_debug("Parsed %u components.", n_components);
+
+                _cleanup_free_ char *again = bus_match_to_string(components, n_components);
+                if (!again) {
+                        bus_match_parse_free(components, n_components);
+                        log_oom();
+                        break;
+                }
+
+                if (g)
+                        fprintf(g, "%s\n", again);
+
+                r = bus_match_add(&root, components, n_components, &slot.match_callback);
+                bus_match_parse_free(components, n_components);
+                if (r < 0) {
+                        log_error_errno(r, "Failed to add match: %m");
+                        break;
+                }
+        }
+
+        bus_match_dump(g ?: stdout, &root, 0); /* We do this even on failure, to check consistency after error. */
+        bus_match_free(&root);
+
+        return 0;
+}

src/libsystemd/sd-bus/test-bus-chat.c

@@ -101,7 +101,7 @@ static int server_init(sd_bus **_bus) {
                 goto fail;
         }
 
-        bus_match_dump(&bus->match_callbacks, 0);
+        bus_match_dump(stdout, &bus->match_callbacks, 0);
 
         *_bus = bus;
         return 0;

src/libsystemd/sd-bus/test-bus-match.c

@@ -37,13 +37,12 @@ static bool mask_contains(unsigned a[], unsigned n) {
 }
 
 static int match_add(sd_bus_slot *slots, struct bus_match_node *root, const char *match, int value) {
-        struct bus_match_component *components = NULL;
-        unsigned n_components = 0;
+        struct bus_match_component *components;
+        unsigned n_components;
         sd_bus_slot *s;
         int r;
 
         s = slots + value;
-        zero(*s);
 
         r = bus_match_parse(match, &components, &n_components);
         if (r < 0)
@@ -74,8 +73,7 @@ int main(int argc, char *argv[]) {
 
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL;
         _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
-        enum bus_match_node_type i;
-        sd_bus_slot slots[19];
+        sd_bus_slot slots[19] = {};
         int r;
 
         test_setup_logging(LOG_INFO);
@@ -105,7 +103,7 @@ int main(int argc, char *argv[]) {
         assert_se(match_add(slots, &root, "arg4has='po'", 17) >= 0);
         assert_se(match_add(slots, &root, "arg4='pi'", 18) >= 0);
 
-        bus_match_dump(&root, 0);
+        bus_match_dump(stdout, &root, 0);
 
         assert_se(sd_bus_message_new_signal(bus, &m, "/foo/bar", "bar.x", "waldo") >= 0);
         assert_se(sd_bus_message_append(m, "ssssas", "one", "two", "/prefix/three", "prefix.four", 3, "pi", "pa", "po") >= 0);
@@ -118,13 +116,13 @@ int main(int argc, char *argv[]) {
         assert_se(bus_match_remove(&root, &slots[8].match_callback) >= 0);
         assert_se(bus_match_remove(&root, &slots[13].match_callback) >= 0);
 
-        bus_match_dump(&root, 0);
+        bus_match_dump(stdout, &root, 0);
 
         zero(mask);
         assert_se(bus_match_run(NULL, &root, m) == 0);
         assert_se(mask_contains((unsigned[]) { 9, 5, 10, 12, 14, 7, 15, 16, 17 }, 9));
 
-        for (i = 0; i < _BUS_MATCH_NODE_TYPE_MAX; i++) {
+        for (enum bus_match_node_type i = 0; i < _BUS_MATCH_NODE_TYPE_MAX; i++) {
                 char buf[32];
                 const char *x;
 

src/network/wait-online/manager.c

@@ -323,12 +323,8 @@ int manager_new(Manager **ret, Hashmap *interfaces, char **ignore,
         (void) sd_event_add_signal(m->event, NULL, SIGINT, NULL, NULL);
 
         if (timeout > 0) {
-                usec_t usec;
-
-                usec = usec_add(now(clock_boottime_or_monotonic()), timeout);
-
-                r = sd_event_add_time(m->event, NULL, clock_boottime_or_monotonic(), usec, 0, NULL, INT_TO_PTR(-ETIMEDOUT));
-                if (r < 0)
+                r = sd_event_add_time_relative(m->event, NULL, clock_boottime_or_monotonic(), timeout, 0, NULL, INT_TO_PTR(-ETIMEDOUT));
+                if (r < 0 && r != -EOVERFLOW)
                         return r;
         }
 

src/resolve/resolved-dns-answer.c

@@ -640,6 +640,31 @@ int dns_answer_remove_by_rr(DnsAnswer **a, DnsResourceRecord *rm) {
         return 1;
 }
 
+int dns_answer_remove_by_answer_keys(DnsAnswer **a, DnsAnswer *b) {
+        _cleanup_(dns_resource_key_unrefp) DnsResourceKey *prev = NULL;
+        DnsAnswerItem *item;
+        int r;
+
+        /* Removes all items from '*a' that have a matching key in 'b' */
+
+        DNS_ANSWER_FOREACH_ITEM(item, b) {
+
+                if (prev && dns_resource_key_equal(item->rr->key, prev)) /* Skip this one, we already looked at it */
+                        continue;
+
+                r = dns_answer_remove_by_key(a, item->rr->key);
+                if (r < 0)
+                        return r;
+
+                /* Let's remember this entry's RR key, to optimize the loop a bit: if we have an RRset with
+                 * more than one item then we don't need to remove the key multiple times */
+                dns_resource_key_unref(prev);
+                prev = dns_resource_key_ref(item->rr->key);
+        }
+
+        return 0;
+}
+
 int dns_answer_copy_by_key(
                 DnsAnswer **a,
                 DnsAnswer *source,

src/resolve/resolved-dns-answer.h

@@ -68,6 +68,7 @@ int dns_answer_reserve_or_clone(DnsAnswer **a, size_t n_free);
 
 int dns_answer_remove_by_key(DnsAnswer **a, const DnsResourceKey *key);
 int dns_answer_remove_by_rr(DnsAnswer **a, DnsResourceRecord *rr);
+int dns_answer_remove_by_answer_keys(DnsAnswer **a, DnsAnswer *b);
 
 int dns_answer_copy_by_key(DnsAnswer **a, DnsAnswer *source, const DnsResourceKey *key, DnsAnswerFlags or_flags, DnsResourceRecord *rrsig);
 int dns_answer_move_by_key(DnsAnswer **to, DnsAnswer **from, const DnsResourceKey *key, DnsAnswerFlags or_flags, DnsResourceRecord *rrsig);

src/resolve/resolved-dns-query.c

@@ -10,7 +10,6 @@
 #include "resolved-etc-hosts.h"
 #include "string-util.h"
 
-#define CNAME_MAX 8
 #define QUERIES_MAX 2048
 #define AUXILIARY_QUERIES_MAX 64
 
@@ -977,7 +976,7 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname)
         assert(q);
 
         q->n_cname_redirects++;
-        if (q->n_cname_redirects > CNAME_MAX)
+        if (q->n_cname_redirects > CNAME_REDIRECT_MAX)
                 return -ELOOP;
 
         r = dns_question_cname_redirect(q->question_idna, cname, &nq_idna);

src/resolve/resolved-dns-query.h

@@ -145,3 +145,5 @@ static inline uint64_t dns_query_reply_flags_make(DnsQuery *q) {
                                       dns_query_fully_confidential(q)) |
                 (q->answer_query_flags & (SD_RESOLVED_FROM_MASK|SD_RESOLVED_SYNTHETIC));
 }
+
+#define CNAME_REDIRECT_MAX 16

src/resolve/resolved-dns-rr.c

@@ -244,6 +244,9 @@ int dns_resource_key_match_cname_or_dname(const DnsResourceKey *key, const DnsRe
         if (cname->class != key->class && key->class != DNS_CLASS_ANY)
                 return 0;
 
+        if (!dns_type_may_redirect(key->type))
+                return 0;
+
         if (cname->type == DNS_TYPE_CNAME)
                 r = dns_name_equal(dns_resource_key_name(key), dns_resource_key_name(cname));
         else if (cname->type == DNS_TYPE_DNAME)
@@ -1743,9 +1746,16 @@ int dns_resource_record_get_cname_target(DnsResourceKey *key, DnsResourceRecord
         assert(key);
         assert(cname);
 
+        /* Checks if the RR `cname` is a CNAME/DNAME RR that matches the specified `key`. If so, returns the
+         * target domain. If not, returns -EUNATCH */
+
         if (key->class != cname->key->class && key->class != DNS_CLASS_ANY)
                 return -EUNATCH;
 
+        if (!dns_type_may_redirect(key->type)) /* This key type is not subject to CNAME/DNAME redirection?
+                                                * Then let's refuse right-away */
+                return -EUNATCH;
+
         if (cname->key->type == DNS_TYPE_CNAME) {
                 r = dns_name_equal(dns_resource_key_name(key),
                                    dns_resource_key_name(cname->key));

src/resolve/resolved-dns-stub.c

@@ -162,79 +162,88 @@ static int dns_stub_collect_answer_by_question(
                 bool with_rrsig) { /* Add RRSIG RR matching each RR */
 
         _cleanup_(dns_resource_key_unrefp) DnsResourceKey *redirected_key = NULL;
+        unsigned n_cname_redirects = 0;
         DnsAnswerItem *item;
         int r;
 
         assert(reply);
 
-        /* Copies all RRs from 'answer' into 'reply', if they match 'question'. */
+        /* Copies all RRs from 'answer' into 'reply', if they match 'question'. There might be direct and
+         * indirect matches (i.e. via CNAME/DNAME). If they have an indirect one, remember where we need to
+         * go, and restart the loop */
+
+        for (;;) {
+                _cleanup_(dns_resource_key_unrefp) DnsResourceKey *next_redirected_key = NULL;
+
+                DNS_ANSWER_FOREACH_ITEM(item, answer) {
+                        DnsResourceKey *k = NULL;
+
+                        if (redirected_key) {
+                                /* There was a redirect in this packet, let's collect all matching RRs for the redirect */
+                                r = dns_resource_key_match_rr(redirected_key, item->rr, NULL);
+                                if (r < 0)
+                                        return r;
+
+                                k = redirected_key;
+                        } else if (question) {
+                                /* We have a question, let's see if this RR matches it */
+                                r = dns_question_matches_rr(question, item->rr, NULL);
+                                if (r < 0)
+                                        return r;
+
+                                k = question->keys[0];
+                        } else
+                                r = 1; /* No question, everything matches */
 
-        DNS_ANSWER_FOREACH_ITEM(item, answer) {
-                if (question) {
-                        r = dns_question_matches_rr(question, item->rr, NULL);
-                        if (r < 0)
-                                return r;
                         if (r == 0) {
                                 _cleanup_free_ char *target = NULL;
 
                                 /* OK, so the RR doesn't directly match. Let's see if the RR is a matching
                                  * CNAME or DNAME */
 
-                                r = dns_resource_record_get_cname_target(
-                                                question->keys[0],
-                                                item->rr,
-                                                &target);
+                                assert(k);
+
+                                r = dns_resource_record_get_cname_target(k, item->rr, &target);
                                 if (r == -EUNATCH)
                                         continue; /* Not a CNAME/DNAME or doesn't match */
                                 if (r < 0)
                                         return r;
 
-                                dns_resource_key_unref(redirected_key);
+                                /* Oh, wow, this is a redirect. Let's remember where this points, and store
+                                 * it in 'next_redirected_key'. Once we finished iterating through the rest
+                                 * of the RR's we'll start again, with the redirected RR key. */
+
+                                n_cname_redirects++;
+                                if (n_cname_redirects > CNAME_REDIRECT_MAX) /* don't loop forever */
+                                        return -ELOOP;
+
+                                dns_resource_key_unref(next_redirected_key);
 
                                 /* There can only be one CNAME per name, hence no point in storing more than one here */
-                                redirected_key = dns_resource_key_new(question->keys[0]->class, question->keys[0]->type, target);
-                                if (!redirected_key)
+                                next_redirected_key = dns_resource_key_new(k->class, k->type, target);
+                                if (!next_redirected_key)
                                         return -ENOMEM;
                         }
+
+                        /* Mask the section info, we want the primary answers to always go without section info, so
+                         * that it is added to the answer section when we synthesize a reply. */
+
+                        r = reply_add_with_rrsig(
+                                        reply,
+                                        item->rr,
+                                        item->ifindex,
+                                        item->flags & ~DNS_ANSWER_MASK_SECTIONS,
+                                        item->rrsig,
+                                        with_rrsig);
+                        if (r < 0)
+                                return r;
                 }
 
-                /* Mask the section info, we want the primary answers to always go without section info, so
-                 * that it is added to the answer section when we synthesize a reply. */
+                if (!next_redirected_key)
+                        break;
 
-                r = reply_add_with_rrsig(
-                                reply,
-                                item->rr,
-                                item->ifindex,
-                                item->flags & ~DNS_ANSWER_MASK_SECTIONS,
-                                item->rrsig,
-                                with_rrsig);
-                if (r < 0)
-                        return r;
-        }
-
-        if (!redirected_key)
-                return 0;
-
-        /* This is a CNAME/DNAME answer. In this case also append where the redirections point to to the main
-         * answer section */
-
-        DNS_ANSWER_FOREACH_ITEM(item, answer) {
-
-                r = dns_resource_key_match_rr(redirected_key, item->rr, NULL);
-                if (r < 0)
-                        return r;
-                if (r == 0)
-                        continue;
-
-                r = reply_add_with_rrsig(
-                                reply,
-                                item->rr,
-                                item->ifindex,
-                                item->flags & ~DNS_ANSWER_MASK_SECTIONS,
-                                item->rrsig,
-                                with_rrsig);
-                if (r < 0)
-                        return r;
+                dns_resource_key_unref(redirected_key);
+                redirected_key = TAKE_PTR(next_redirected_key);
         }
 
         return 0;
@@ -552,6 +561,37 @@ static int dns_stub_send(
         return 0;
 }
 
+static int dns_stub_reply_with_edns0_do(DnsQuery *q) {
+         assert(q);
+
+        /* Reply with DNSSEC DO set? Only if client supports it; and we did any DNSSEC verification
+         * ourselves, or consider the data fully authenticated because we generated it locally, or the client
+         * set cd */
+
+         return DNS_PACKET_DO(q->request_packet) &&
+                 (q->answer_dnssec_result >= 0 ||        /* we did proper DNSSEC validation … */
+                  dns_query_fully_authenticated(q) ||    /* … or we considered it authentic otherwise … */
+                  DNS_PACKET_CD(q->request_packet));     /* … or client set CD */
+}
+
+static void dns_stub_suppress_duplicate_section_rrs(DnsQuery *q) {
+        /* If we follow a CNAME/DNAME chain we might end up populating our sections with redundant RRs
+         * because we built up the sections from multiple reply packets (one from each CNAME/DNAME chain
+         * element). E.g. it could be that an RR that was included in the first reply's additional section
+         * ends up being relevant as main answer in a subsequent reply in the chain. Let's clean this up, and
+         * remove everything in the "higher priority" sections from the "lower priority" sections.
+         *
+         * Note that this removal matches by RR keys instead of the full RRs. This is because RRsets should
+         * always end up in one section fully or not at all, but never be split among sections.
+         *
+         * Specifically: we remove ANSWER section RRs from the AUTHORITATIVE and ADDITIONAL sections, as well
+         * as AUTHORITATIVE section RRs from the ADDITIONAL section. */
+
+        dns_answer_remove_by_answer_keys(&q->reply_authoritative, q->reply_answer);
+        dns_answer_remove_by_answer_keys(&q->reply_additional, q->reply_answer);
+        dns_answer_remove_by_answer_keys(&q->reply_additional, q->reply_authoritative);
+}
+
 static int dns_stub_send_reply(
                 DnsQuery *q,
                 int rcode) {
@@ -562,21 +602,7 @@ static int dns_stub_send_reply(
 
         assert(q);
 
-        /* Reply with DNSSEC DO set? Only if client supports it; and we did any DNSSEC verification
-         * ourselves, or consider the data fully authenticated because we generated it locally, or
-         * the client set cd */
-        edns0_do =
-                DNS_PACKET_DO(q->request_packet) &&
-                (q->answer_dnssec_result >= 0 ||        /* we did proper DNSSEC validation … */
-                 dns_query_fully_authenticated(q) ||    /* … or we considered it authentic otherwise … */
-                 DNS_PACKET_CD(q->request_packet));     /* … or client set CD */
-
-        r = dns_stub_assign_sections(
-                        q,
-                        q->request_packet->question,
-                        edns0_do);
-        if (r < 0)
-                return log_debug_errno(r, "Failed to assign sections: %m");
+        edns0_do = dns_stub_reply_with_edns0_do(q); /* let's check if we shall reply with EDNS0 DO? */
 
         r = dns_stub_make_reply_packet(
                         &reply,
@@ -586,6 +612,8 @@ static int dns_stub_send_reply(
         if (r < 0)
                 return log_debug_errno(r, "Failed to build reply packet: %m");
 
+        dns_stub_suppress_duplicate_section_rrs(q);
+
         r = dns_stub_add_reply_packet_body(
                         reply,
                         q->reply_answer,
@@ -728,13 +756,37 @@ static void dns_stub_query_complete(DnsQuery *q) {
                 }
         }
 
-        /* Note that we don't bother with following CNAMEs here. We propagate the authoritative/additional
-         * sections from the upstream answer however, hence if the upstream server collected that information
-         * already we don't have to collect it ourselves anymore. */
+        /* Take all data from the current reply, and merge it into the three reply sections we are building
+         * up. We do this before processing CNAME redirects, so that we gradually build up our sections, and
+         * and keep adding all RRs in the CNAME chain. */
+        r = dns_stub_assign_sections(
+                        q,
+                        q->request_packet->question,
+                        dns_stub_reply_with_edns0_do(q));
+        if (r < 0) {
+                log_debug_errno(r, "Failed to assign sections: %m");
+                dns_query_free(q);
+                return;
+        }
 
         switch (q->state) {
 
         case DNS_TRANSACTION_SUCCESS:
+                r = dns_query_process_cname(q);
+                if (r == -ELOOP) { /* CNAME loop, let's send what we already have */
+                        log_debug_errno(r, "Detected CNAME loop, returning what we already have.");
+                        (void) dns_stub_send_reply(q, q->answer_rcode);
+                        break;
+                }
+                if (r < 0) {
+                        log_debug_errno(r, "Failed to process CNAME: %m");
+                        break;
+                }
+                if (r == DNS_QUERY_RESTARTED)
+                        return;
+
+                _fallthrough_;
+
         case DNS_TRANSACTION_RCODE_FAILURE:
                 (void) dns_stub_send_reply(q, q->answer_rcode);
                 break;
@@ -873,7 +925,6 @@ static void dns_stub_process_query(Manager *m, DnsStubListenerExtra *l, DnsStrea
                 r = dns_query_new(m, &q, p->question, p->question, NULL, 0,
                                   SD_RESOLVED_PROTOCOLS_ALL|
                                   SD_RESOLVED_NO_SEARCH|
-                                  SD_RESOLVED_NO_CNAME|
                                   (DNS_PACKET_DO(p) ? SD_RESOLVED_REQUIRE_PRIMARY : 0)|
                                   SD_RESOLVED_CLAMP_TTL);
         if (r < 0) {

src/shared/dissect-image.c

@@ -1570,6 +1570,7 @@ DecryptedImage* decrypted_image_unref(DecryptedImage* d) {
                 free(p->name);
         }
 
+        free(d->decrypted);
         free(d);
 #endif
         return NULL;

src/shared/format-table.h

@@ -102,9 +102,9 @@ void table_set_cell_height_max(Table *t, size_t height);
 int table_set_empty_string(Table *t, const char *empty);
 int table_set_display_all(Table *t);
 int table_set_display_internal(Table *t, size_t first_column, ...);
-#define table_set_display(...) table_set_display_internal(__VA_ARGS__, (size_t) SIZE_MAX)
+#define table_set_display(...) table_set_display_internal(__VA_ARGS__, SIZE_MAX)
 int table_set_sort_internal(Table *t, size_t first_column, ...);
-#define table_set_sort(...) table_set_sort_internal(__VA_ARGS__, (size_t) SIZE_MAX)
+#define table_set_sort(...) table_set_sort_internal(__VA_ARGS__, SIZE_MAX)
 int table_set_reverse(Table *t, size_t column, bool b);
 int table_hide_column_from_display(Table *t, size_t column);
 

src/shared/nsflags.h

@@ -16,7 +16,7 @@
                           CLONE_NEWUSER|                                \
                           CLONE_NEWUTS))
 
-#define NAMESPACE_FLAGS_INITIAL  (ULONG_MAX)
+#define NAMESPACE_FLAGS_INITIAL  ULONG_MAX
 
 int namespace_flags_from_string(const char *name, unsigned long *ret);
 int namespace_flags_to_string(unsigned long flags, char **ret);

src/systemd/sd-lldp.h

@@ -81,7 +81,7 @@ enum {
         SD_LLDP_SYSTEM_CAPABILITIES_TPMR     = 1 << 10,
 };
 
-#define SD_LLDP_SYSTEM_CAPABILITIES_ALL (UINT16_MAX)
+#define SD_LLDP_SYSTEM_CAPABILITIES_ALL UINT16_MAX
 
 #define SD_LLDP_SYSTEM_CAPABILITIES_ALL_ROUTERS                         \
         ((uint16_t)                                                     \

test/fuzz/fuzz-bus-match/bugzilla1935084.input

@@ -0,0 +1 @@
+type='signal',sender='org.freedesktop.DBus',path='/org/freedesktop/DBus',interface='org.freedesktop.DBus',member='NameOwnerChanged',arg0=':1.134'

test/fuzz/fuzz-bus-match/test.input

@@ -0,0 +1,18 @@
+arg2='wal\'do',sender='foo',type='signal',interface='bar.x',
+arg2='wal\'do2',sender='foo',type='signal',interface='bar.x',
+arg3='test',sender='foo',type='signal',interface='bar.x',
+arg3='test',sender='foo',type='method_call',interface='bar.x',
+
+interface='quux.x'
+interface='bar.x'
+member='waldo',path='/foo/bar'
+path='/foo/bar'
+path_namespace='/foo'
+path_namespace='/foo/quux'
+arg1='two'
+member='waldo',arg2path='/prefix/'
+member=waldo,path='/foo/bar',arg3namespace='prefix'
+arg4has='pi'
+arg4has='pa'
+arg4has='po'
+arg4='pi'