From 602d8ba5bcfdd2c0408eb3cff62e4dc7dc4b95b8 Mon Sep 17 00:00:00 2001 From: Dumitru Ceara Date: Fri, 14 Jul 2023 14:39:07 +0200 Subject: [PATCH] pinctrl: Cap the max size of a prefix delegation DUID value. It's specified in RFC 8415. This also avoids having to free/realloc the pfd->uuid.data memory. That part was not correct anyway and was flagged by ASAN as a memleak: Direct leak of 42 byte(s) in 3 object(s) allocated from: #0 0x55e5b6354c9e in malloc (/workspace/ovn-tmp/controller/ovn-controller+0x2edc9e) (BuildId: f963f8c756bd5a2207a9b3c922d4362e46bb3162) #1 0x55e5b671878d in xmalloc__ /workspace/ovn-tmp/ovs/lib/util.c:140:15 #2 0x55e5b671878d in xmalloc /workspace/ovn-tmp/ovs/lib/util.c:175:12 #3 0x55e5b642cebc in pinctrl_parse_dhcpv6_reply /workspace/ovn-tmp/controller/pinctrl.c:997:20 #4 0x55e5b642cebc in pinctrl_handle_dhcp6_server /workspace/ovn-tmp/controller/pinctrl.c:1040:9 #5 0x55e5b642cebc in process_packet_in /workspace/ovn-tmp/controller/pinctrl.c:3210:9 #6 0x55e5b642cebc in pinctrl_recv /workspace/ovn-tmp/controller/pinctrl.c:3290:9 #7 0x55e5b642cebc in pinctrl_handler /workspace/ovn-tmp/controller/pinctrl.c:3385:17 #8 0x55e5b66ef664 in ovsthread_wrapper /workspace/ovn-tmp/ovs/lib/ovs-thread.c:423:12 #9 0x7faa30194b42 (/lib/x86_64-linux-gnu/libc.so.6+0x94b42) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d) Fixes: faa44a0c60a3 ("controller: IPv6 Prefix-Delegation: introduce RENEW/REBIND msg support") Signed-off-by: Ales Musil Co-authored-by: Ales Musil Signed-off-by: Dumitru Ceara Acked-by: Lorenzo Bianconi --- controller/pinctrl.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 6027ba0afb6..bed90fe0b7d 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -674,6 +674,14 @@ enum { PREFIX_REBIND, }; +/* According to RFC 8415, section 11: + * A DUID consists of a 2-octet type code represented in network byte + * order, followed by a variable number of octets that make up the + * actual identifier. The length of the DUID (not including the type + * code) is at least 1 octet and at most 128 octets. +*/ +#define DHCPV6_MAX_DUID_LEN 130 + struct ipv6_prefixd_state { long long int next_announce; long long int last_complete; @@ -683,7 +691,7 @@ struct ipv6_prefixd_state { struct eth_addr sa; /* server_id_info */ struct { - uint8_t *data; + uint8_t data[DHCPV6_MAX_DUID_LEN]; uint8_t len; } uuid; struct in6_addr ipv6_addr; @@ -899,7 +907,7 @@ pinctrl_prefixd_state_handler(const struct flow *ip_flow, struct eth_addr sa, struct in6_addr server_addr, char prefix_len, unsigned t1, unsigned t2, unsigned plife_time, unsigned vlife_time, - uint8_t *uuid, uint8_t uuid_len) + const uint8_t *uuid, uint8_t uuid_len) { struct ipv6_prefixd_state *pfd; @@ -908,7 +916,7 @@ pinctrl_prefixd_state_handler(const struct flow *ip_flow, pfd->state = PREFIX_PENDING; pfd->server_addr = server_addr; pfd->sa = sa; - pfd->uuid.data = uuid; + memcpy(pfd->uuid.data, uuid, uuid_len); pfd->uuid.len = uuid_len; pfd->plife_time = plife_time * 1000; pfd->vlife_time = vlife_time * 1000; @@ -933,8 +941,9 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in, unsigned char *in_dhcpv6_data = (unsigned char *)(udp_in + 1); size_t dlen = MIN(ntohs(udp_in->udp_len), dp_packet_l4_size(pkt_in)); unsigned t1 = 0, t2 = 0, vlife_time = 0, plife_time = 0; - uint8_t *end = (uint8_t *)udp_in + dlen, *uuid = NULL; + uint8_t *end = (uint8_t *) udp_in + dlen; uint8_t prefix_len = 0, uuid_len = 0; + uint8_t uuid[DHCPV6_MAX_DUID_LEN]; struct in6_addr ipv6 = in6addr_any; bool status = false; unsigned aid = 0; @@ -993,8 +1002,7 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in, break; } case DHCPV6_OPT_SERVER_ID_CODE: - uuid_len = ntohs(in_opt->len); - uuid = xmalloc(uuid_len); + uuid_len = MIN(ntohs(in_opt->len), DHCPV6_MAX_DUID_LEN); memcpy(uuid, in_opt + 1, uuid_len); break; default: @@ -1014,8 +1022,6 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in, pinctrl_prefixd_state_handler(ip_flow, ipv6, aid, eth->eth_src, ip6_src, prefix_len, t1, t2, plife_time, vlife_time, uuid, uuid_len); - } else if (uuid) { - free(uuid); } } @@ -1212,10 +1218,7 @@ static bool ipv6_prefixd_should_inject(void) if (pfd->state == PREFIX_RENEW && cur_time > pfd->last_complete + pfd->t2) { pfd->state = PREFIX_REBIND; - if (pfd->uuid.len) { - free(pfd->uuid.data); - pfd->uuid.len = 0; - } + pfd->uuid.len = 0; return true; } if (pfd->state == PREFIX_REBIND && @@ -1409,12 +1412,8 @@ prepare_ipv6_prefixd(struct ovsdb_idl_txn *ovnsb_idl_txn, SHASH_FOR_EACH_SAFE (iter, &ipv6_prefixd) { struct ipv6_prefixd_state *pfd = iter->data; if (pfd->last_used + IPV6_PREFIXD_STALE_TIMEOUT < time_msec()) { - if (pfd->uuid.len) { - free(pfd->uuid.data); - pfd->uuid.len = 0; - } - free(pfd); shash_delete(&ipv6_prefixd, iter); + free(pfd); } }