From 95931532aa0f9f951454a45e078a49dca4341dfc Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 16 Mar 2022 21:12:37 +0900 Subject: [PATCH 1/3] sd-radv: voidify sd_radv_remove_prefix() If the prefix is only referenced by sd_radv, then the returned pointer is already freed. networkd does not uses the returned value. Let's voidify the function. --- src/libsystemd-network/sd-radv.c | 24 ++++++++++--------- src/network/networkd-dhcp-prefix-delegation.c | 4 ++-- src/systemd/sd-radv.h | 3 +-- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/libsystemd-network/sd-radv.c b/src/libsystemd-network/sd-radv.c index e332f6a2ab..752c09d890 100644 --- a/src/libsystemd-network/sd-radv.c +++ b/src/libsystemd-network/sd-radv.c @@ -659,15 +659,20 @@ int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p) { return 0; } -sd_radv_prefix *sd_radv_remove_prefix(sd_radv *ra, - const struct in6_addr *prefix, - unsigned char prefixlen) { - sd_radv_prefix *cur, *next; +void sd_radv_remove_prefix( + sd_radv *ra, + const struct in6_addr *prefix, + unsigned char prefixlen) { - assert_return(ra, NULL); - assert_return(prefix, NULL); + sd_radv_prefix *cur; - LIST_FOREACH_SAFE(prefix, cur, next, ra->prefixes) { + if (!ra) + return; + + if (!prefix) + return; + + LIST_FOREACH(prefix, cur, ra->prefixes) { if (prefixlen != cur->opt.prefixlen) continue; @@ -677,11 +682,8 @@ sd_radv_prefix *sd_radv_remove_prefix(sd_radv *ra, LIST_REMOVE(prefix, ra->prefixes, cur); ra->n_prefixes--; sd_radv_prefix_unref(cur); - - break; + return; } - - return cur; } int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p) { diff --git a/src/network/networkd-dhcp-prefix-delegation.c b/src/network/networkd-dhcp-prefix-delegation.c index edc893e9f6..775acb3f95 100644 --- a/src/network/networkd-dhcp-prefix-delegation.c +++ b/src/network/networkd-dhcp-prefix-delegation.c @@ -187,7 +187,7 @@ int dhcp_pd_remove(Link *link, bool only_marked) { continue; if (link->radv) - (void) sd_radv_remove_prefix(link->radv, &route->dst.in6, 64); + sd_radv_remove_prefix(link->radv, &route->dst.in6, 64); link_remove_dhcp_pd_subnet_prefix(link, &route->dst.in6); @@ -212,7 +212,7 @@ int dhcp_pd_remove(Link *link, bool only_marked) { in6_addr_mask(&prefix, 64); if (link->radv) - (void) sd_radv_remove_prefix(link->radv, &prefix, 64); + sd_radv_remove_prefix(link->radv, &prefix, 64); link_remove_dhcp_pd_subnet_prefix(link, &prefix); diff --git a/src/systemd/sd-radv.h b/src/systemd/sd-radv.h index e604df1371..6eeb92c48b 100644 --- a/src/systemd/sd-radv.h +++ b/src/systemd/sd-radv.h @@ -59,8 +59,7 @@ int sd_radv_set_other_information(sd_radv *ra, int other); int sd_radv_set_preference(sd_radv *ra, unsigned preference); int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p); int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p); -sd_radv_prefix *sd_radv_remove_prefix(sd_radv *ra, const struct in6_addr *prefix, - unsigned char prefixlen); +void sd_radv_remove_prefix(sd_radv *ra, const struct in6_addr *prefix, unsigned char prefixlen); int sd_radv_set_rdnss(sd_radv *ra, uint32_t lifetime, const struct in6_addr *dns, size_t n_dns); int sd_radv_set_dnssl(sd_radv *ra, uint32_t lifetime, char **search_list); From 56aa51432e0dd284f202c327f29b74cf2b10734a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 16 Mar 2022 21:16:54 +0900 Subject: [PATCH 2/3] sd-radv: do not use iterater outside of the loop --- src/libsystemd-network/sd-radv.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/libsystemd-network/sd-radv.c b/src/libsystemd-network/sd-radv.c index 752c09d890..56ccd4867c 100644 --- a/src/libsystemd-network/sd-radv.c +++ b/src/libsystemd-network/sd-radv.c @@ -578,8 +578,7 @@ int sd_radv_set_preference(sd_radv *ra, unsigned preference) { int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p) { _cleanup_free_ char *addr_p = NULL; - sd_radv_prefix *cur; - bool update = false; + sd_radv_prefix *cur, *found = NULL; int r; assert_return(ra, -EINVAL); @@ -604,7 +603,7 @@ int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p) { continue; if (cur->opt.prefixlen == p->opt.prefixlen) { - update = true; + found = cur; break; } @@ -615,15 +614,13 @@ int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p) { strna(addr_p), strna(addr_cur)); } - if (update) { - assert(cur); - + if (found) { /* p and cur may be equivalent. First increment the reference counter. */ sd_radv_prefix_ref(p); /* Then, remove the old entry. */ - LIST_REMOVE(prefix, ra->prefixes, cur); - sd_radv_prefix_unref(cur); + LIST_REMOVE(prefix, ra->prefixes, found); + sd_radv_prefix_unref(found); /* Finally, add the new entry. */ LIST_APPEND(prefix, ra->prefixes, p); @@ -688,8 +685,7 @@ void sd_radv_remove_prefix( int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p) { _cleanup_free_ char *addr_p = NULL; - sd_radv_route_prefix *cur; - bool update = false; + sd_radv_route_prefix *cur, *found = NULL; int r; assert_return(ra, -EINVAL); @@ -710,7 +706,7 @@ int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p) { continue; if (cur->opt.prefixlen == p->opt.prefixlen) { - update = true; + found = cur; break; } @@ -721,15 +717,13 @@ int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p) { strna(addr_p), strna(addr_cur)); } - if (update) { - assert(cur); - + if (found) { /* p and cur may be equivalent. First increment the reference counter. */ sd_radv_route_prefix_ref(p); /* Then, remove the old entry. */ - LIST_REMOVE(prefix, ra->route_prefixes, cur); - sd_radv_route_prefix_unref(cur); + LIST_REMOVE(prefix, ra->route_prefixes, found); + sd_radv_route_prefix_unref(found); /* Finally, add the new entry. */ LIST_APPEND(prefix, ra->route_prefixes, p); From faaf3d66ceb42169f754df22e82ac6219cd77d40 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 16 Mar 2022 21:18:57 +0900 Subject: [PATCH 3/3] sd-radv: fix indentation --- src/libsystemd-network/sd-radv.c | 39 ++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/src/libsystemd-network/sd-radv.c b/src/libsystemd-network/sd-radv.c index 56ccd4867c..c2f633a82b 100644 --- a/src/libsystemd-network/sd-radv.c +++ b/src/libsystemd-network/sd-radv.c @@ -758,8 +758,12 @@ int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p) { return 0; } -int sd_radv_set_rdnss(sd_radv *ra, uint32_t lifetime, - const struct in6_addr *dns, size_t n_dns) { +int sd_radv_set_rdnss( + sd_radv *ra, + uint32_t lifetime, + const struct in6_addr *dns, + size_t n_dns) { + _cleanup_free_ struct sd_radv_opt_dns *opt_rdnss = NULL; size_t len; @@ -792,8 +796,11 @@ int sd_radv_set_rdnss(sd_radv *ra, uint32_t lifetime, return 0; } -int sd_radv_set_dnssl(sd_radv *ra, uint32_t lifetime, - char **search_list) { +int sd_radv_set_dnssl( + sd_radv *ra, + uint32_t lifetime, + char **search_list) { + _cleanup_free_ struct sd_radv_opt_dns *opt_dnssl = NULL; size_t len = 0; char **s; @@ -872,8 +879,11 @@ int sd_radv_prefix_new(sd_radv_prefix **ret) { DEFINE_PUBLIC_TRIVIAL_REF_UNREF_FUNC(sd_radv_prefix, sd_radv_prefix, mfree); -int sd_radv_prefix_set_prefix(sd_radv_prefix *p, const struct in6_addr *in6_addr, - unsigned char prefixlen) { +int sd_radv_prefix_set_prefix( + sd_radv_prefix *p, + const struct in6_addr *in6_addr, + unsigned char prefixlen) { + assert_return(p, -EINVAL); assert_return(in6_addr, -EINVAL); @@ -890,8 +900,11 @@ int sd_radv_prefix_set_prefix(sd_radv_prefix *p, const struct in6_addr *in6_addr return 0; } -int sd_radv_prefix_get_prefix(sd_radv_prefix *p, struct in6_addr *ret_in6_addr, - unsigned char *ret_prefixlen) { +int sd_radv_prefix_get_prefix( + sd_radv_prefix *p, + struct in6_addr *ret_in6_addr, + unsigned char *ret_prefixlen) { + assert_return(p, -EINVAL); assert_return(ret_in6_addr, -EINVAL); assert_return(ret_prefixlen, -EINVAL); @@ -910,8 +923,7 @@ int sd_radv_prefix_set_onlink(sd_radv_prefix *p, int onlink) { return 0; } -int sd_radv_prefix_set_address_autoconfiguration(sd_radv_prefix *p, - int address_autoconfiguration) { +int sd_radv_prefix_set_address_autoconfiguration(sd_radv_prefix *p, int address_autoconfiguration) { assert_return(p, -EINVAL); SET_FLAG(p->opt.flags, ND_OPT_PI_FLAG_AUTO, address_autoconfiguration); @@ -963,8 +975,11 @@ int sd_radv_route_prefix_new(sd_radv_route_prefix **ret) { DEFINE_PUBLIC_TRIVIAL_REF_UNREF_FUNC(sd_radv_route_prefix, sd_radv_route_prefix, mfree); -int sd_radv_route_prefix_set_prefix(sd_radv_route_prefix *p, const struct in6_addr *in6_addr, - unsigned char prefixlen) { +int sd_radv_route_prefix_set_prefix( + sd_radv_route_prefix *p, + const struct in6_addr *in6_addr, + unsigned char prefixlen) { + assert_return(p, -EINVAL); assert_return(in6_addr, -EINVAL);