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_ecommunity.c b/bgpd/bgp_ecommunity.c
index 8d5fa74..8d91c74 100644
--- a/bgpd/bgp_ecommunity.c
+++ b/bgpd/bgp_ecommunity.c
@@ -42,13 +42,14 @@
/* Allocate ecommunities. */
void
-ecommunity_free (struct ecommunity *ecom)
+ecommunity_free (struct ecommunity **ecom)
{
- if (ecom->val)
- XFREE (MTYPE_ECOMMUNITY_VAL, ecom->val);
- if (ecom->str)
- XFREE (MTYPE_ECOMMUNITY_STR, ecom->str);
- XFREE (MTYPE_ECOMMUNITY, ecom);
+ if ((*ecom)->val)
+ XFREE (MTYPE_ECOMMUNITY_VAL, (*ecom)->val);
+ if ((*ecom)->str)
+ XFREE (MTYPE_ECOMMUNITY_STR, (*ecom)->str);
+ XFREE (MTYPE_ECOMMUNITY, *ecom);
+ ecom = NULL;
}
/* Add a new Extended Communities value to Extended Communities
@@ -197,7 +198,7 @@
find = (struct ecommunity *) hash_get (ecomhash, ecom, hash_alloc_intern);
if (find != ecom)
- ecommunity_free (ecom);
+ ecommunity_free (&ecom);
find->refcnt++;
@@ -209,18 +210,18 @@
/* Unintern Extended Communities Attribute. */
void
-ecommunity_unintern (struct ecommunity *ecom)
+ecommunity_unintern (struct ecommunity **ecom)
{
struct ecommunity *ret;
- if (ecom->refcnt)
- ecom->refcnt--;
-
+ if ((*ecom)->refcnt)
+ (*ecom)->refcnt--;
+
/* Pull off from hash. */
- if (ecom->refcnt == 0)
+ if ((*ecom)->refcnt == 0)
{
/* Extended community must be in the hash. */
- ret = (struct ecommunity *) hash_release (ecomhash, ecom);
+ ret = (struct ecommunity *) hash_release (ecomhash, *ecom);
assert (ret != NULL);
ecommunity_free (ecom);
@@ -516,7 +517,7 @@
if (! keyword_included || keyword)
{
if (ecom)
- ecommunity_free (ecom);
+ ecommunity_free (&ecom);
return NULL;
}
keyword = 1;
@@ -536,7 +537,7 @@
if (! keyword)
{
if (ecom)
- ecommunity_free (ecom);
+ ecommunity_free (&ecom);
return NULL;
}
keyword = 0;
@@ -549,7 +550,7 @@
case ecommunity_token_unknown:
default:
if (ecom)
- ecommunity_free (ecom);
+ ecommunity_free (&ecom);
return NULL;
}
}