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_route.c b/bgpd/bgp_route.c
index 13ff63c..87cb677 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -137,7 +137,7 @@
bgp_info_free (struct bgp_info *binfo)
{
if (binfo->attr)
- bgp_attr_unintern (binfo->attr);
+ bgp_attr_unintern (&binfo->attr);
bgp_info_extra_free (&binfo->extra);
@@ -1802,14 +1802,14 @@
/* Apply import policy. */
if (bgp_import_modifier (rsclient, peer, p, &new_attr, afi, safi) == RMAP_DENY)
{
- bgp_attr_unintern (attr_new2);
+ bgp_attr_unintern (&attr_new2);
reason = "import-policy;";
goto filtered;
}
attr_new = bgp_attr_intern (&new_attr);
- bgp_attr_unintern (attr_new2);
+ bgp_attr_unintern (&attr_new2);
/* IPv4 unicast next hop check. */
if (afi == AFI_IP && safi == SAFI_UNICAST)
@@ -1818,7 +1818,7 @@
if (new_attr.nexthop.s_addr == 0
|| ntohl (new_attr.nexthop.s_addr) >= 0xe0000000)
{
- bgp_attr_unintern (attr_new);
+ bgp_attr_unintern (&attr_new);
reason = "martian next-hop;";
goto filtered;
@@ -1848,7 +1848,7 @@
p->prefixlen, rsclient->host);
bgp_unlock_node (rn);
- bgp_attr_unintern (attr_new);
+ bgp_attr_unintern (&attr_new);
return;
}
@@ -1868,7 +1868,7 @@
bgp_info_set_flag (rn, ri, BGP_INFO_ATTR_CHANGED);
/* Update to new attribute. */
- bgp_attr_unintern (ri->attr);
+ bgp_attr_unintern (&ri->attr);
ri->attr = attr_new;
/* Update MPLS tag. */
@@ -2128,7 +2128,7 @@
}
bgp_unlock_node (rn);
- bgp_attr_unintern (attr_new);
+ bgp_attr_unintern (&attr_new);
bgp_attr_extra_free (&new_attr);
return 0;
@@ -2175,7 +2175,7 @@
}
/* Update to new attribute. */
- bgp_attr_unintern (ri->attr);
+ bgp_attr_unintern (&ri->attr);
ri->attr = attr_new;
/* Update MPLS tag. */
@@ -2467,7 +2467,7 @@
}
bgp_attr_extra_free (&attr);
- aspath_unintern (aspath);
+ aspath_unintern (&aspath);
}
static void
@@ -3215,7 +3215,7 @@
bgp_attr_flush (&attr_tmp);
/* Unintern original. */
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_static_withdraw_rsclient (bgp, rsclient, p, afi, safi);
bgp_attr_extra_free (&attr);
@@ -3242,8 +3242,8 @@
bgp->peer_self->rmap_type = 0;
- bgp_attr_unintern (attr_new);
- aspath_unintern (attr.aspath);
+ bgp_attr_unintern (&attr_new);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
bgp_static_withdraw_rsclient (bgp, rsclient, p, afi, safi);
@@ -3253,7 +3253,7 @@
bgp->peer_self->rmap_type = 0;
- bgp_attr_unintern (attr_new);
+ bgp_attr_unintern (&attr_new);
attr_new = bgp_attr_intern (&new_attr);
bgp_attr_extra_free (&new_attr);
@@ -3268,8 +3268,8 @@
!CHECK_FLAG(ri->flags, BGP_INFO_REMOVED))
{
bgp_unlock_node (rn);
- bgp_attr_unintern (attr_new);
- aspath_unintern (attr.aspath);
+ bgp_attr_unintern (&attr_new);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
return;
}
@@ -3281,14 +3281,14 @@
/* Rewrite BGP route information. */
if (CHECK_FLAG(ri->flags, BGP_INFO_REMOVED))
bgp_info_restore(rn, ri);
- bgp_attr_unintern (ri->attr);
+ bgp_attr_unintern (&ri->attr);
ri->attr = attr_new;
ri->uptime = bgp_clock ();
/* Process change. */
bgp_process (bgp, rn, afi, safi);
bgp_unlock_node (rn);
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
return;
}
@@ -3313,7 +3313,7 @@
bgp_process (bgp, rn, afi, safi);
/* Unintern original. */
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
}
@@ -3363,7 +3363,7 @@
bgp_attr_flush (&attr_tmp);
/* Unintern original. */
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
bgp_static_withdraw (bgp, p, afi, safi);
return;
@@ -3384,8 +3384,8 @@
!CHECK_FLAG(ri->flags, BGP_INFO_REMOVED))
{
bgp_unlock_node (rn);
- bgp_attr_unintern (attr_new);
- aspath_unintern (attr.aspath);
+ bgp_attr_unintern (&attr_new);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
return;
}
@@ -3399,7 +3399,7 @@
bgp_info_restore(rn, ri);
else
bgp_aggregate_decrement (bgp, p, ri, afi, safi);
- bgp_attr_unintern (ri->attr);
+ bgp_attr_unintern (&ri->attr);
ri->attr = attr_new;
ri->uptime = bgp_clock ();
@@ -3407,7 +3407,7 @@
bgp_aggregate_increment (bgp, p, ri, afi, safi);
bgp_process (bgp, rn, afi, safi);
bgp_unlock_node (rn);
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
return;
}
@@ -3435,7 +3435,7 @@
bgp_process (bgp, rn, afi, safi);
/* Unintern original. */
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
}
@@ -5297,7 +5297,7 @@
bgp_attr_extra_free (&attr_new);
/* Unintern original. */
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
bgp_redistribute_delete (p, type);
return;
@@ -5320,8 +5320,8 @@
if (attrhash_cmp (bi->attr, new_attr) &&
!CHECK_FLAG(bi->flags, BGP_INFO_REMOVED))
{
- bgp_attr_unintern (new_attr);
- aspath_unintern (attr.aspath);
+ bgp_attr_unintern (&new_attr);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
bgp_unlock_node (bn);
return;
@@ -5336,7 +5336,7 @@
bgp_info_restore(bn, bi);
else
bgp_aggregate_decrement (bgp, p, bi, afi, SAFI_UNICAST);
- bgp_attr_unintern (bi->attr);
+ bgp_attr_unintern (&bi->attr);
bi->attr = new_attr;
bi->uptime = bgp_clock ();
@@ -5344,7 +5344,7 @@
bgp_aggregate_increment (bgp, p, bi, afi, SAFI_UNICAST);
bgp_process (bgp, bn, afi, SAFI_UNICAST);
bgp_unlock_node (bn);
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
return;
}
@@ -5366,7 +5366,7 @@
}
/* Unintern original. */
- aspath_unintern (attr.aspath);
+ aspath_unintern (&attr.aspath);
bgp_attr_extra_free (&attr);
}