Fixup of peer memory leaks in bgp

When deleting a set of peers, the peer->group pointer was being set to NULL
and then passed into peer_delete.

peer_delete has functionality to safely remove the peer->group structure if it
non-null and to remove the peer->group if it's refcnt reaches zero.

This is a day one bug in the quagga source tree.

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index dec6b7d688fb2c4e9c64670feb7f0fc726887f26..5ef07f0c4bce476781ecde2e627d75e93fb472fe 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -963,9 +963,13 @@ peer_free (struct peer *peer)
                                                 
 /* increase reference count on a struct peer */
 struct peer *
-peer_lock (struct peer *peer)
+peer_lock_with_caller (const char *name, struct peer *peer)
 {
   assert (peer && (peer->lock >= 0));
+
+#if 0
+    zlog_debug("%s peer_lock %p %d", name, peer, peer->lock);
+#endif
     
   peer->lock++;
   
@@ -976,30 +980,22 @@ peer_lock (struct peer *peer)
  * struct peer is freed and NULL returned if last reference
  */
 struct peer *
-peer_unlock (struct peer *peer)
+peer_unlock_with_caller (const char *name, struct peer *peer)
 {
   assert (peer && (peer->lock > 0));
+
+#if 0
+  zlog_debug("%s peer_unlock %p %d", name, peer, peer->lock);
+#endif
   
   peer->lock--;
   
   if (peer->lock == 0)
     {
-#if 0
-      zlog_debug ("unlocked and freeing");
-      zlog_backtrace (LOG_DEBUG);
-#endif
       peer_free (peer);
       return NULL;
     }
 
-#if 0
-  if (peer->lock == 1)
-    {
-      zlog_debug ("unlocked to 1");
-      zlog_backtrace (LOG_DEBUG);
-    }
-#endif
-
   return peer;
 }
 
@@ -1277,7 +1273,7 @@ peer_create (union sockunion *su, const char *conf_if, struct bgp *bgp,
     peer->v_routeadv = BGP_DEFAULT_IBGP_ROUTEADV;
   else
     peer->v_routeadv = BGP_DEFAULT_EBGP_ROUTEADV;
-    
+
   peer = peer_lock (peer); /* bgp peer list reference */
   listnode_add_sort (bgp->peer, peer);
 
@@ -2222,7 +2218,6 @@ peer_group_delete (struct peer_group *group)
   for (ALL_LIST_ELEMENTS (group->peer, node, nnode, peer))
     {
       other = peer->doppelganger;
-      peer->group = NULL;
       peer_delete (peer);
       if (other && other->status != Deleted)
        {
@@ -2271,7 +2266,6 @@ peer_group_remote_as_delete (struct peer_group *group)
     {
       other = peer->doppelganger;
 
-      peer->group = NULL;
       peer_delete (peer);
 
       if (other && other->status != Deleted)

diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 050731c94bc9ddc3a6ffecdc5f3ef100b008fd54..5c9457feac7d407d4ea864ffe6395ada45c3149a 100644 (file)
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -1129,8 +1129,17 @@ extern struct peer_group *peer_group_lookup_dynamic_neighbor (struct bgp *,
 extern struct peer *peer_lookup_dynamic_neighbor (struct bgp *,
                      union sockunion *);
 extern void peer_drop_dynamic_neighbor (struct peer *);
-extern struct peer *peer_lock (struct peer *);
-extern struct peer *peer_unlock (struct peer *);
+
+/*
+ * Peers are incredibly easy to memory leak
+ * due to the various ways that they are actually used
+ * Provide some functionality to debug locks and unlocks
+ */
+extern struct peer *peer_lock_with_caller(const char *, struct peer *);
+extern struct peer *peer_unlock_with_caller(const char *, struct peer *);
+#define peer_unlock(A) peer_unlock_with_caller(__FUNCTION__, (A))
+#define peer_lock(B) peer_lock_with_caller(__FUNCTION__, (B))
+
 extern bgp_peer_sort_t peer_sort (struct peer *peer);
 extern int peer_active (struct peer *);
 extern int peer_active_nego (struct peer *);