From 9d66b48c7c517ef56c648ebe5d068ecc89c584e6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Sep 2018 01:52:04 +0900 Subject: [PATCH 01/12] network: use structured initializer at one more place --- src/network/networkd-routing-policy-rule.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 588e618351..afa1449191 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -17,12 +17,14 @@ int routing_policy_rule_new(RoutingPolicyRule **ret) { RoutingPolicyRule *rule; - rule = new0(RoutingPolicyRule, 1); + rule = new(RoutingPolicyRule, 1); if (!rule) return -ENOMEM; - rule->family = AF_INET; - rule->table = RT_TABLE_MAIN; + *rule = (RoutingPolicyRule) { + .family = AF_INET, + .table = RT_TABLE_MAIN, + }; *ret = rule; return 0; From 1b3194d8f2e9ae31b713ae58c6926a454ee84a5b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Sep 2018 01:59:19 +0900 Subject: [PATCH 02/12] network: fix memleak about routing policy --- src/network/networkd-manager.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index bdb1aacf7f..1c23830047 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -1470,9 +1470,8 @@ void manager_free(Manager *m) { while ((pool = m->address_pools)) address_pool_free(pool); - set_free(m->rules); - set_free(m->rules_foreign); - + set_free_with_destructor(m->rules, routing_policy_rule_free); + set_free_with_destructor(m->rules_foreign, routing_policy_rule_free); set_free_with_destructor(m->rules_saved, routing_policy_rule_free); sd_netlink_unref(m->rtnl); From 8e37fcbd2151a93584e07a36d7f700be22aaf9e3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Sep 2018 17:26:04 +0900 Subject: [PATCH 03/12] network: use TAKE_PTR() at one more place --- src/network/networkd-routing-policy-rule.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index afa1449191..ce9e1416a9 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -394,9 +394,8 @@ static int routing_policy_rule_new_static(Network *network, const char *filename if (r < 0) return r; - rule->section = n; + rule->section = TAKE_PTR(n); rule->network = network; - n = NULL; r = hashmap_put(network->rules_by_section, rule->section, rule); if (r < 0) From eeab051b28ba6e1b4a56d369d4c6bf7cfa71947c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Sep 2018 17:27:04 +0900 Subject: [PATCH 04/12] network: strdup iif and oif when creating RoutingPolicyRule object --- src/network/networkd-manager.c | 6 ++-- src/network/networkd-routing-policy-rule.c | 37 +++++++++++++++------- src/network/networkd-routing-policy-rule.h | 6 ++-- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 1c23830047..a949f746c9 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -718,7 +718,7 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, voi union in_addr_union to = {}, from = {}; RoutingPolicyRule *rule = NULL; uint32_t fwmark = 0, table = 0; - char *iif = NULL, *oif = NULL; + const char *iif = NULL, *oif = NULL; Manager *m = userdata; uint16_t type; int family; @@ -834,13 +834,13 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, voi return 0; } - r = sd_netlink_message_read_string(message, FRA_IIFNAME, (const char **) &iif); + r = sd_netlink_message_read_string(message, FRA_IIFNAME, &iif); if (r < 0 && r != -ENODATA) { log_warning_errno(r, "rtnl: could not get FRA_IIFNAME attribute, ignoring: %m"); return 0; } - r = sd_netlink_message_read_string(message, FRA_OIFNAME, (const char **) &oif); + r = sd_netlink_message_read_string(message, FRA_OIFNAME, &oif); if (r < 0 && r != -ENODATA) { log_warning_errno(r, "rtnl: could not get FRA_OIFNAME attribute, ignoring: %m"); return 0; diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index ce9e1416a9..51b817e692 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -156,8 +156,8 @@ int routing_policy_rule_get(Manager *m, uint8_t tos, uint32_t fwmark, uint32_t table, - char *iif, - char *oif, + const char *iif, + const char *oif, RoutingPolicyRule **ret) { RoutingPolicyRule rule, *existing; @@ -173,8 +173,8 @@ int routing_policy_rule_get(Manager *m, .tos = tos, .fwmark = fwmark, .table = table, - .iif = iif, - .oif = oif + .iif = (char*) iif, + .oif = (char*) oif }; if (m->rules) { @@ -226,15 +226,28 @@ static int routing_policy_rule_add_internal(Manager *m, uint8_t tos, uint32_t fwmark, uint32_t table, - char *iif, - char *oif, + const char *_iif, + const char *_oif, RoutingPolicyRule **ret) { _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *rule = NULL; + _cleanup_free_ char *iif = NULL, *oif = NULL; int r; assert_return(rules, -EINVAL); + if (_iif) { + iif = strdup(_iif); + if (!iif) + return -ENOMEM; + } + + if (_oif) { + oif = strdup(_oif); + if (!oif) + return -ENOMEM; + } + r = routing_policy_rule_new(&rule); if (r < 0) return r; @@ -248,8 +261,8 @@ static int routing_policy_rule_add_internal(Manager *m, rule->tos = tos; rule->fwmark = fwmark; rule->table = table; - rule->iif = iif; - rule->oif = oif; + rule->iif = TAKE_PTR(iif); + rule->oif = TAKE_PTR(oif); r = set_ensure_allocated(rules, &routing_policy_rule_hash_ops); if (r < 0) @@ -276,8 +289,8 @@ int routing_policy_rule_add(Manager *m, uint8_t tos, uint32_t fwmark, uint32_t table, - char *iif, - char *oif, + const char *iif, + const char *oif, RoutingPolicyRule **ret) { return routing_policy_rule_add_internal(m, &m->rules, family, from, from_prefixlen, to, to_prefixlen, tos, fwmark, table, iif, oif, ret); @@ -292,8 +305,8 @@ int routing_policy_rule_add_foreign(Manager *m, uint8_t tos, uint32_t fwmark, uint32_t table, - char *iif, - char *oif, + const char *iif, + const char *oif, RoutingPolicyRule **ret) { return routing_policy_rule_add_internal(m, &m->rules_foreign, family, from, from_prefixlen, to, to_prefixlen, tos, fwmark, table, iif, oif, ret); } diff --git a/src/network/networkd-routing-policy-rule.h b/src/network/networkd-routing-policy-rule.h index 7843a54db4..2e7474ef56 100644 --- a/src/network/networkd-routing-policy-rule.h +++ b/src/network/networkd-routing-policy-rule.h @@ -54,11 +54,11 @@ int link_routing_policy_rule_remove_handler(sd_netlink *rtnl, sd_netlink_message int link_routing_policy_rule_handler(sd_netlink *rtnl, sd_netlink_message *m, void *userdata); int routing_policy_rule_add(Manager *m, int family, const union in_addr_union *from, uint8_t from_prefixlen, const union in_addr_union *to, uint8_t to_prefixlen, - uint8_t tos, uint32_t fwmark, uint32_t table, char *iif, char *oif, RoutingPolicyRule **ret); + uint8_t tos, uint32_t fwmark, uint32_t table, const char *iif, const char *oif, RoutingPolicyRule **ret); int routing_policy_rule_add_foreign(Manager *m, int family, const union in_addr_union *from, uint8_t from_prefixlen, const union in_addr_union *to, uint8_t to_prefixlen, - uint8_t tos, uint32_t fwmark, uint32_t table, char *iif, char *oif, RoutingPolicyRule **ret); + uint8_t tos, uint32_t fwmark, uint32_t table, const char *iif, const char *oif, RoutingPolicyRule **ret); int routing_policy_rule_get(Manager *m, int family, const union in_addr_union *from, uint8_t from_prefixlen, const union in_addr_union *to, uint8_t to_prefixlen, uint8_t tos, - uint32_t fwmark, uint32_t table, char *iif, char *oif, RoutingPolicyRule **ret); + uint32_t fwmark, uint32_t table, const char *iif, const char *oif, RoutingPolicyRule **ret); int routing_policy_rule_make_local(Manager *m, RoutingPolicyRule *rule); int routing_policy_serialize_rules(Set *rules, FILE *f); int routing_policy_load_rules(const char *state_file, Set **rules); From 598c85cc46d07ac65df53e41a39c8da7b57f21da Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Sep 2018 17:39:16 +0900 Subject: [PATCH 05/12] network: fix alignment --- src/network/networkd-network-gperf.gperf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/networkd-network-gperf.gperf b/src/network/networkd-network-gperf.gperf index a7c94d1dc4..ac924596f3 100644 --- a/src/network/networkd-network-gperf.gperf +++ b/src/network/networkd-network-gperf.gperf @@ -138,7 +138,7 @@ DHCP.UseTimezone, config_parse_bool, DHCP.IAID, config_parse_iaid, 0, offsetof(Network, iaid) DHCP.ListenPort, config_parse_uint16, 0, offsetof(Network, dhcp_client_port) DHCP.RapidCommit, config_parse_bool, 0, offsetof(Network, rapid_commit) -DHCP.ForceDHCPv6PDOtherInformation, config_parse_bool, 0, offsetof(Network, dhcp6_force_pd_other_information) +DHCP.ForceDHCPv6PDOtherInformation, config_parse_bool, 0, offsetof(Network, dhcp6_force_pd_other_information) IPv6AcceptRA.UseDNS, config_parse_bool, 0, offsetof(Network, ipv6_accept_ra_use_dns) IPv6AcceptRA.UseDomains, config_parse_dhcp_use_domains, 0, offsetof(Network, ipv6_accept_ra_use_domains) IPv6AcceptRA.RouteTable, config_parse_uint32, 0, offsetof(Network, ipv6_accept_ra_route_table) From 727ba17f551cff98356c304cbacd83792783c2e7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Sep 2018 17:48:11 +0900 Subject: [PATCH 06/12] network: fix memleak around Network.dhcp_vendor_class_identifier --- src/network/networkd-network.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 107762d519..8d7795d72c 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -80,8 +80,7 @@ void network_apply_anonymize_if_set(Network *network) { network->dhcp_client_identifier = DHCP_CLIENT_ID_MAC; /* RFC 7844 3.10: SHOULD NOT use the Vendor Class Identifier option */ - /* NOTE: it was not initiallized to any value in network_load_one. */ - network->dhcp_vendor_class_identifier = false; + network->dhcp_vendor_class_identifier = mfree(network->dhcp_vendor_class_identifier); /* RFC7844 section 3.6.: The client intending to protect its privacy SHOULD only request a minimal number of options in the PRL and SHOULD also randomly shuffle @@ -201,10 +200,6 @@ static int network_load_one(Manager *manager, const char *filename) { network->dhcp_client_identifier = DHCP_CLIENT_ID_DUID; network->dhcp_route_table = RT_TABLE_MAIN; network->dhcp_route_table_set = false; - /* NOTE: the following vars were not set to any default, - * even if they are commented in the man? - * These vars might be overwriten by network_apply_anonymize_if_set */ - network->dhcp_vendor_class_identifier = false; /* NOTE: from man: UseMTU=... Defaults to false*/ network->dhcp_use_mtu = false; /* NOTE: from man: UseTimezone=... Defaults to "no".*/ From 09348d40817f5c8a2508c219c6479fef99800e82 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Sep 2018 18:04:08 +0900 Subject: [PATCH 07/12] sd-dhecp-lease: use free_and_replace() at one more --- src/libsystemd-network/sd-dhcp-lease.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-lease.c b/src/libsystemd-network/sd-dhcp-lease.c index a65e6166fb..45eab1d41c 100644 --- a/src/libsystemd-network/sd-dhcp-lease.c +++ b/src/libsystemd-network/sd-dhcp-lease.c @@ -330,8 +330,7 @@ static int lease_parse_string(const uint8_t *option, size_t len, char **ret) { if (!string) return -ENOMEM; - free(*ret); - *ret = string; + free_and_replace(*ret, string); } return 0; From e2975f854831d08a25b4f5eb329b6d04102e115f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Sep 2018 18:04:59 +0900 Subject: [PATCH 08/12] sd-dhcp-lease: fix memleaks --- src/libsystemd-network/sd-dhcp-lease.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libsystemd-network/sd-dhcp-lease.c b/src/libsystemd-network/sd-dhcp-lease.c index 45eab1d41c..863818d6da 100644 --- a/src/libsystemd-network/sd-dhcp-lease.c +++ b/src/libsystemd-network/sd-dhcp-lease.c @@ -258,6 +258,8 @@ static sd_dhcp_lease *dhcp_lease_free(sd_dhcp_lease *lease) { free(option); } + free(lease->root_path); + free(lease->timezone); free(lease->hostname); free(lease->domainname); free(lease->dns); From 23976cb92867007a1b9f9d52b9efd3f904472ed0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Sep 2018 23:31:34 +0900 Subject: [PATCH 09/12] dhcp6: use offsetof instead of sizeof --- src/libsystemd-network/sd-dhcp6-client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c index 74bf38bfb3..6e7ccce6dc 100644 --- a/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/libsystemd-network/sd-dhcp6-client.c @@ -1001,7 +1001,7 @@ static int client_parse_message( break; } - pos += sizeof(*option) + optlen; + pos += offsetof(DHCP6Option, data) + optlen; } if (!clientid) { From cb1bdeaf56852275e6b0dd1fba932bb174767f70 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Sep 2018 23:48:51 +0900 Subject: [PATCH 10/12] dhcp6: fix buffer size checking --- src/libsystemd-network/sd-dhcp6-client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c index 6e7ccce6dc..78994364cb 100644 --- a/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/libsystemd-network/sd-dhcp6-client.c @@ -844,8 +844,8 @@ static int client_parse_message( uint8_t *optval; be32_t iaid_lease; - if (len < offsetof(DHCP6Option, data) || - len < offsetof(DHCP6Option, data) + be16toh(option->len)) + if (len < pos + offsetof(DHCP6Option, data) || + len < pos + offsetof(DHCP6Option, data) + be16toh(option->len)) return -ENOBUFS; optcode = be16toh(option->code); From 431a4bc8c0e707ecf8efdd76266dc8357949b469 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 28 Sep 2018 00:10:26 +0900 Subject: [PATCH 11/12] dhcp6: fix memleak Let's drop client_set_lease() because just writing what it does in both places is simpler. Fixes #10157. --- src/libsystemd-network/sd-dhcp6-client.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c index 78994364cb..b793fc0285 100644 --- a/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/libsystemd-network/sd-dhcp6-client.c @@ -387,17 +387,10 @@ static void client_notify(sd_dhcp6_client *client, int event) { client->callback(client, event, client->userdata); } -static void client_set_lease(sd_dhcp6_client *client, sd_dhcp6_lease *lease) { - assert(client); - - (void) sd_dhcp6_lease_unref(client->lease); - client->lease = sd_dhcp6_lease_ref(lease); -} - static int client_reset(sd_dhcp6_client *client) { assert(client); - client_set_lease(client, NULL); + client->lease = sd_dhcp6_lease_unref(client->lease); client->receive_message = sd_event_source_unref(client->receive_message); @@ -1061,8 +1054,8 @@ static int client_receive_reply(sd_dhcp6_client *client, DHCP6Message *reply, si return 0; } - client_set_lease(client, lease); - lease = NULL; + sd_dhcp6_lease_unref(client->lease); + client->lease = TAKE_PTR(lease); return DHCP6_STATE_BOUND; } @@ -1090,8 +1083,8 @@ static int client_receive_advertise(sd_dhcp6_client *client, DHCP6Message *adver r = dhcp6_lease_get_preference(client->lease, &pref_lease); if (r < 0 || pref_advertise > pref_lease) { - client_set_lease(client, lease); - lease = NULL; + sd_dhcp6_lease_unref(client->lease); + client->lease = TAKE_PTR(lease); r = 0; } From b87dadcdb2badcd3f6d570915c09242fc796edfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 27 Sep 2018 17:50:25 +0200 Subject: [PATCH 12/12] networkd-routing-policy-rule.c: drop unnecessary check set_get() already does that. --- src/network/networkd-routing-policy-rule.c | 24 +++++++++------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 51b817e692..bbab393546 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -177,22 +177,18 @@ int routing_policy_rule_get(Manager *m, .oif = (char*) oif }; - if (m->rules) { - existing = set_get(m->rules, &rule); - if (existing) { - if (ret) - *ret = existing; - return 1; - } + existing = set_get(m->rules, &rule); + if (existing) { + if (ret) + *ret = existing; + return 1; } - if (m->rules_foreign) { - existing = set_get(m->rules_foreign, &rule); - if (existing) { - if (ret) - *ret = existing; - return 1; - } + existing = set_get(m->rules_foreign, &rule); + if (existing) { + if (ret) + *ret = existing; + return 1; } return -ENOENT;