From: Yu Watanabe Date: Sat, 4 May 2019 17:43:45 +0000 (+0200) Subject: network: fix use-after-free X-Git-Tag: v243-rc1~475^2~3 X-Git-Url: http://git.ipfire.org/?p=thirdparty%2Fsystemd.git;a=commitdiff_plain;h=2c448c8a174a4dc299c68941f742365d8fb773f0 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 b921eed8d7b..7db59d3acad 100644 --- 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 0a679e4ba63..c61ad8c52a7 100644 --- 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 1366a299242..930dd25f924 100644 --- 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 98d72597822..13e3d7f2004 100644 --- 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))) {