> 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: Dumitru Ceara <[email protected]>
> ---
> controller/pinctrl.c | 33 ++++++++++++++++-----------------
> 1 file changed, 16 insertions(+), 17 deletions(-)
thx for fixing it.
Acked-by: Lorenzo Bianconi <[email protected]>
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 6027ba0afb..bed90fe0b7 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);
> }
> }
>
> --
> 2.31.1
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
