networkd: use OrderedSets instead of strvs to store lists of domains

We were already using OrderedSets in the manager object, but strvs in the
configuration parsing code. Using sets gives us better scaling when many
domains are used.

In oss-fuzz #13059 the attached reproducer takes approximately 30.5 s to be
parsed. Converting to sets makes this go down to 10s. This is not _vastly_
faster, but using sets seems like a nicer approach anyway. In particular, we
avoid the quadratic de-unification operation after each addition.

diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index 2aea0b839c1ec91b518acda9a52587431483a06c..8418d660897bf9e7a9a584cd95d7f644ee366e39 100644 (file)
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -4083,9 +4083,7 @@ int link_save(Link *link) {
                                 (void) sd_dhcp6_lease_get_domains(dhcp6_lease, &dhcp6_domains);
                 }
 
-                fputs("DOMAINS=", f);
-                space = false;
-                fputstrv(f, link->network->search_domains, NULL, &space);
+                ordered_set_print(f, "DOMAINS=", link->network->search_domains);
 
                 if (link->network->dhcp_use_domains == DHCP_USE_DOMAINS_YES) {
                         NDiscDNSSL *dd;
@@ -4103,9 +4101,7 @@ int link_save(Link *link) {
 
                 fputc('\n', f);
 
-                fputs("ROUTE_DOMAINS=", f);
-                space = false;
-                fputstrv(f, link->network->route_domains, NULL, &space);
+                ordered_set_print(f, "ROUTE_DOMAINS=", link->network->route_domains);
 
                 if (link->network->dhcp_use_domains == DHCP_USE_DOMAINS_ROUTE) {
                         NDiscDNSSL *dd;

diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c
index 758132d39d097c4ec559b92e614140f40689ef25..8ee7bca4357004d6670078a7f27f39086485372c 100644 (file)
--- a/src/network/networkd-manager.c
+++ b/src/network/networkd-manager.c
@@ -1108,11 +1108,11 @@ static int manager_save(Manager *m) {
                 if (r < 0)
                         return r;
 
-                r = ordered_set_put_strdupv(search_domains, link->network->search_domains);
+                r = ordered_set_put_string_set(search_domains, link->network->search_domains);
                 if (r < 0)
                         return r;
 
-                r = ordered_set_put_strdupv(route_domains, link->network->route_domains);
+                r = ordered_set_put_string_set(route_domains, link->network->route_domains);
                 if (r < 0)
                         return r;
 

diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c
index 727b2da9f42d3fecee6d759105347059c5a41ad6..a83df49038aa8f238594969cfcab27681636ac33 100644 (file)
--- a/src/network/networkd-network.c
+++ b/src/network/networkd-network.c
@@ -385,11 +385,11 @@ void network_free(Network *network) {
 
         strv_free(network->ntp);
         free(network->dns);
-        strv_free(network->search_domains);
-        strv_free(network->route_domains);
+        ordered_set_free_free(network->search_domains);
+        ordered_set_free_free(network->route_domains);
         strv_free(network->bind_carrier);
 
-        strv_free(network->router_search_domains);
+        ordered_set_free_free(network->router_search_domains);
         free(network->router_dns);
 
         netdev_unref(network->bridge);
@@ -553,8 +553,8 @@ int network_apply(Network *network, Link *link) {
 
         if (network->n_dns > 0 ||
             !strv_isempty(network->ntp) ||
-            !strv_isempty(network->search_domains) ||
-            !strv_isempty(network->route_domains))
+            !ordered_set_isempty(network->search_domains) ||
+            !ordered_set_isempty(network->route_domains))
                 link_dirty(link);
 
         return 0;
@@ -686,8 +686,8 @@ int config_parse_domains(
         assert(rvalue);
 
         if (isempty(rvalue)) {
-                n->search_domains = strv_free(n->search_domains);
-                n->route_domains = strv_free(n->route_domains);
+                n->search_domains = ordered_set_free_free(n->search_domains);
+                n->route_domains = ordered_set_free_free(n->route_domains);
                 return 0;
         }
 
@@ -733,17 +733,16 @@ int config_parse_domains(
                         }
                 }
 
-                if (is_route)
-                        r = strv_extend(&n->route_domains, domain);
-                else
-                        r = strv_extend(&n->search_domains, domain);
+                OrderedSet **set = is_route ? &n->route_domains : &n->search_domains;
+                r = ordered_set_ensure_allocated(set, &string_hash_ops);
+                if (r < 0)
+                        return r;
+
+                r = ordered_set_put_strdup(*set, domain);
                 if (r < 0)
                         return log_oom();
         }
 
-        strv_uniq(n->route_domains);
-        strv_uniq(n->search_domains);
-
         return 0;
 }
 
@@ -1218,16 +1217,17 @@ int config_parse_radv_search_domains(
                         log_syntax(unit, LOG_ERR, filename, line, r,
                                    "Failed to apply IDNA to domain name '%s', ignoring: %m", w);
                         continue;
-                }
-                if (r > 0) {
-                        r = strv_push(&n->router_search_domains, idna);
-                        if (r >= 0)
-                                idna = NULL;
-                } else {
-                        r = strv_push(&n->router_search_domains, w);
-                        if (r >= 0)
-                                w = NULL;
-                }
+                } else if (r == 0)
+                        /* transfer ownership to simplify subsequent operations */
+                        idna = TAKE_PTR(w);
+
+                r = ordered_set_ensure_allocated(&n->router_search_domains, &string_hash_ops);
+                if (r < 0)
+                        return r;
+
+                r = ordered_set_consume(n->router_search_domains, TAKE_PTR(idna));
+                if (r < 0)
+                        return r;
         }
 
         return 0;

diff --git a/src/network/networkd-network.h b/src/network/networkd-network.h
index e61c391c7661eb638bdd535f35c6f5d8618a908f..5681e8d2121262d0a5355ff7d9eb2a47ea147340 100644 (file)
--- a/src/network/networkd-network.h
+++ b/src/network/networkd-network.h
@@ -20,6 +20,7 @@
 #include "networkd-route.h"
 #include "networkd-routing-policy-rule.h"
 #include "networkd-util.h"
+#include "ordered-set.h"
 #include "resolve-util.h"
 
 #define DHCP_ROUTE_METRIC 1024
@@ -173,7 +174,7 @@ struct Network {
         usec_t router_dns_lifetime_usec;
         struct in6_addr *router_dns;
         unsigned n_router_dns;
-        char **router_search_domains;
+        OrderedSet *router_search_domains;
         bool dhcp6_force_pd_other_information; /* Start DHCPv6 PD also when 'O'
                                                   RA flag is set, see RFC 7084,
                                                   WPD-4 */
@@ -267,7 +268,8 @@ struct Network {
         /* All kinds of DNS configuration */
         struct in_addr_data *dns;
         unsigned n_dns;
-        char **search_domains, **route_domains;
+        OrderedSet *search_domains, *route_domains;
+
         int dns_default_route;
         ResolveSupport llmnr;
         ResolveSupport mdns;

diff --git a/src/network/networkd-radv.c b/src/network/networkd-radv.c
index 92cead052a66797a67bbbc054e38a2e65c02420f..ecf984e226b34dcc1488b11b24a0499f6a177333 100644 (file)
--- a/src/network/networkd-radv.c
+++ b/src/network/networkd-radv.c
@@ -391,8 +391,9 @@ static int radv_set_dns(Link *link, Link *uplink) {
 }
 
 static int radv_set_domains(Link *link, Link *uplink) {
-        char **search_domains;
+        OrderedSet *search_domains;
         usec_t lifetime_usec;
+        _cleanup_free_ char **s = NULL; /* just free() because the strings are owned by the set */
 
         if (!link->network->router_emit_domains)
                 return 0;
@@ -423,9 +424,13 @@ static int radv_set_domains(Link *link, Link *uplink) {
         return 0;
 
  set_domains:
+        s = ordered_set_get_strv(search_domains);
+        if (!s)
+                return log_oom();
+
         return sd_radv_set_dnssl(link->radv,
                                  DIV_ROUND_UP(lifetime_usec, USEC_PER_SEC),
-                                 search_domains);
+                                 s);
 
 }
 

diff --git a/test/fuzz/fuzz-network-parser/oss-fuzz-13059 b/test/fuzz/fuzz-network-parser/oss-fuzz-13059
new file mode 100644 (file)
index 0000000..a605c79
Binary files /dev/null and b/test/fuzz/fuzz-network-parser/oss-fuzz-13059 differ