Merge pull request #5696 from qlyoung/stable-7.2-backport-more

More 7.2 bugfix backports

18 files changed, 276 insertions, 250 deletions

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 53e6acc424..2e83f7d051 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -422,14 +422,15 @@ static struct transit *transit_intern(struct transit *transit)
 	return find;
 }
 
-void transit_unintern(struct transit *transit)
+static void transit_unintern(struct transit **transit)
 {
-	if (transit->refcnt)
-		transit->refcnt--;
+	if ((*transit)->refcnt)
+		(*transit)->refcnt--;
 
-	if (transit->refcnt == 0) {
-		hash_release(transit_hash, transit);
-		transit_free(transit);
+	if ((*transit)->refcnt == 0) {
+		hash_release(transit_hash, *transit);
+		transit_free(*transit);
+		*transit = NULL;
 	}
 }
 
@@ -465,15 +466,6 @@ static void transit_finish(void)
 /* Attribute hash routines. */
 static struct hash *attrhash;
 
-/* Shallow copy of an attribute
- * Though, not so shallow that it doesn't copy the contents
- * of the attr_extra pointed to by 'extra'
- */
-void bgp_attr_dup(struct attr *new, struct attr *orig)
-{
-	*new = *orig;
-}
-
 unsigned long int attr_count(void)
 {
 	return attrhash->count;
@@ -859,7 +851,7 @@ void bgp_attr_unintern_sub(struct attr *attr)
 	UNSET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_CLUSTER_LIST));
 
 	if (attr->transit)
-		transit_unintern(attr->transit);
+		transit_unintern(&attr->transit);
 
 	if (attr->encap_subtlvs)
 		encap_unintern(&attr->encap_subtlvs, ENCAP_SUBTLV_TYPE);
@@ -2475,7 +2467,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
 
 			bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
 					BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
-			return BGP_ATTR_PARSE_ERROR;
+			ret = BGP_ATTR_PARSE_ERROR;
+			goto done;
 		}
 
 		/* Fetch attribute flag and type. */
@@ -2498,7 +2491,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
 
 			bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
 					BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
-			return BGP_ATTR_PARSE_ERROR;
+			ret = BGP_ATTR_PARSE_ERROR;
+			goto done;
 		}
 
 		/* Check extended attribue length bit. */
@@ -2519,7 +2513,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
 
 			bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
 					BGP_NOTIFY_UPDATE_MAL_ATTR);
-			return BGP_ATTR_PARSE_ERROR;
+			ret = BGP_ATTR_PARSE_ERROR;
+			goto done;
 		}
 
 		/* Set type to bitmap to check duplicate attribute.  `type' is
@@ -2576,7 +2571,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
 				BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, ndata,
 				ndl + lfl + 1);
 
-			return BGP_ATTR_PARSE_ERROR;
+			ret = BGP_ATTR_PARSE_ERROR;
+			goto done;
 		}
 
 		struct bgp_attr_parser_args attr_args = {
@@ -2601,7 +2597,7 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
 				attr_args.total);
 			if (ret == BGP_ATTR_PARSE_PROCEED)
 				continue;
-			return ret;
+			goto done;
 		}
 
 		/* OK check attribute and store it's value. */
@@ -2679,32 +2675,25 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
 			bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
 					BGP_NOTIFY_UPDATE_MAL_ATTR);
 			ret = BGP_ATTR_PARSE_ERROR;
+			goto done;
 		}
 
 		if (ret == BGP_ATTR_PARSE_EOR) {
-			if (as4_path)
-				aspath_unintern(&as4_path);
-			return ret;
+			goto done;
 		}
 
-		/* If hard error occurred immediately return to the caller. */
 		if (ret == BGP_ATTR_PARSE_ERROR) {
 			flog_warn(EC_BGP_ATTRIBUTE_PARSE_ERROR,
 				  "%s: Attribute %s, parse error", peer->host,
 				  lookup_msg(attr_str, type, NULL));
-			if (as4_path)
-				aspath_unintern(&as4_path);
-			return ret;
+			goto done;
 		}
 		if (ret == BGP_ATTR_PARSE_WITHDRAW) {
-
 			flog_warn(
 				EC_BGP_ATTRIBUTE_PARSE_WITHDRAW,
 				"%s: Attribute %s, parse error - treating as withdrawal",
 				peer->host, lookup_msg(attr_str, type, NULL));
-			if (as4_path)
-				aspath_unintern(&as4_path);
-			return ret;
+			goto done;
 		}
 
 		/* Check the fetched length. */
@@ -2714,9 +2703,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
 				  peer->host, lookup_msg(attr_str, type, NULL));
 			bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
 					BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
-			if (as4_path)
-				aspath_unintern(&as4_path);
-			return BGP_ATTR_PARSE_ERROR;
+			ret = BGP_ATTR_PARSE_ERROR;
+			goto done;
 		}
 	}
 
@@ -2727,9 +2715,9 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
 			  lookup_msg(attr_str, type, NULL));
 		bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
 				BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
-		if (as4_path)
-			aspath_unintern(&as4_path);
-		return BGP_ATTR_PARSE_ERROR;
+
+		ret = BGP_ATTR_PARSE_ERROR;
+		goto done;
 	}
 
 	/*
@@ -2748,16 +2736,14 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
 	if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP))
 	    && !CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_MP_REACH_NLRI))) {
 		if (bgp_attr_nexthop_valid(peer, attr) < 0) {
-			return BGP_ATTR_PARSE_ERROR;
+			ret = BGP_ATTR_PARSE_ERROR;
+			goto done;
 		}
 	}
 
 	/* Check all mandatory well-known attributes are present */
-	if ((ret = bgp_attr_check(peer, attr)) < 0) {
-		if (as4_path)
-			aspath_unintern(&as4_path);
-		return ret;
-	}
+	if ((ret = bgp_attr_check(peer, attr)) < 0)
+		goto done;
 
 	/*
 	 * At this place we can see whether we got AS4_PATH and/or
@@ -2780,28 +2766,10 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
 					&as4_aggregator_addr)) {
 		bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
 				BGP_NOTIFY_UPDATE_MAL_ATTR);
-		if (as4_path)
-			aspath_unintern(&as4_path);
-		return BGP_ATTR_PARSE_ERROR;
+		ret = BGP_ATTR_PARSE_ERROR;
+		goto done;
 	}
 
-	/* At this stage, we have done all fiddling with as4, and the
-	 * resulting info is in attr->aggregator resp. attr->aspath
-	 * so we can chuck as4_aggregator and as4_path alltogether in
-	 * order to save memory
-	 */
-	if (as4_path) {
-		aspath_unintern(&as4_path); /* unintern - it is in the hash */
-		/* The flag that we got this is still there, but that does not
-		 * do any trouble
-		 */
-	}
-	/*
-	 * The "rest" of the code does nothing with as4_aggregator.
-	 * there is no memory attached specifically which is not part
-	 * of the attr.
-	 * so ignoring just means do nothing.
-	 */
 	/*
 	 * Finally do the checks on the aspath we did not do yet
 	 * because we waited for a potentially synthesized aspath.
@@ -2809,21 +2777,59 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
 	if (attr->flag & (ATTR_FLAG_BIT(BGP_ATTR_AS_PATH))) {
 		ret = bgp_attr_aspath_check(peer, attr);
 		if (ret != BGP_ATTR_PARSE_PROCEED)
-			return ret;
+			goto done;
+	}
+
+	ret = BGP_ATTR_PARSE_PROCEED;
+done:
+
+	/*
+	 * At this stage, we have done all fiddling with as4, and the
+	 * resulting info is in attr->aggregator resp. attr->aspath so
+	 * we can chuck as4_aggregator and as4_path alltogether in order
+	 * to save memory
+	 */
+	if (as4_path) {
+		/*
+		 * unintern - it is in the hash
+		 * The flag that we got this is still there, but that
+		 * does not do any trouble
+		 */
+		aspath_unintern(&as4_path);
 	}
-	/* Finally intern unknown attribute. */
+
+	if (ret != BGP_ATTR_PARSE_ERROR) {
+		/* Finally intern unknown attribute. */
+		if (attr->transit)
+			attr->transit = transit_intern(attr->transit);
+		if (attr->encap_subtlvs)
+			attr->encap_subtlvs = encap_intern(attr->encap_subtlvs,
+							   ENCAP_SUBTLV_TYPE);
+#if ENABLE_BGP_VNC
+		if (attr->vnc_subtlvs)
+			attr->vnc_subtlvs = encap_intern(attr->vnc_subtlvs,
+							 VNC_SUBTLV_TYPE);
+#endif
+	} else {
+		if (attr->transit) {
+			transit_free(attr->transit);
+			attr->transit = NULL;
+		}
+
+		bgp_attr_flush_encap(attr);
+	};
+
+	/* Sanity checks */
 	if (attr->transit)
-		attr->transit = transit_intern(attr->transit);
+		assert(attr->transit->refcnt > 0);
 	if (attr->encap_subtlvs)
-		attr->encap_subtlvs =
-			encap_intern(attr->encap_subtlvs, ENCAP_SUBTLV_TYPE);
+		assert(attr->encap_subtlvs->refcnt > 0);
 #if ENABLE_BGP_VNC
 	if (attr->vnc_subtlvs)
-		attr->vnc_subtlvs =
-			encap_intern(attr->vnc_subtlvs, VNC_SUBTLV_TYPE);
+		assert(attr->vnc_subtlvs->refcnt > 0);
 #endif
 
-	return BGP_ATTR_PARSE_PROCEED;
+	return ret;
 }
 
 /*
diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
index f1a871fe43..2dda2e37d7 100644
--- a/bgpd/bgp_attr.h
+++ b/bgpd/bgp_attr.h
@@ -260,7 +260,6 @@ extern void bgp_attr_finish(void);
 extern bgp_attr_parse_ret_t bgp_attr_parse(struct peer *, struct attr *,
 					   bgp_size_t, struct bgp_nlri *,
 					   struct bgp_nlri *);
-extern void bgp_attr_dup(struct attr *, struct attr *);
 extern void bgp_attr_undup(struct attr *new, struct attr *old);
 extern struct attr *bgp_attr_intern(struct attr *attr);
 extern void bgp_attr_unintern_sub(struct attr *);
@@ -293,9 +292,6 @@ extern unsigned long int attr_unknown_count(void);
 extern int cluster_loop_check(struct cluster_list *, struct in_addr);
 extern void cluster_unintern(struct cluster_list *);
 
-/* Transit attribute prototypes. */
-void transit_unintern(struct transit *);
-
 /* Below exported for unit-test purposes only */
 struct bgp_attr_parser_args {
 	struct peer *peer;
diff --git a/bgpd/bgp_ecommunity.c b/bgpd/bgp_ecommunity.c
index 850b85aa6a..b75676e86e 100644
--- a/bgpd/bgp_ecommunity.c
+++ b/bgpd/bgp_ecommunity.c
@@ -77,37 +77,36 @@ static void ecommunity_hash_free(struct ecommunity *ecom)
    else return 0.  */
 int ecommunity_add_val(struct ecommunity *ecom, struct ecommunity_val *eval)
 {
-	uint8_t *p;
-	int ret;
 	int c;
 
-	/* When this is fist value, just add it.  */
+	/* When this is fist value, just add it. */
 	if (ecom->val == NULL) {
-		ecom->size++;
-		ecom->val = XMALLOC(MTYPE_ECOMMUNITY_VAL, ecom_length(ecom));
+		ecom->size = 1;
+		ecom->val = XCALLOC(MTYPE_ECOMMUNITY_VAL, ECOMMUNITY_SIZE);
 		memcpy(ecom->val, eval->val, ECOMMUNITY_SIZE);
 		return 1;
 	}
 
 	/* If the value already exists in the structure return 0.  */
 	c = 0;
-	for (p = ecom->val; c < ecom->size; p += ECOMMUNITY_SIZE, c++) {
-		ret = memcmp(p, eval->val, ECOMMUNITY_SIZE);
+	for (uint8_t *p = ecom->val; c < ecom->size;
+	     p += ECOMMUNITY_SIZE, c++) {
+		int ret = memcmp(p, eval->val, ECOMMUNITY_SIZE);
 		if (ret == 0)
 			return 0;
-		if (ret > 0)
+		else if (ret > 0)
 			break;
 	}
 
 	/* Add the value to the structure with numerical sorting.  */
 	ecom->size++;
-	ecom->val =
-		XREALLOC(MTYPE_ECOMMUNITY_VAL, ecom->val, ecom_length(ecom));
+	ecom->val = XREALLOC(MTYPE_ECOMMUNITY_VAL, ecom->val,
+			     ecom->size * ECOMMUNITY_SIZE);
 
-	memmove(ecom->val + (c + 1) * ECOMMUNITY_SIZE,
-		ecom->val + c * ECOMMUNITY_SIZE,
+	memmove(ecom->val + ((c + 1) * ECOMMUNITY_SIZE),
+		ecom->val + (c * ECOMMUNITY_SIZE),
 		(ecom->size - 1 - c) * ECOMMUNITY_SIZE);
-	memcpy(ecom->val + c * ECOMMUNITY_SIZE, eval->val, ECOMMUNITY_SIZE);
+	memcpy(ecom->val + (c * ECOMMUNITY_SIZE), eval->val, ECOMMUNITY_SIZE);
 
 	return 1;
 }
@@ -556,8 +555,8 @@ struct ecommunity *ecommunity_str2com(const char *str, int type,
 	return ecom;
 }
 
-static int ecommunity_rt_soo_str(char *buf, uint8_t *pnt, int type,
-				 int sub_type, int format)
+static int ecommunity_rt_soo_str(char *buf, size_t bufsz, uint8_t *pnt,
+				 int type, int sub_type, int format)
 {
 	int len = 0;
 	const char *prefix;
@@ -589,23 +588,25 @@ static int ecommunity_rt_soo_str(char *buf, uint8_t *pnt, int type,
 		eas.val = (*pnt++ << 8);
 		eas.val |= (*pnt++);
 
-		len = sprintf(buf, "%s%u:%u", prefix, eas.as, eas.val);
+		len = snprintf(buf, bufsz, "%s%u:%u", prefix, eas.as, eas.val);
 	} else if (type == ECOMMUNITY_ENCODE_AS) {
 		eas.as = (*pnt++ << 8);
 		eas.as |= (*pnt++);
 		pnt = ptr_get_be32(pnt, &eas.val);
 
-		len = sprintf(buf, "%s%u:%u", prefix, eas.as, eas.val);
+		len = snprintf(buf, bufsz, "%s%u:%u", prefix, eas.as, eas.val);
 	} else if (type == ECOMMUNITY_ENCODE_IP) {
 		memcpy(&eip.ip, pnt, 4);
 		pnt += 4;
 		eip.val = (*pnt++ << 8);
 		eip.val |= (*pnt++);
 
-		len = sprintf(buf, "%s%s:%u", prefix, inet_ntoa(eip.ip),
-			      eip.val);
+		len = snprintf(buf, bufsz, "%s%s:%u", prefix, inet_ntoa(eip.ip),
+			       eip.val);
 	}
-	(void)pnt; /* consume value */
+
+	/* consume value */
+	(void)pnt;
 
 	return len;
 }
@@ -640,44 +641,31 @@ char *ecommunity_ecom2str(struct ecommunity *ecom, int format, int filter)
 	uint8_t *pnt;
 	uint8_t type = 0;
 	uint8_t sub_type = 0;
-#define ECOMMUNITY_STR_DEFAULT_LEN  64
+#define ECOMMUNITY_STRLEN 64
 	int str_size;
-	int str_pnt;
 	char *str_buf;
-	int len = 0;
-	int first = 1;
 
-	if (ecom->size == 0) {
-		str_buf = XMALLOC(MTYPE_ECOMMUNITY_STR, 1);
-		str_buf[0] = '\0';
-		return str_buf;
-	}
+	if (ecom->size == 0)
+		return XCALLOC(MTYPE_ECOMMUNITY_STR, 1);
 
-	/* Prepare buffer.  */
-	str_buf = XMALLOC(MTYPE_ECOMMUNITY_STR, ECOMMUNITY_STR_DEFAULT_LEN + 1);
-	str_size = ECOMMUNITY_STR_DEFAULT_LEN + 1;
-	str_buf[0] = '\0';
-	str_pnt = 0;
+	/* ecom strlen + space + null term */
+	str_size = (ecom->size * (ECOMMUNITY_STRLEN + 1)) + 1;
+	str_buf = XCALLOC(MTYPE_ECOMMUNITY_STR, str_size);
+
+	char encbuf[128];
 
 	for (i = 0; i < ecom->size; i++) {
 		int unk_ecom = 0;
-
-		/* Make it sure size is enough.  */
-		while (str_pnt + ECOMMUNITY_STR_DEFAULT_LEN >= str_size) {
-			str_size *= 2;
-			str_buf = XREALLOC(MTYPE_ECOMMUNITY_STR, str_buf,
-					   str_size);
-		}
+		memset(encbuf, 0x00, sizeof(encbuf));
 
 		/* Space between each value.  */
-		if (!first) {
-			str_buf[str_pnt++] = ' ';
-			len++;
-		}
+		if (i > 0)
+			strlcat(str_buf, " ", str_size);
 
+		/* Retrieve value field */
 		pnt = ecom->val + (i * 8);
 
-		/* High-order octet of type. */
+		/* High-order octet is the type */
 		type = *pnt++;
 
 		if (type == ECOMMUNITY_ENCODE_AS || type == ECOMMUNITY_ENCODE_IP
@@ -696,15 +684,15 @@ char *ecommunity_ecom2str(struct ecommunity *ecom, int format, int filter)
 					inet_ntop(AF_INET, ipv4,
 						  ipv4str,
 						  INET_ADDRSTRLEN);
-					len = sprintf(str_buf + str_pnt,
-						      "NH:%s:%d",
-						      ipv4str, pnt[5]);
+					snprintf(encbuf, sizeof(encbuf),
+						 "NH:%s:%d", ipv4str, pnt[5]);
 				} else
 					unk_ecom = 1;
-			} else
-				len = ecommunity_rt_soo_str(str_buf + str_pnt,
-							    pnt, type, sub_type,
-							    format);
+			} else {
+				ecommunity_rt_soo_str(encbuf, sizeof(encbuf),
+						      pnt, type, sub_type,
+						      format);
+			}
 		} else if (type == ECOMMUNITY_ENCODE_OPAQUE) {
 			if (filter == ECOMMUNITY_ROUTE_TARGET)
 				continue;
@@ -712,13 +700,15 @@ char *ecommunity_ecom2str(struct ecommunity *ecom, int format, int filter)
 				uint16_t tunneltype;
 				memcpy(&tunneltype, pnt + 5, 2);
 				tunneltype = ntohs(tunneltype);
-				len = sprintf(str_buf + str_pnt, "ET:%d",
-					      tunneltype);
+
+				snprintf(encbuf, sizeof(encbuf), "ET:%d",
+					 tunneltype);
 			} else if (*pnt == ECOMMUNITY_EVPN_SUBTYPE_DEF_GW) {
-				len = sprintf(str_buf + str_pnt,
-					      "Default Gateway");
-			} else
+				strlcpy(encbuf, "Default Gateway",
+					sizeof(encbuf));
+			} else {
 				unk_ecom = 1;
+			}
 		} else if (type == ECOMMUNITY_ENCODE_EVPN) {
 			if (filter == ECOMMUNITY_ROUTE_TARGET)
 				continue;
@@ -726,15 +716,15 @@ char *ecommunity_ecom2str(struct ecommunity *ecom, int format, int filter)
 				struct ethaddr rmac;
 				pnt++;
 				memcpy(&rmac, pnt, ETH_ALEN);
-				len = sprintf(
-					str_buf + str_pnt,
-					"Rmac:%02x:%02x:%02x:%02x:%02x:%02x",
-					(uint8_t)rmac.octet[0],
-					(uint8_t)rmac.octet[1],
-					(uint8_t)rmac.octet[2],
-					(uint8_t)rmac.octet[3],
-					(uint8_t)rmac.octet[4],
-					(uint8_t)rmac.octet[5]);
+
+				snprintf(encbuf, sizeof(encbuf),
+					 "Rmac:%02x:%02x:%02x:%02x:%02x:%02x",
+					 (uint8_t)rmac.octet[0],
+					 (uint8_t)rmac.octet[1],
+					 (uint8_t)rmac.octet[2],
+					 (uint8_t)rmac.octet[3],
+					 (uint8_t)rmac.octet[4],
+					 (uint8_t)rmac.octet[5]);
 			} else if (*pnt
 				   == ECOMMUNITY_EVPN_SUBTYPE_MACMOBILITY) {
 				uint32_t seqnum;
@@ -742,29 +732,30 @@ char *ecommunity_ecom2str(struct ecommunity *ecom, int format, int filter)
 
 				memcpy(&seqnum, pnt + 2, 4);
 				seqnum = ntohl(seqnum);
-				if (flags
-				    & ECOMMUNITY_EVPN_SUBTYPE_MACMOBILITY_FLAG_STICKY)
-					len = sprintf(str_buf + str_pnt,
-						      "MM:%u, sticky MAC",
-						      seqnum);
-				else
-					len = sprintf(str_buf + str_pnt,
-						      "MM:%u", seqnum);
+
+				snprintf(encbuf, sizeof(encbuf), "MM:%u",
+					 seqnum);
+
+				if (CHECK_FLAG(
+					    flags,
+					    ECOMMUNITY_EVPN_SUBTYPE_MACMOBILITY_FLAG_STICKY))
+					strlcat(encbuf, ", sticky MAC",
+						sizeof(encbuf));
 			} else if (*pnt == ECOMMUNITY_EVPN_SUBTYPE_ND) {
 				uint8_t flags = *++pnt;
 
-				if (flags
-				    & ECOMMUNITY_EVPN_SUBTYPE_ND_ROUTER_FLAG)
-					len = sprintf(str_buf + str_pnt,
-						      "ND:Router Flag");
+				if (CHECK_FLAG(
+					    flags,
+					    ECOMMUNITY_EVPN_SUBTYPE_ND_ROUTER_FLAG))
+					strlcpy(encbuf, "ND:Router Flag",
+						sizeof(encbuf));
 			} else
 				unk_ecom = 1;
 		} else if (type == ECOMMUNITY_ENCODE_REDIRECT_IP_NH) {
 			sub_type = *pnt++;
 			if (sub_type == ECOMMUNITY_REDIRECT_IP_NH) {
-				len = sprintf(
-					str_buf + str_pnt,
-					"FS:redirect IP 0x%x", *(pnt+5));
+				snprintf(encbuf, sizeof(encbuf),
+					 "FS:redirect IP 0x%x", *(pnt + 5));
 			} else
 				unk_ecom = 1;
 		} else if (type == ECOMMUNITY_ENCODE_TRANS_EXP ||
@@ -772,38 +763,35 @@ char *ecommunity_ecom2str(struct ecommunity *ecom, int format, int filter)
 			   type == ECOMMUNITY_EXTENDED_COMMUNITY_PART_3) {
 			sub_type = *pnt++;
 			if (sub_type == ECOMMUNITY_REDIRECT_VRF) {
-				char buf[16];
-
-				memset(buf, 0, sizeof(buf));
-				ecommunity_rt_soo_str(buf, (uint8_t *)pnt,
-						      type &
-						      ~ECOMMUNITY_ENCODE_TRANS_EXP,
-						      ECOMMUNITY_ROUTE_TARGET,
-						      ECOMMUNITY_FORMAT_DISPLAY);
-				len = snprintf(str_buf + str_pnt,
-					       str_size - len,
-					       "FS:redirect VRF %s", buf);
+				char buf[16] = {};
+				ecommunity_rt_soo_str(
+					buf, sizeof(buf), (uint8_t *)pnt,
+					type & ~ECOMMUNITY_ENCODE_TRANS_EXP,
+					ECOMMUNITY_ROUTE_TARGET,
+					ECOMMUNITY_FORMAT_DISPLAY);
+				snprintf(encbuf, sizeof(encbuf),
+					 "FS:redirect VRF %s", buf);
 			} else if (type != ECOMMUNITY_ENCODE_TRANS_EXP)
 				unk_ecom = 1;
 			else if (sub_type == ECOMMUNITY_TRAFFIC_ACTION) {
 				char action[64];
-				char *ptr = action;
 
 				if (*(pnt+3) ==
 				    1 << FLOWSPEC_TRAFFIC_ACTION_TERMINAL)
-					ptr += snprintf(ptr, sizeof(action),
-							"terminate (apply)");
+					strlcpy(action, "terminate (apply)",
+						sizeof(action));
 				else
-					ptr += snprintf(ptr, sizeof(action),
-						       "eval stops");
+					strlcpy(action, "eval stops",
+						sizeof(action));
+
 				if (*(pnt+3) ==
 				    1 << FLOWSPEC_TRAFFIC_ACTION_SAMPLE)
-					snprintf(ptr, sizeof(action) -
-						 (size_t)(ptr-action),
-						 ", sample");
-				len = snprintf(str_buf + str_pnt,
-					       str_size - len,
-					      "FS:action %s", action);
+					strlcat(action, ", sample",
+						sizeof(action));
+
+
+				snprintf(encbuf, sizeof(encbuf), "FS:action %s",
+					 action);
 			} else if (sub_type == ECOMMUNITY_TRAFFIC_RATE) {
 				union traffic_rate data;
 
@@ -811,21 +799,19 @@ char *ecommunity_ecom2str(struct ecommunity *ecom, int format, int filter)
 				data.rate_byte[2] = *(pnt+3);
 				data.rate_byte[1] = *(pnt+4);
 				data.rate_byte[0] = *(pnt+5);
-				len = sprintf(
-					str_buf + str_pnt,
-					"FS:rate %f", data.rate_float);
+				snprintf(encbuf, sizeof(encbuf), "FS:rate %f",
+					 data.rate_float);
 			} else if (sub_type == ECOMMUNITY_TRAFFIC_MARKING) {
-				len = sprintf(
-					str_buf + str_pnt,
-					"FS:marking %u", *(pnt+5));
+				snprintf(encbuf, sizeof(encbuf),
+					 "FS:marking %u", *(pnt + 5));
 			} else if (*pnt
 				   == ECOMMUNITY_EVPN_SUBTYPE_ES_IMPORT_RT) {
 				struct ethaddr mac;
 
-				pnt++;
 				memcpy(&mac, pnt, ETH_ALEN);
-				len = sprintf(
-					str_buf + str_pnt,
+
+				snprintf(
+					encbuf, sizeof(encbuf),
 					"ES-Import-Rt:%02x:%02x:%02x:%02x:%02x:%02x",
 					(uint8_t)mac.octet[0],
 					(uint8_t)mac.octet[1],
@@ -841,11 +827,11 @@ char *ecommunity_ecom2str(struct ecommunity *ecom, int format, int filter)
 		}
 
 		if (unk_ecom)
-			len = sprintf(str_buf + str_pnt, "UNK:%d, %d",
-				      type, sub_type);
+			snprintf(encbuf, sizeof(encbuf), "UNK:%d, %d", type,
+				 sub_type);
 
-		str_pnt += len;
-		first = 0;
+		int r = strlcat(str_buf, encbuf, str_size);
+		assert(r < str_size);
 	}
 
 	return str_buf;
diff --git a/bgpd/bgp_ecommunity.h b/bgpd/bgp_ecommunity.h
index 79be4ee422..a4c0f45c65 100644
--- a/bgpd/bgp_ecommunity.h
+++ b/bgpd/bgp_ecommunity.h
@@ -105,8 +105,6 @@ struct ecommunity_val {
 	char val[ECOMMUNITY_SIZE];
 };
 
-#define ecom_length(X)    ((X)->size * ECOMMUNITY_SIZE)
-
 /*
  * Encode BGP Route Target AS:nn.
  */
diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c
index 739f8e605f..e86379d1d4 100644
--- a/bgpd/bgp_evpn.c
+++ b/bgpd/bgp_evpn.c
@@ -1531,7 +1531,7 @@ static int update_evpn_type5_route(struct bgp *bgp_vrf, struct prefix_evpn *evp,
 	 * present, else treat as locally originated.
 	 */
 	if (src_attr)
-		bgp_attr_dup(&attr, src_attr);
+		attr = *src_attr;
 	else {
 		memset(&attr, 0, sizeof(struct attr));
 		bgp_attr_default_set(&attr, BGP_ORIGIN_IGP);
@@ -2497,7 +2497,7 @@ static int install_evpn_route_entry_in_vrf(struct bgp *bgp_vrf,
 	 * address for the rest of the code to flow through. In the case of IPv4,
 	 * make sure to set the flag for next hop attribute.
 	 */
-	bgp_attr_dup(&attr, parent_pi->attr);
+	attr = *parent_pi->attr;
 	if (afi == AFI_IP6)
 		evpn_convert_nexthop_to_ipv6(&attr);
 	else
diff --git a/bgpd/bgp_lcommunity.c b/bgpd/bgp_lcommunity.c
index aeb290719a..8d99548fc2 100644
--- a/bgpd/bgp_lcommunity.c
+++ b/bgpd/bgp_lcommunity.c
@@ -177,15 +177,14 @@ static void set_lcommunity_string(struct lcommunity *lcom, bool make_json)
 {
 	int i;
 	int len;
-	bool first = true;
 	char *str_buf;
-	char *str_pnt;
 	uint8_t *pnt;
 	uint32_t global, local1, local2;
 	json_object *json_lcommunity_list = NULL;
 	json_object *json_string = NULL;
 
-#define LCOMMUNITY_STR_DEFAULT_LEN 32
+	/* 3 32-bit integers, 2 colons, and a space */
+#define LCOMMUNITY_STRLEN (10 * 3 + 2 + 1)
 
 	if (!lcom)
 		return;
@@ -196,8 +195,7 @@ static void set_lcommunity_string(struct lcommunity *lcom, bool make_json)
 	}
 
 	if (lcom->size == 0) {
-		str_buf = XMALLOC(MTYPE_LCOMMUNITY_STR, 1);
-		str_buf[0] = '\0';
+		str_buf = XCALLOC(MTYPE_LCOMMUNITY_STR, 1);
 
 		if (make_json) {
 			json_object_string_add(lcom->json, "string", "");
@@ -209,15 +207,13 @@ static void set_lcommunity_string(struct lcommunity *lcom, bool make_json)
 		return;
 	}
 
-	str_buf = str_pnt =
-		XMALLOC(MTYPE_LCOMMUNITY_STR,
-			(LCOMMUNITY_STR_DEFAULT_LEN * lcom->size) + 1);
+	/* 1 space + lcom->size lcom strings + null terminator */
+	size_t str_buf_sz = (LCOMMUNITY_STRLEN * lcom->size) + 2;
+	str_buf = XCALLOC(MTYPE_LCOMMUNITY_STR, str_buf_sz);
 
 	for (i = 0; i < lcom->size; i++) {
-		if (first)
-			first = false;
-		else
-			*str_pnt++ = ' ';
+		if (i > 0)
+			strlcat(str_buf, " ", str_buf_sz);
 
 		pnt = lcom->val + (i * LCOMMUNITY_SIZE);
 		pnt = ptr_get_be32(pnt, &global);
@@ -225,19 +221,21 @@ static void set_lcommunity_string(struct lcommunity *lcom, bool make_json)
 		pnt = ptr_get_be32(pnt, &local2);
 		(void)pnt;
 
-		len = sprintf(str_pnt, "%u:%u:%u", global, local1, local2);
+		char lcsb[LCOMMUNITY_STRLEN + 1];
+
+		snprintf(lcsb, sizeof(lcsb), "%u:%u:%u", global, local1,
+			 local2);
+
+		len = strlcat(str_buf, lcsb, str_buf_sz);
+		assert((unsigned int)len < str_buf_sz);
+
 		if (make_json) {
-			json_string = json_object_new_string(str_pnt);
+			json_string = json_object_new_string(lcsb);
 			json_object_array_add(json_lcommunity_list,
 					      json_string);
 		}
-
-		str_pnt += len;
 	}
 
-	str_buf =
-		XREALLOC(MTYPE_LCOMMUNITY_STR, str_buf, str_pnt - str_buf + 1);
-
 	if (make_json) {
 		json_object_string_add(lcom->json, "string", str_buf);
 		json_object_object_add(lcom->json, "list",
diff --git a/bgpd/bgp_mpath.c b/bgpd/bgp_mpath.c
index 648c3be47e..90b37f0dd2 100644
--- a/bgpd/bgp_mpath.c
+++ b/bgpd/bgp_mpath.c
@@ -722,7 +722,7 @@ void bgp_path_info_mpath_aggregate_update(struct bgp_path_info *new_best,
 		return;
 	}
 
-	bgp_attr_dup(&attr, new_best->attr);
+	attr = *new_best->attr;
 
 	if (new_best->peer && bgp_flag_check(new_best->peer->bgp,
 					     BGP_FLAG_MULTIPATH_RELAX_AS_SET)) {
diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c
index 3ad41ca620..f99ffce1ad 100644
--- a/bgpd/bgp_mplsvpn.c
+++ b/bgpd/bgp_mplsvpn.c
@@ -700,7 +700,8 @@ void vpn_leak_from_vrf_update(struct bgp *bgp_vpn,	    /* to */
 		return;
 	}
 
-	bgp_attr_dup(&static_attr, path_vrf->attr); /* shallow copy */
+	/* shallow copy */
+	static_attr = *path_vrf->attr;
 
 	/*
 	 * route map handling
@@ -1081,7 +1082,8 @@ vpn_leak_to_vrf_update_onevrf(struct bgp *bgp_vrf,	    /* to */
 				buf_prefix, bgp_vrf->name_pretty);
 	}
 
-	bgp_attr_dup(&static_attr, path_vpn->attr); /* shallow copy */
+	/* shallow copy */
+	static_attr = *path_vpn->attr;
 
 	/*
 	 * Nexthop: stash and clear
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index ea451f5f8d..2fe35d0b01 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -1695,7 +1695,7 @@ int subgroup_announce_check(struct bgp_node *rn, struct bgp_path_info *pi,
 	}
 
 	/* For modify attribute, copy it to temporary structure. */
-	bgp_attr_dup(attr, piattr);
+	*attr = *piattr;
 
 	/* If local-preference is not set. */
 	if ((peer->sort == BGP_PEER_IBGP || peer->sort == BGP_PEER_CONFED)
@@ -1814,7 +1814,7 @@ int subgroup_announce_check(struct bgp_node *rn, struct bgp_path_info *pi,
 		if ((from->sort == BGP_PEER_IBGP && peer->sort == BGP_PEER_IBGP)
 		    && !bgp_flag_check(bgp,
 				       BGP_FLAG_RR_ALLOW_OUTBOUND_POLICY)) {
-			bgp_attr_dup(&dummy_attr, attr);
+			dummy_attr = *attr;
 			rmap_path.attr = &dummy_attr;
 		}
 
@@ -3154,7 +3154,7 @@ int bgp_update(struct peer *peer, struct prefix *p, uint32_t addpath_id,
 			goto filtered;
 		}
 
-	bgp_attr_dup(&new_attr, attr);
+	new_attr = *attr;
 
 	/* Apply incoming route-map.
 	 * NB: new_attr may now contain newly allocated values from route-map
@@ -4465,7 +4465,7 @@ int bgp_nlri_parse_ip(struct peer *peer, struct attr *attr,
 		if (addpath_encoded) {
 
 			/* When packet overflow occurs return immediately. */
-			if (pnt + BGP_ADDPATH_ID_LEN > lim)
+			if (pnt + BGP_ADDPATH_ID_LEN >= lim)
 				return BGP_NLRI_PARSE_ERROR_PACKET_OVERFLOW;
 
 			memcpy(&addpath_id, pnt, BGP_ADDPATH_ID_LEN);
@@ -6744,7 +6744,7 @@ void bgp_redistribute_add(struct bgp *bgp, struct prefix *p,
 		struct attr attr_new;
 
 		/* Copy attribute for modification. */
-		bgp_attr_dup(&attr_new, &attr);
+		attr_new = attr;
 
 		if (red->redist_metric_flag)
 			attr_new.med = red->redist_metric;
@@ -9255,7 +9255,7 @@ static int bgp_show_table(struct vty *vty, struct bgp *bgp, safi_t safi,
 				struct attr dummy_attr;
 				route_map_result_t ret;
 
-				bgp_attr_dup(&dummy_attr, pi->attr);
+				dummy_attr = *pi->attr;
 
 				path.peer = pi->peer;
 				path.attr = &dummy_attr;
@@ -11330,7 +11330,7 @@ static void show_adj_route(struct vty *vty, struct peer *peer, afi_t afi,
 					header2 = 0;
 				}
 
-				bgp_attr_dup(&attr, ain->attr);
+				attr = *ain->attr;
 				route_filtered = false;
 
 				/* Filter prefix using distribute list,
@@ -11431,7 +11431,7 @@ static void show_adj_route(struct vty *vty, struct peer *peer, afi_t afi,
 						header2 = 0;
 					}
 
-					bgp_attr_dup(&attr, adj->attr);
+					attr = *adj->attr;
 					ret = bgp_output_modifier(
 						peer, &rn->p, &attr, afi, safi,
 						rmap_name);
diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c
index 0cc2e354d5..6117d62ab5 100644
--- a/bgpd/bgp_updgrp_adv.c
+++ b/bgpd/bgp_updgrp_adv.c
@@ -755,7 +755,7 @@ void subgroup_default_originate(struct update_subgroup *subgrp, int withdraw)
 
 				/* Provide dummy so the route-map can't modify
 				 * the attributes */
-				bgp_attr_dup(&dummy_attr, ri->attr);
+				dummy_attr = *ri->attr;
 				tmp_info.peer = ri->peer;
 				tmp_info.attr = &dummy_attr;
 
diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
index 71f7f6d0e3..e561832e2d 100644
--- a/bgpd/bgp_zebra.c
+++ b/bgpd/bgp_zebra.c
@@ -1726,7 +1726,7 @@ int bgp_redistribute_metric_set(struct bgp *bgp, struct bgp_redist *red,
 				struct attr *old_attr;
 				struct attr new_attr;
 
-				bgp_attr_dup(&new_attr, pi->attr);
+				new_attr = *pi->attr;
 				new_attr.med = red->redist_metric;
 				old_attr = pi->attr;
 				pi->attr = bgp_attr_intern(&new_attr);
diff --git a/bgpd/rfapi/vnc_export_bgp.c b/bgpd/rfapi/vnc_export_bgp.c
index 3d8d5bccb0..c85c78a77a 100644
--- a/bgpd/rfapi/vnc_export_bgp.c
+++ b/bgpd/rfapi/vnc_export_bgp.c
@@ -78,7 +78,7 @@ static void encap_attr_export_ce(struct attr *new, struct attr *orig,
 	 * Make "new" a ghost attr copy of "orig"
 	 */
 	memset(new, 0, sizeof(struct attr));
-	bgp_attr_dup(new, orig);
+	*new = *orig;
 
 	/*
 	 * Set nexthop
@@ -616,7 +616,7 @@ encap_attr_export(struct attr *new, struct attr *orig,
 	 * Make "new" a ghost attr copy of "orig"
 	 */
 	memset(new, 0, sizeof(struct attr));
-	bgp_attr_dup(new, orig);
+	*new = *orig;
 
 	/*
 	 * Set nexthop
diff --git a/bgpd/rfapi/vnc_import_bgp.c b/bgpd/rfapi/vnc_import_bgp.c
index eb2d0fd889..f9bfe76d66 100644
--- a/bgpd/rfapi/vnc_import_bgp.c
+++ b/bgpd/rfapi/vnc_import_bgp.c
@@ -356,7 +356,8 @@ static int process_unicast_route(struct bgp *bgp,		 /* in */
 	 * all of the possible returns above.
 	 */
 	memset(&hattr, 0, sizeof(struct attr));
-	bgp_attr_dup(&hattr, attr); /* hattr becomes a ghost attr */
+	/* hattr becomes a ghost attr */
+	hattr = *attr;
 
 	if (rmap) {
 		struct bgp_path_info info;
@@ -808,7 +809,8 @@ static void vnc_import_bgp_add_route_mode_plain(struct bgp *bgp,
 	 * all of the possible returns above.
 	 */
 	memset(&hattr, 0, sizeof(struct attr));
-	bgp_attr_dup(&hattr, attr); /* hattr becomes a ghost attr */
+	/* hattr becomes a ghost attr */
+	hattr = *attr;
 
 	if (rmap) {
 		struct bgp_path_info info;
@@ -1010,7 +1012,8 @@ vnc_import_bgp_add_route_mode_nvegroup(struct bgp *bgp, struct prefix *prefix,
 	 * all of the possible returns above.
 	 */
 	memset(&hattr, 0, sizeof(struct attr));
-	bgp_attr_dup(&hattr, attr); /* hattr becomes a ghost attr */
+	/* hattr becomes a ghost attr */
+	hattr = *attr;
 
 	if (rmap) {
 		struct bgp_path_info path;
@@ -1797,7 +1800,7 @@ static void vnc_import_bgp_exterior_add_route_it(
 
 				/* use local_pref from unicast route */
 				memset(&new_attr, 0, sizeof(struct attr));
-				bgp_attr_dup(&new_attr, bpi_interior->attr);
+				new_attr = *bpi_interior->attr;
 				if (info->attr->flag
 				    & ATTR_FLAG_BIT(BGP_ATTR_LOCAL_PREF)) {
 					new_attr.local_pref =
@@ -2128,7 +2131,7 @@ void vnc_import_bgp_exterior_add_route_interior(
 
 			/* use local_pref from unicast route */
 			memset(&new_attr, 0, sizeof(struct attr));
-			bgp_attr_dup(&new_attr, bpi_interior->attr);
+			new_attr = *bpi_interior->attr;
 			if (bpi_exterior
 			    && (bpi_exterior->attr->flag
 				& ATTR_FLAG_BIT(BGP_ATTR_LOCAL_PREF))) {
@@ -2265,7 +2268,7 @@ void vnc_import_bgp_exterior_add_route_interior(
 
 				/* use local_pref from unicast route */
 				memset(&new_attr, 0, sizeof(struct attr));
-				bgp_attr_dup(&new_attr, bpi_interior->attr);
+				new_attr = *bpi_interior->attr;
 				if (bpi_exterior
 				    && (bpi_exterior->attr->flag
 					& ATTR_FLAG_BIT(BGP_ATTR_LOCAL_PREF))) {
@@ -2386,7 +2389,7 @@ void vnc_import_bgp_exterior_add_route_interior(
 
 			/* use local_pref from unicast route */
 			memset(&new_attr, 0, sizeof(struct attr));
-			bgp_attr_dup(&new_attr, bpi_interior->attr);
+			new_attr = *bpi_interior->attr;
 			if (bpi_exterior
 			    && (bpi_exterior->attr->flag
 				& ATTR_FLAG_BIT(BGP_ATTR_LOCAL_PREF))) {
@@ -2577,7 +2580,7 @@ void vnc_import_bgp_exterior_del_route_interior(
 
 				/* use local_pref from unicast route */
 				memset(&new_attr, 0, sizeof(struct attr));
-				bgp_attr_dup(&new_attr, bpi->attr);
+				new_attr = *bpi->attr;
 				if (bpi_exterior
 				    && (bpi_exterior->attr->flag
 					& ATTR_FLAG_BIT(BGP_ATTR_LOCAL_PREF))) {
diff --git a/lib/plist.c b/lib/plist.c
index 64571a05b7..a0976cd6bd 100644
--- a/lib/plist.c
+++ b/lib/plist.c
@@ -1891,6 +1891,8 @@ int prefix_bgp_orf_set(char *name, afi_t afi, struct orf_prefix *orfp,
 	if (!plist)
 		return CMD_WARNING_CONFIG_FAILED;
 
+	apply_mask(&orfp->p);
+
 	if (set) {
 		pentry = prefix_list_entry_make(
 			&orfp->p, (permit ? PREFIX_PERMIT : PREFIX_DENY),
diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 5a29c1fb07..e80d826a3c 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -2319,7 +2319,7 @@ static struct stream *ospf_recv_packet(struct ospf *ospf, int fd,
 			  safe_strerror(errno));
 		return NULL;
 	}
-	if ((unsigned int)ret < sizeof(iph)) /* ret must be > 0 now */
+	if ((unsigned int)ret < sizeof(struct ip))
 	{
 		flog_warn(
 			EC_OSPF_PACKET,
@@ -2994,9 +2994,23 @@ int ospf_read(struct thread *thread)
 		return 0;
 	}
 
-	/* Advance from IP header to OSPF header (iph->ip_hl has been verified
-	   by ospf_recv_packet() to be correct). */
-	stream_forward_getp(ibuf, iph->ip_hl * 4);
+	/* Check that we have enough for an IP header */
+	if ((unsigned int)(iph->ip_hl << 2) >= STREAM_READABLE(ibuf)) {
+		if ((unsigned int)(iph->ip_hl << 2) == STREAM_READABLE(ibuf)) {
+			flog_warn(
+				EC_OSPF_PACKET,
+				"Rx'd IP packet with OSPF protocol number but no payload");
+		} else {
+			flog_warn(
+				EC_OSPF_PACKET,
+				"IP header length field claims header is %u bytes, but we only have %zu",
+				(unsigned int)(iph->ip_hl << 2),
+				STREAM_READABLE(ibuf));
+		}
+
+		return -1;
+	}
+	stream_forward_getp(ibuf, iph->ip_hl << 2);
 
 	ospfh = (struct ospf_header *)stream_pnt(ibuf);
 	if (MSG_OK
diff --git a/pimd/pim_igmp.c b/pimd/pim_igmp.c
index 7dfd26ea65..d87cea0d35 100644
--- a/pimd/pim_igmp.c
+++ b/pimd/pim_igmp.c
@@ -478,10 +478,24 @@ int pim_igmp_packet(struct igmp_sock *igmp, char *buf, size_t len)
 			ip_hdr->ip_p);
 	}
 
+	if (ip_hlen > len) {
+		zlog_warn(
+			"IGMP packet header claims size %zu, but we only have %zu bytes",
+			ip_hlen, len);
+		return -1;
+	}
+
 	igmp_msg = buf + ip_hlen;
-	msg_type = *igmp_msg;
 	igmp_msg_len = len - ip_hlen;
 
+	if (igmp_msg_len < PIM_IGMP_MIN_LEN) {
+		zlog_warn("IGMP message size=%d shorter than minimum=%d",
+			  igmp_msg_len, PIM_IGMP_MIN_LEN);
+		return -1;
+	}
+
+	msg_type = *igmp_msg;
+
 	if (PIM_DEBUG_IGMP_PACKETS) {
 		zlog_debug(
 			"Recv IGMP packet from %s to %s on %s: ttl=%d msg_type=%d msg_size=%d",
@@ -489,12 +503,6 @@ int pim_igmp_packet(struct igmp_sock *igmp, char *buf, size_t len)
 			msg_type, igmp_msg_len);
 	}
 
-	if (igmp_msg_len < PIM_IGMP_MIN_LEN) {
-		zlog_warn("IGMP message size=%d shorter than minimum=%d",
-			  igmp_msg_len, PIM_IGMP_MIN_LEN);
-		return -1;
-	}
-
 	switch (msg_type) {
 	case PIM_IGMP_MEMBERSHIP_QUERY: {
 		int max_resp_code = igmp_msg[1];
diff --git a/pimd/pim_igmp_mtrace.c b/pimd/pim_igmp_mtrace.c
index 0758e2f784..695d04c7c2 100644
--- a/pimd/pim_igmp_mtrace.c
+++ b/pimd/pim_igmp_mtrace.c
@@ -864,6 +864,16 @@ int igmp_mtrace_recv_response(struct igmp_sock *igmp, struct ip *ip_hdr,
 	pim_ifp = ifp->info;
 	pim = pim_ifp->pim;
 
+	if (igmp_msg_len < (int)sizeof(struct igmp_mtrace)) {
+		if (PIM_DEBUG_MTRACE)
+			zlog_warn(
+				"Recv mtrace packet from %s on %s: too short,"
+				" len=%d, min=%zu",
+				from_str, ifp->name, igmp_msg_len,
+				sizeof(struct igmp_mtrace));
+		return -1;
+	}
+
 	mtracep = (struct igmp_mtrace *)igmp_msg;
 
 	recv_checksum = mtracep->checksum;
diff --git a/pimd/pim_mroute.c b/pimd/pim_mroute.c
index f7f4b54aea..6472de42d4 100644
--- a/pimd/pim_mroute.c
+++ b/pimd/pim_mroute.c
@@ -585,6 +585,9 @@ static int pim_mroute_msg(struct pim_instance *pim, const char *buf,
 	struct in_addr ifaddr;
 	struct igmp_sock *igmp;
 
+	if (buf_size < (int)sizeof(struct ip))
+		return 0;
+
 	ip_hdr = (const struct ip *)buf;
 
 	if (ip_hdr->ip_p == IPPROTO_IGMP) {