Skip to content

Commit

Permalink
pinctrl: Cap the max size of a prefix delegation DUID value.
Browse files Browse the repository at this point in the history
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: faa44a0 ("controller: IPv6 Prefix-Delegation: introduce RENEW/REBIND msg support")
Signed-off-by: Ales Musil <amusil@redhat.com>
Co-authored-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
(cherry picked from commit 602d8ba)
  • Loading branch information
dceara committed Jul 14, 2023
1 parent 725292f commit e0ab37d
Showing 1 changed file with 16 additions and 17 deletions.
33 changes: 16 additions & 17 deletions controller/pinctrl.c
Expand Up @@ -622,6 +622,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;
Expand All @@ -631,7 +639,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;
Expand Down Expand Up @@ -847,7 +855,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;

Expand All @@ -856,7 +864,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;
Expand All @@ -879,8 +887,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;
Expand Down Expand Up @@ -939,8 +948,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:
Expand All @@ -960,8 +968,6 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in,
pinctrl_prefixd_state_handler(ip_flow, ipv6, aid, eth->eth_src,
in_ip->ip6_src, prefix_len, t1, t2,
plife_time, vlife_time, uuid, uuid_len);
} else if (uuid) {
free(uuid);
}
}

Expand Down Expand Up @@ -1155,10 +1161,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 &&
Expand Down Expand Up @@ -1353,12 +1356,8 @@ prepare_ipv6_prefixd(struct ovsdb_idl_txn *ovnsb_idl_txn,
SHASH_FOR_EACH_SAFE (iter, next, &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);
}
}

Expand Down

0 comments on commit e0ab37d

Please sign in to comment.