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?)

13 files changed, 126 insertions, 109 deletions

diff --git a/bgpd/bgp_advertise.c b/bgpd/bgp_advertise.c
index 87eb7ac7..666218fa 100644
--- a/bgpd/bgp_advertise.c
+++ b/bgpd/bgp_advertise.c
@@ -140,13 +140,13 @@ bgp_advertise_unintern (struct hash *hash, struct bgp_advertise_attr *baa)
     baa->refcnt--;
 
   if (baa->refcnt && baa->attr)
-    bgp_attr_unintern (baa->attr);
+    bgp_attr_unintern (&baa->attr);
   else
     {
       if (baa->attr)
 	{
 	  hash_release (hash, baa);
-	  bgp_attr_unintern (baa->attr);
+	  bgp_attr_unintern (&baa->attr);
 	}
       baa_free (baa);
     }
@@ -319,7 +319,7 @@ bgp_adj_out_remove (struct bgp_node *rn, struct bgp_adj_out *adj,
 		    struct peer *peer, afi_t afi, safi_t safi)
 {
   if (adj->attr)
-    bgp_attr_unintern (adj->attr);
+    bgp_attr_unintern (&adj->attr);
 
   if (adj->adv)
     bgp_advertise_clean (peer, adj, afi, safi);
@@ -339,7 +339,7 @@ bgp_adj_in_set (struct bgp_node *rn, struct peer *peer, struct attr *attr)
 	{
 	  if (adj->attr != attr)
 	    {
-	      bgp_attr_unintern (adj->attr);
+	      bgp_attr_unintern (&adj->attr);
 	      adj->attr = bgp_attr_intern (attr);
 	    }
 	  return;
@@ -355,7 +355,7 @@ bgp_adj_in_set (struct bgp_node *rn, struct peer *peer, struct attr *attr)
 void
 bgp_adj_in_remove (struct bgp_node *rn, struct bgp_adj_in *bai)
 {
-  bgp_attr_unintern (bai->attr);
+  bgp_attr_unintern (&bai->attr);
   BGP_ADJ_IN_DEL (rn, bai);
   peer_unlock (bai->peer); /* adj_in peer reference */
   XFREE (MTYPE_BGP_ADJ_IN, bai);
diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
index 89a27531..176a960e 100644
--- a/bgpd/bgp_aspath.c
+++ b/bgpd/bgp_aspath.c
@@ -340,19 +340,21 @@ aspath_free (struct aspath *aspath)
 
 /* Unintern aspath from AS path bucket. */
 void
-aspath_unintern (struct aspath *aspath)
+aspath_unintern (struct aspath **aspath)
 {
   struct aspath *ret;
+  struct aspath *asp = *aspath;
+  
+  if (asp->refcnt)
+    asp->refcnt--;
 
-  if (aspath->refcnt)
-    aspath->refcnt--;
-
-  if (aspath->refcnt == 0)
+  if (asp->refcnt == 0)
     {
       /* This aspath must exist in aspath hash table. */
-      ret = hash_release (ashash, aspath);
+      ret = hash_release (ashash, asp);
       assert (ret != NULL);
-      aspath_free (aspath);
+      aspath_free (asp);
+      *aspath = NULL;
     }
 }
 
diff --git a/bgpd/bgp_aspath.h b/bgpd/bgp_aspath.h
index b8a5dfab..d55f9ce6 100644
--- a/bgpd/bgp_aspath.h
+++ b/bgpd/bgp_aspath.h
@@ -80,7 +80,7 @@ extern struct aspath *aspath_empty_get (void);
 extern struct aspath *aspath_str2aspath (const char *);
 extern void aspath_free (struct aspath *);
 extern struct aspath *aspath_intern (struct aspath *);
-extern void aspath_unintern (struct aspath *);
+extern void aspath_unintern (struct aspath **);
 extern const char *aspath_print (struct aspath *);
 extern void aspath_print_vty (struct vty *, const char *, struct aspath *, const char *);
 extern void aspath_print_all_vty (struct vty *);
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 7a0ab560..0c5355b4 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -500,6 +500,7 @@ bgp_attr_intern (struct attr *attr)
             attre->ecommunity = ecommunity_intern (attre->ecommunity);
           else
             attre->ecommunity->refcnt++;
+          
         }
       if (attre->cluster)
         {
@@ -516,10 +517,10 @@ bgp_attr_intern (struct attr *attr)
             attre->transit->refcnt++;
         }
     }
-
+  
   find = (struct attr *) hash_get (attrhash, attr, bgp_attr_hash_alloc);
   find->refcnt++;
-
+  
   return find;
 }
 
@@ -561,7 +562,7 @@ bgp_attr_default_intern (u_char origin)
   new = bgp_attr_intern (&attr);
   bgp_attr_extra_free (&attr);
   
-  aspath_unintern (new->aspath);
+  aspath_unintern (&new->aspath);
   return new;
 }
 
@@ -613,13 +614,13 @@ bgp_attr_aggregate_intern (struct bgp *bgp, u_char origin,
   new = bgp_attr_intern (&attr);
   bgp_attr_extra_free (&attr);
   
-  aspath_unintern (new->aspath);
+  aspath_unintern (&new->aspath);
   return new;
 }
 
 /* Free bgp attribute and aspath. */
 void
-bgp_attr_unintern (struct attr *attr)
+bgp_attr_unintern (struct attr **attr)
 {
   struct attr *ret;
   struct aspath *aspath;
@@ -627,34 +628,35 @@ bgp_attr_unintern (struct attr *attr)
   struct ecommunity *ecommunity = NULL;
   struct cluster_list *cluster = NULL;
   struct transit *transit = NULL;
-
+  
   /* Decrement attribute reference. */
-  attr->refcnt--;
-  aspath = attr->aspath;
-  community = attr->community;
-  if (attr->extra)
+  (*attr)->refcnt--;
+  aspath = (*attr)->aspath;
+  community = (*attr)->community;
+  if ((*attr)->extra)
     {
-      ecommunity = attr->extra->ecommunity;
-      cluster = attr->extra->cluster;
-      transit = attr->extra->transit;
+      ecommunity = (*attr)->extra->ecommunity;
+      cluster = (*attr)->extra->cluster;
+      transit = (*attr)->extra->transit;
     }
-
+  
   /* If reference becomes zero then free attribute object. */
-  if (attr->refcnt == 0)
+  if ((*attr)->refcnt == 0)
     {    
-      ret = hash_release (attrhash, attr);
+      ret = hash_release (attrhash, *attr);
       assert (ret != NULL);
-      bgp_attr_extra_free (attr);
-      XFREE (MTYPE_ATTR, attr);
+      bgp_attr_extra_free (*attr);
+      XFREE (MTYPE_ATTR, *attr);
+      *attr = NULL;
     }
 
   /* aspath refcount shoud be decrement. */
   if (aspath)
-    aspath_unintern (aspath);
+    aspath_unintern (&aspath);
   if (community)
-    community_unintern (community);
+    community_unintern (&community);
   if (ecommunity)
-    ecommunity_unintern (ecommunity);
+    ecommunity_unintern (&ecommunity);
   if (cluster)
     cluster_unintern (cluster);
   if (transit)
@@ -671,8 +673,9 @@ bgp_attr_flush (struct attr *attr)
   if (attr->extra)
     {
       struct attr_extra *attre = attr->extra;
+
       if (attre->ecommunity && ! attre->ecommunity->refcnt)
-        ecommunity_free (attre->ecommunity);
+        ecommunity_free (&attre->ecommunity);
       if (attre->cluster && ! attre->cluster->refcnt)
         cluster_free (attre->cluster);
       if (attre->transit && ! attre->transit->refcnt)
@@ -840,7 +843,7 @@ static int bgp_attr_aspath_check( struct peer *peer,
     {
       aspath = aspath_dup (attr->aspath);
       aspath = aspath_add_seq (aspath, peer->change_local_as);
-      aspath_unintern (attr->aspath);
+      aspath_unintern (&attr->aspath);
       attr->aspath = aspath_intern (aspath);
     }
 
@@ -1146,7 +1149,7 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
   if ( !ignore_as4_path && (attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS4_PATH))) )
     {
        newpath = aspath_reconcile_as4 (attr->aspath, as4_path);
-       aspath_unintern (attr->aspath);
+       aspath_unintern (&attr->aspath);
        attr->aspath = aspath_intern (newpath);
     }
   return 0;
@@ -1707,8 +1710,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
    */
   if ( as4_path )
     {
-      aspath_unintern( as4_path ); /* unintern - it is in the hash */
-      as4_path = NULL;
+      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
        */
diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
index af9dcf5e..3433a1a5 100644
--- a/bgpd/bgp_attr.h
+++ b/bgpd/bgp_attr.h
@@ -142,7 +142,7 @@ extern struct attr_extra *bgp_attr_extra_get (struct attr *);
 extern void bgp_attr_extra_free (struct attr *);
 extern void bgp_attr_dup (struct attr *, struct attr *);
 extern struct attr *bgp_attr_intern (struct attr *attr);
-extern void bgp_attr_unintern (struct attr *);
+extern void bgp_attr_unintern (struct attr **);
 extern void bgp_attr_flush (struct attr *);
 extern struct attr *bgp_attr_default_set (struct attr *attr, u_char);
 extern struct attr *bgp_attr_default_intern (u_char);
diff --git a/bgpd/bgp_clist.c b/bgpd/bgp_clist.c
index d6601674..6c9976e3 100644
--- a/bgpd/bgp_clist.c
+++ b/bgpd/bgp_clist.c
@@ -70,7 +70,7 @@ community_entry_free (struct community_entry *entry)
       if (entry->config)
         XFREE (MTYPE_ECOMMUNITY_STR, entry->config);
       if (entry->u.ecom)
-        ecommunity_free (entry->u.ecom);
+        ecommunity_free (&entry->u.ecom);
       break;
     case COMMUNITY_LIST_EXPANDED:
     case EXTCOMMUNITY_LIST_EXPANDED:
@@ -806,7 +806,7 @@ extcommunity_list_unset (struct community_list_handler *ch,
     entry = community_list_entry_lookup (list, str, direct);
 
   if (ecom)
-    ecommunity_free (ecom);
+    ecommunity_free (&ecom);
   if (regex)
     bgp_regex_free (regex);
 
diff --git a/bgpd/bgp_community.c b/bgpd/bgp_community.c
index ae1d7a15..2ba45f6e 100644
--- a/bgpd/bgp_community.c
+++ b/bgpd/bgp_community.c
@@ -321,21 +321,22 @@ community_intern (struct community *com)
 
 /* Free community attribute. */
 void
-community_unintern (struct community *com)
+community_unintern (struct community **com)
 {
   struct community *ret;
 
-  if (com->refcnt)
-    com->refcnt--;
+  if ((*com)->refcnt)
+    (*com)->refcnt--;
 
   /* Pull off from hash.  */
-  if (com->refcnt == 0)
+  if ((*com)->refcnt == 0)
     {
       /* Community value com must exist in hash. */
-      ret = (struct community *) hash_release (comhash, com);
+      ret = (struct community *) hash_release (comhash, *com);
       assert (ret != NULL);
 
-      community_free (com);
+      community_free (*com);
+      *com = NULL;
     }
 }
 
diff --git a/bgpd/bgp_community.h b/bgpd/bgp_community.h
index bc1e56ef..9e483770 100644
--- a/bgpd/bgp_community.h
+++ b/bgpd/bgp_community.h
@@ -57,7 +57,7 @@ extern void community_free (struct community *);
 extern struct community *community_uniq_sort (struct community *);
 extern struct community *community_parse (u_int32_t *, u_short);
 extern struct community *community_intern (struct community *);
-extern void community_unintern (struct community *);
+extern void community_unintern (struct community **);
 extern char *community_str (struct community *);
 extern unsigned int community_hash_make (struct community *);
 extern struct community *community_str2com (const char *);
diff --git a/bgpd/bgp_ecommunity.c b/bgpd/bgp_ecommunity.c
index 8d5fa741..8d91c746 100644
--- a/bgpd/bgp_ecommunity.c
+++ b/bgpd/bgp_ecommunity.c
@@ -42,13 +42,14 @@ ecommunity_new (void)
 
 /* 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 @@ ecommunity_intern (struct ecommunity *ecom)
   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 @@ ecommunity_intern (struct ecommunity *ecom)
 
 /* 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 @@ ecommunity_str2com (const char *str, int type, int keyword_included)
 	  if (! keyword_included || keyword)
 	    {
 	      if (ecom)
-		ecommunity_free (ecom);
+		ecommunity_free (&ecom);
 	      return NULL;
 	    }
 	  keyword = 1;
@@ -536,7 +537,7 @@ ecommunity_str2com (const char *str, int type, int keyword_included)
 	      if (! keyword)
 		{
 		  if (ecom)
-		    ecommunity_free (ecom);
+		    ecommunity_free (&ecom);
 		  return NULL;
 		}
 	      keyword = 0;
@@ -549,7 +550,7 @@ ecommunity_str2com (const char *str, int type, int keyword_included)
 	case ecommunity_token_unknown:
 	default:
 	  if (ecom)
-	    ecommunity_free (ecom);
+	    ecommunity_free (&ecom);
 	  return NULL;
 	}
     }
diff --git a/bgpd/bgp_ecommunity.h b/bgpd/bgp_ecommunity.h
index 942fdc73..2f59dc40 100644
--- a/bgpd/bgp_ecommunity.h
+++ b/bgpd/bgp_ecommunity.h
@@ -67,13 +67,13 @@ struct ecommunity_val
 
 extern void ecommunity_init (void);
 extern void ecommunity_finish (void);
-extern void ecommunity_free (struct ecommunity *);
+extern void ecommunity_free (struct ecommunity **);
 extern struct ecommunity *ecommunity_parse (u_int8_t *, u_short);
 extern struct ecommunity *ecommunity_dup (struct ecommunity *);
 extern struct ecommunity *ecommunity_merge (struct ecommunity *, struct ecommunity *);
 extern struct ecommunity *ecommunity_intern (struct ecommunity *);
 extern int ecommunity_cmp (const void *, const void *);
-extern void ecommunity_unintern (struct ecommunity *);
+extern void ecommunity_unintern (struct ecommunity **);
 extern unsigned int ecommunity_hash_make (void *);
 extern struct ecommunity *ecommunity_str2com (const char *, int, int);
 extern char *ecommunity_ecom2str (struct ecommunity *, int);
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 9102add7..dee0b51f 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -206,7 +206,7 @@ bgp_update_packet (struct peer *peer, afi_t afi, safi_t safi)
 
       /* Synchnorize attribute.  */
       if (adj->attr)
-	bgp_attr_unintern (adj->attr);
+	bgp_attr_unintern (&adj->attr);
       else
 	peer->scount[afi][safi]++;
 
@@ -1785,13 +1785,13 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
   /* Everything is done.  We unintern temporary structures which
      interned in bgp_attr_parse(). */
   if (attr.aspath)
-    aspath_unintern (attr.aspath);
+    aspath_unintern (&attr.aspath);
   if (attr.community)
-    community_unintern (attr.community);
+    community_unintern (&attr.community);
   if (attr.extra)
     {
       if (attr.extra->ecommunity)
-        ecommunity_unintern (attr.extra->ecommunity);
+        ecommunity_unintern (&attr.extra->ecommunity);
       if (attr.extra->cluster)
         cluster_unintern (attr.extra->cluster);
       if (attr.extra->transit)
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 13ff63ca..87cb677c 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -137,7 +137,7 @@ static void
 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 @@ bgp_update_rsclient (struct peer *rsclient, afi_t afi, safi_t safi,
   /* 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 @@ bgp_update_rsclient (struct peer *rsclient, afi_t afi, safi_t safi,
       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 @@ bgp_update_rsclient (struct peer *rsclient, afi_t afi, safi_t safi,
                     p->prefixlen, rsclient->host);
 
           bgp_unlock_node (rn);
-          bgp_attr_unintern (attr_new);
+          bgp_attr_unintern (&attr_new);
 
           return;
         }
@@ -1868,7 +1868,7 @@ bgp_update_rsclient (struct peer *rsclient, afi_t afi, safi_t safi,
       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_update_main (struct peer *peer, struct prefix *p, struct attr *attr,
 	    }
 
 	  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 @@ bgp_update_main (struct peer *peer, struct prefix *p, struct attr *attr,
 	}
 	
       /* 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_default_originate (struct peer *peer, afi_t afi, safi_t safi, int withdraw)
     }
   
   bgp_attr_extra_free (&attr);
-  aspath_unintern (aspath);
+  aspath_unintern (&aspath);
 }
 
 static void
@@ -3215,7 +3215,7 @@ bgp_static_update_rsclient (struct peer *rsclient, struct prefix *p,
           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_static_update_rsclient (struct peer *rsclient, struct prefix *p,
 
       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_static_update_rsclient (struct peer *rsclient, struct prefix *p,
 
   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 @@ bgp_static_update_rsclient (struct peer *rsclient, struct prefix *p,
 	  !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 @@ bgp_static_update_rsclient (struct peer *rsclient, struct prefix *p,
           /* 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_static_update_rsclient (struct peer *rsclient, struct prefix *p,
   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_static_update_main (struct bgp *bgp, struct prefix *p,
 	  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 @@ bgp_static_update_main (struct bgp *bgp, struct prefix *p,
 	  !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_static_update_main (struct bgp *bgp, struct prefix *p,
 	    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_static_update_main (struct bgp *bgp, struct prefix *p,
 	  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_static_update_main (struct bgp *bgp, struct prefix *p,
   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_redistribute_add (struct prefix *p, struct in_addr *nexthop,
 		  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 @@ bgp_redistribute_add (struct prefix *p, struct in_addr *nexthop,
  	      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_redistribute_add (struct prefix *p, struct in_addr *nexthop,
 		    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_redistribute_add (struct prefix *p, struct in_addr *nexthop,
  		  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 @@ bgp_redistribute_add (struct prefix *p, struct in_addr *nexthop,
     }
 
   /* Unintern original. */
-  aspath_unintern (attr.aspath);
+  aspath_unintern (&attr.aspath);
   bgp_attr_extra_free (&attr);
 }
 
diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index 81ff48db..451458b5 100644
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -1475,10 +1475,10 @@ route_set_ecommunity_rt (void *rule, struct prefix *prefix,
       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 @@ route_set_ecommunity_rt_compile (const char *arg)
   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 @@ static void
 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 @@ static route_map_result_t
 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 @@ route_set_ecommunity_soo (void *rule, struct prefix *prefix,
       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 @@ route_set_ecommunity_soo_compile (const char *arg)
   if (! ecom)
     return NULL;
   
-  return ecom;
+  return ecommunity_intern (ecom);
 }
 
 /* Free function for set community. */
@@ -1556,7 +1567,7 @@ static void
 route_set_ecommunity_soo_free (void *rule)
 {
   struct ecommunity *ecom = rule;
-  ecommunity_free (ecom);
+  ecommunity_unintern (&ecom);
 }
 
 /* Set community rule structure. */