bgpd: Try fix extcommunity resource allocation probs, particularly with 'set extcom..'
* Extended communities has some kind of resource allocation problem which
causes a double-free if the 'set extcommunity ...' command is used.
Try fix by properly interning extcommunities.
Also, more generally, make unintern functions take a double pointer
so they can NULL out callers references - a usefully defensive programming
pattern for functions which make refs invalid.
Sadly, this patch doesn't fix the problem entirely - crashes still
occur on session clear.
* bgp_ecommunity.h: (ecommunity_{free,unintern}) take double pointer
args.
* bgp_community.h: (community_unintern) ditto
* bgp_attr.h: (bgp_attr_intern) ditto
* bgp_aspath.h: (bgp_aspath.h) ditto
* (general) update all callers of above
* bgp_routemap.c: (route_set_ecommunity_{rt,soo}) intern the new extcom added
to the attr, and unintern any old one.
(route_set_ecommunity_{rt,soo}_compile) intern the extcom to be used
for the route-map set.
(route_set_ecommunity_*_free) unintern to match, instead of free
(route_set_ecommunity_soo) Do as _rt does and don't just leak
any pre-existing community, add to it (is additive right though?)
diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index 81ff48d..451458b 100644
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -1475,10 +1475,10 @@
else
new_ecom = ecommunity_dup (ecom);
- bgp_info->attr->extra->ecommunity = new_ecom;
+ bgp_info->attr->extra->ecommunity = ecommunity_intern (new_ecom);
if (old_ecom)
- ecommunity_free (old_ecom);
+ ecommunity_unintern (&old_ecom);
bgp_info->attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES);
}
@@ -1494,7 +1494,7 @@
ecom = ecommunity_str2com (arg, ECOMMUNITY_ROUTE_TARGET, 0);
if (! ecom)
return NULL;
- return ecom;
+ return ecommunity_intern (ecom);
}
/* Free function for set community. */
@@ -1502,7 +1502,7 @@
route_set_ecommunity_rt_free (void *rule)
{
struct ecommunity *ecom = rule;
- ecommunity_free (ecom);
+ ecommunity_unintern (&ecom);
}
/* Set community rule structure. */
@@ -1521,7 +1521,7 @@
route_set_ecommunity_soo (void *rule, struct prefix *prefix,
route_map_object_t type, void *object)
{
- struct ecommunity *ecom;
+ struct ecommunity *ecom, *old_ecom, *new_ecom;
struct bgp_info *bgp_info;
if (type == RMAP_BGP)
@@ -1532,8 +1532,19 @@
if (! ecom)
return RMAP_OKAY;
+ old_ecom = (bgp_attr_extra_get (bgp_info->attr))->ecommunity;
+
+ if (old_ecom)
+ new_ecom = ecommunity_merge (ecommunity_dup (old_ecom), ecom);
+ else
+ new_ecom = ecommunity_dup (ecom);
+
+ bgp_info->attr->extra->ecommunity = ecommunity_intern (new_ecom);
+
+ if (old_ecom)
+ ecommunity_unintern (&old_ecom);
+
bgp_info->attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES);
- (bgp_attr_extra_get (bgp_info->attr))->ecommunity = ecommunity_dup (ecom);
}
return RMAP_OKAY;
}
@@ -1548,7 +1559,7 @@
if (! ecom)
return NULL;
- return ecom;
+ return ecommunity_intern (ecom);
}
/* Free function for set community. */
@@ -1556,7 +1567,7 @@
route_set_ecommunity_soo_free (void *rule)
{
struct ecommunity *ecom = rule;
- ecommunity_free (ecom);
+ ecommunity_unintern (&ecom);
}
/* Set community rule structure. */