bgpd,lib: deal with bgp-flowspec pointer in bgpd

BGP-FlowSpec (BGP-FS) utilizes the 'ptr' pointer within the lib to store
a raw copy of BGP-FS NLRI as seen in the structure definitions:

> struct flowspec_prefix {
>         [...]
>         uintptr_t ptr;
> };
>
> /* FRR generic prefix structure. */
> struct prefix {
>         uint8_t family;
>         uint16_t prefixlen;
>         union {
>                 uint8_t prefix;
>                 [...]
>                 struct linkstate_prefix prefix_flowspec; /* AF_FLOWSPEC */
>                 [...]
>         } u __attribute__((aligned(8)));
> };

When copying a BGP-FS prefix with prefix_copy(), memory is allocated for
a new 'ptr' pointer in the copy. If a 'struct prefix' for BGP-FS is
declared on the stack and filled using prefix_copy(), this new pointer
must be freed before the function exits. Releasing this pointer is not
obvious. Forgetting to do so might leads to memory leaks. For example,
the flowspec 'ptr' release is missing in route_node_get().

Deal with BGP-FS 'ptr' pointer memory in bgpd itself.

Signed-off-by: Louis Scalbert <[email protected]>

bgpd/bgp_flowspec.c

@@ -90,7 +90,6 @@ int bgp_nlri_parse_flowspec(struct peer *peer, struct attr *attr,
 	safi_t safi;
 	int psize = 0;
 	struct prefix p;
-	void *temp;
 
 	/* Start processing the NLRI - there may be multiple in the MP_REACH */
 	pnt = packet->nlri;
@@ -152,9 +151,7 @@ int bgp_nlri_parse_flowspec(struct peer *peer, struct attr *attr,
 		/* Flowspec encoding is in bytes */
 		p.u.prefix_flowspec.prefixlen = psize;
 		p.u.prefix_flowspec.family = afi2family(afi);
-		temp = XCALLOC(MTYPE_TMP, psize);
-		memcpy(temp, pnt, psize);
-		p.u.prefix_flowspec.ptr = (uintptr_t) temp;
+		p.u.prefix_flowspec.ptr = bgp_flowspec_ptr_new(pnt, psize);
 
 		if (BGP_DEBUG(flowspec, FLOWSPEC)) {
 			char return_string[BGP_FLOWSPEC_NLRI_STRING_MAX];
@@ -197,8 +194,6 @@ int bgp_nlri_parse_flowspec(struct peer *peer, struct attr *attr,
 			bgp_withdraw(peer, &p, 0, afi, safi, ZEBRA_ROUTE_BGP,
 				     BGP_ROUTE_NORMAL, NULL, NULL, 0, NULL);
 		}
-
-		XFREE(MTYPE_TMP, temp);
 	}
 	return BGP_NLRI_PARSE_OK;
 }

bgpd/bgp_flowspec_util.c

@@ -19,6 +19,26 @@
 #include "bgp_pbr.h"
 #include "bgp_errors.h"
 
+DEFINE_MTYPE_STATIC(BGPD, PTR_FLOWSPEC, "FLOWSPEC Prefix Pointer");
+
+uintptr_t bgp_flowspec_ptr_new(uint8_t *pnt, uint16_t length)
+{
+	void *ptr = XCALLOC(MTYPE_PTR_FLOWSPEC, length);
+
+	memcpy(ptr, pnt, length);
+
+	return (uintptr_t)ptr;
+}
+
+void bgp_flowspec_ptr_free(uintptr_t ptr)
+{
+	void *tmp;
+
+	tmp = (void *)ptr;
+
+	XFREE(MTYPE_PTR_FLOWSPEC, tmp);
+}
+
 static void hex2bin(uint8_t *hex, int *bin)
 {
 	int remainder = *hex;

bgpd/bgp_flowspec_util.h

@@ -17,6 +17,8 @@ enum bgp_flowspec_util_nlri_t {
 	BGP_FLOWSPEC_RETURN_JSON = 3,
 };
 
+extern uintptr_t bgp_flowspec_ptr_new(uint8_t *pnt, uint16_t length);
+extern void bgp_flowspec_ptr_free(uintptr_t ptr);
 
 extern int bgp_flowspec_op_decode(enum bgp_flowspec_util_nlri_t type,
 				  uint8_t *nlri_ptr,

bgpd/bgp_table.c

@@ -119,6 +119,8 @@ static void bgp_node_destroy(route_table_delegate_t *delegate,
 
 	if (node->p.family == AF_LINKSTATE)
 		bgp_linkstate_ptr_free(node->p.u.prefix_linkstate.ptr);
+	else if (node->p.family == AF_FLOWSPEC)
+		bgp_flowspec_ptr_free(node->p.u.prefix_flowspec.ptr);
 
 	XFREE(MTYPE_ROUTE_NODE, node);
 }

bgpd/bgp_table.h

@@ -12,6 +12,7 @@
 #include "linklist.h"
 #include "bgpd.h"
 #include "bgp_advertise.h"
+#include "bgp_flowspec_util.h"
 #include "bgp_linkstate.h"
 
 struct bgp_table {
@@ -242,6 +243,9 @@ static inline struct bgp_dest *bgp_node_get(struct bgp_table *const table,
 	if (p->family == AF_LINKSTATE && table->route_table->count == count)
 		/* ptr pointer has not referenced in a new route_node, free it. */
 		bgp_linkstate_ptr_free(p->u.prefix_linkstate.ptr);
+	else if (p->family == AF_FLOWSPEC && table->route_table->count == count)
+		/* ptr pointer has not referenced in a new route_node, free it. */
+		bgp_flowspec_ptr_free(p->u.prefix_flowspec.ptr);
 
 	if (!rn->info) {
 		struct bgp_dest *dest = XCALLOC(MTYPE_BGP_NODE,

lib/prefix.c

@@ -333,8 +333,6 @@ void prefix_copy(union prefixptr udest, union prefixconstptr usrc)
 {
 	struct prefix *dest = udest.p;
 	const struct prefix *src = usrc.p;
-	void *temp;
-	int len;
 
 	dest->family = src->family;
 	dest->prefixlen = src->prefixlen;
@@ -353,15 +351,11 @@ void prefix_copy(union prefixptr udest, union prefixconstptr usrc)
 		dest->u.lp.id = src->u.lp.id;
 		dest->u.lp.adv_router = src->u.lp.adv_router;
 	} else if (src->family == AF_FLOWSPEC) {
-		len = src->u.prefix_flowspec.prefixlen;
 		dest->u.prefix_flowspec.prefixlen =
 			src->u.prefix_flowspec.prefixlen;
 		dest->u.prefix_flowspec.family =
 			src->u.prefix_flowspec.family;
-		temp = XCALLOC(MTYPE_PREFIX_FLOWSPEC, len);
-		dest->u.prefix_flowspec.ptr = (uintptr_t)temp;
-		memcpy((void *)dest->u.prefix_flowspec.ptr,
-		       (void *)src->u.prefix_flowspec.ptr, len);
+		dest->u.prefix_flowspec.ptr = src->u.prefix_flowspec.ptr;
 	} else if (src->family == AF_LINKSTATE) {
 		dest->u.prefix_linkstate.nlri_type =
 			src->u.prefix_linkstate.nlri_type;
@@ -1409,24 +1403,17 @@ char *prefix_mac2str(const struct ethaddr *mac, char *buf, int size)
 unsigned prefix_hash_key(const void *pp)
 {
 	struct prefix copy;
-	uint32_t len;
-	void *temp;
 
 	/* make sure *all* unused bits are zero, particularly including
 	 * alignment /
 	 * padding and unused prefix bytes. */
 	memset(&copy, 0, sizeof(copy));
 	prefix_copy(&copy, (struct prefix *)pp);
 
-	if (((struct prefix *)pp)->family == AF_FLOWSPEC) {
-		len = jhash((void *)copy.u.prefix_flowspec.ptr,
-			    copy.u.prefix_flowspec.prefixlen,
-			    0x55aa5a5a);
-		temp = (void *)copy.u.prefix_flowspec.ptr;
-		XFREE(MTYPE_PREFIX_FLOWSPEC, temp);
-		copy.u.prefix_flowspec.ptr = (uintptr_t)NULL;
-		return len;
-	} else if (((struct prefix *)pp)->family == AF_LINKSTATE)
+	if (((struct prefix *)pp)->family == AF_FLOWSPEC)
+		return jhash((void *)copy.u.prefix_flowspec.ptr,
+			     copy.u.prefix_flowspec.prefixlen, 0x55aa5a5a);
+	else if (((struct prefix *)pp)->family == AF_LINKSTATE)
 		return jhash((void *)copy.u.prefix_linkstate.ptr, copy.prefixlen,
 			    0x55aa5a5a);