network: fix use-after-free

The function sd_radv_add_prefix() in dhcp6_pd_prefix_assign() may
return -EEXIST, and in that case the sd_radv_prefix object allocated
in dhcp6_pd_prefix_assign() will be freed when the function returns.
Hence, the key value in Manager::dhcp6_prefixes hashmap is lost.

diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c
index b921eed8d7b7515a69b0f112b505376a3ec50e71..7db59d3acad6176deff08ece300aa23357797d8e 100644 (file)
--- a/src/network/networkd-dhcp6.c
+++ b/src/network/networkd-dhcp6.c
@@ -90,7 +90,7 @@ static int dhcp6_pd_prefix_assign(Link *link, struct in6_addr *prefix,
         if (r < 0 && r != -EEXIST)
                 return r;
 
-        r = manager_dhcp6_prefix_add(link->manager, &p->opt.in6_addr, link);
+        r = manager_dhcp6_prefix_add(link->manager, prefix, link);
         if (r < 0)
                 return r;
 
@@ -203,7 +203,7 @@ static int dhcp6_pd_prefix_distribute(Link *dhcp6_link, Iterator *i,
                         continue;
 
                 assigned_link = manager_dhcp6_prefix_get(manager, &prefix.in6);
-                if (assigned_link != NULL && assigned_link != link)
+                if (assigned_link && assigned_link != link)
                         continue;
 
                 (void) in_addr_to_string(AF_INET6, &prefix, &assigned_buf);

diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index 0a679e4ba63c24b733d9b76aa54619ccf41389af..c61ad8c52a788eccdfd3631ddd272f96dd90a836 100644 (file)
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -524,8 +524,6 @@ static int link_update_flags(Link *link, sd_netlink_message *m) {
         return 0;
 }
 
-DEFINE_TRIVIAL_CLEANUP_FUNC(Link*, link_unref);
-
 static int link_new(Manager *manager, sd_netlink_message *message, Link **ret) {
         _cleanup_(link_unrefp) Link *link = NULL;
         uint16_t type;

diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h
index 1366a299242b0947d90aa258a70df9bb75dfb841..930dd25f924d52a5a5d18c0c698da7917f83496d 100644 (file)
--- a/src/network/networkd-link.h
+++ b/src/network/networkd-link.h
@@ -132,6 +132,7 @@ int get_product_uuid_handler(sd_bus_message *m, void *userdata, sd_bus_error *re
 
 Link *link_unref(Link *link);
 Link *link_ref(Link *link);
+DEFINE_TRIVIAL_CLEANUP_FUNC(Link*, link_unref);
 DEFINE_TRIVIAL_DESTRUCTOR(link_netlink_destroy_callback, Link, link_unref);
 
 int link_get(Manager *m, int ifindex, Link **ret);

diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c
index 98d72597822602ce09b0451025fb6910001d388f..13e3d7f20041c4f8f3eda840216c044dc045e554 100644 (file)
--- a/src/network/networkd-manager.c
+++ b/src/network/networkd-manager.c
@@ -1279,7 +1279,9 @@ static int dhcp6_prefixes_compare_func(const struct in6_addr *a, const struct in
 DEFINE_PRIVATE_HASH_OPS(dhcp6_prefixes_hash_ops, struct in6_addr, dhcp6_prefixes_hash_func, dhcp6_prefixes_compare_func);
 
 int manager_dhcp6_prefix_add(Manager *m, struct in6_addr *addr, Link *link) {
+        _cleanup_free_ struct in6_addr *a = NULL;
         _cleanup_free_ char *buf = NULL;
+        Link *assigned_link;
         Route *route;
         int r;
 
@@ -1298,11 +1300,27 @@ int manager_dhcp6_prefix_add(Manager *m, struct in6_addr *addr, Link *link) {
         (void) in_addr_to_string(AF_INET6, (union in_addr_union *) addr, &buf);
         log_link_debug(link, "Adding prefix route %s/64", strnull(buf));
 
+        assigned_link = hashmap_get(m->dhcp6_prefixes, addr);
+        if (assigned_link) {
+                assert(assigned_link == link);
+                return 0;
+        }
+
+        a = newdup(struct in6_addr, addr, 1);
+        if (!a)
+                return -ENOMEM;
+
         r = hashmap_ensure_allocated(&m->dhcp6_prefixes, &dhcp6_prefixes_hash_ops);
         if (r < 0)
                 return r;
 
-        return hashmap_put(m->dhcp6_prefixes, addr, link);
+        r = hashmap_put(m->dhcp6_prefixes, a, link);
+        if (r < 0)
+                return r;
+
+        TAKE_PTR(a);
+        link_ref(link);
+        return 0;
 }
 
 static int dhcp6_route_remove_handler(sd_netlink *nl, sd_netlink_message *m, Link *link) {
@@ -1318,21 +1336,21 @@ static int dhcp6_route_remove_handler(sd_netlink *nl, sd_netlink_message *m, Lin
 }
 
 static int manager_dhcp6_prefix_remove(Manager *m, struct in6_addr *addr) {
+        _cleanup_free_ struct in6_addr *a = NULL;
+        _cleanup_(link_unrefp) Link *l = NULL;
         _cleanup_free_ char *buf = NULL;
         Route *route;
-        Link *l;
         int r;
 
         assert_return(m, -EINVAL);
         assert_return(addr, -EINVAL);
 
-        l = hashmap_remove(m->dhcp6_prefixes, addr);
+        l = hashmap_remove2(m->dhcp6_prefixes, addr, (void **) &a);
         if (!l)
                 return -EINVAL;
 
         (void) sd_radv_remove_prefix(l->radv, addr, 64);
-        r = route_get(l, AF_INET6, (union in_addr_union *) addr, 64,
-                      0, 0, 0, &route);
+        r = route_get(l, AF_INET6, (union in_addr_union *) addr, 64, 0, 0, 0, &route);
         if (r < 0)
                 return r;
 
@@ -1354,12 +1372,9 @@ int manager_dhcp6_prefix_remove_all(Manager *m, Link *link) {
         assert_return(m, -EINVAL);
         assert_return(link, -EINVAL);
 
-        HASHMAP_FOREACH_KEY(l, addr, m->dhcp6_prefixes, i) {
-                if (l != link)
-                        continue;
-
-                (void) manager_dhcp6_prefix_remove(m, addr);
-        }
+        HASHMAP_FOREACH_KEY(l, addr, m->dhcp6_prefixes, i)
+                if (l == link)
+                        (void) manager_dhcp6_prefix_remove(m, addr);
 
         return 0;
 }
@@ -1424,6 +1439,7 @@ int manager_new(Manager **ret) {
 }
 
 void manager_free(Manager *m) {
+        struct in6_addr *a;
         AddressPool *pool;
         Link *link;
 
@@ -1432,8 +1448,8 @@ void manager_free(Manager *m) {
 
         free(m->state_file);
 
-        while ((link = hashmap_first(m->dhcp6_prefixes)))
-                manager_dhcp6_prefix_remove_all(m, link);
+        while ((a = hashmap_first_key(m->dhcp6_prefixes)))
+                (void) manager_dhcp6_prefix_remove(m, a);
         hashmap_free(m->dhcp6_prefixes);
 
         while ((link = hashmap_steal_first(m->links))) {