bgpd: When creating peer convey if it is a CONFIG_NODE or not

When actually creating a peer in BGP, tell the creation if
it is a config node or not.  There were cases where the
CONFIG_NODE was being set *after* being placed into
the bgp->peerhash, thus causing collisions between the
doppelganger and the peer and eventually use after free's.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c
index fa618232f61e3149b08f2ddcb96841323654774a..7186a507110027de3dee3be75373ebce9279030d 100644 (file)
--- a/bgpd/bgp_network.c
+++ b/bgpd/bgp_network.c
@@ -553,7 +553,7 @@ static void bgp_accept(struct thread *thread)
                                peer1->host);
 
        peer = peer_create(&su, peer1->conf_if, peer1->bgp, peer1->local_as,
-                          peer1->as, peer1->as_type, NULL);
+                          peer1->as, peer1->as_type, NULL, false);
 
        peer_xfer_config(peer, peer1);
        bgp_peer_gr_flags_update(peer);
@@ -570,8 +570,6 @@ static void bgp_accept(struct thread *thread)
                }
        }
 
-       UNSET_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE);
-
        peer->doppelganger = peer1;
        peer1->doppelganger = peer;
        peer->fd = bgp_sock;

diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 1f532d499038bc44f9480b5784a51d22abe1f8fb..90182be85f0a9daa76dea1f300b96cfa2d48fcd1 100644 (file)
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -4685,7 +4685,7 @@ static int peer_conf_interface_get(struct vty *vty, const char *conf_if,
                        ret = peer_remote_as(bgp, NULL, conf_if, &as, as_type);
        } else {
                peer = peer_create(NULL, conf_if, bgp, bgp->as, as, as_type,
-                                  NULL);
+                                  NULL, true);
 
                if (!peer) {
                        vty_out(vty, "%% BGP failed to create peer\n");

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 992306773b41b0c5cc45ee095d12f1fcd4c18f60..36ff8866e65eacb68f326e75ee4e51ba62c38f72 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -950,6 +950,7 @@ static bool peer_hash_same(const void *p1, const void *p2)
 {
        const struct peer *peer1 = p1;
        const struct peer *peer2 = p2;
+
        return (sockunion_same(&peer1->su, &peer2->su)
                && CHECK_FLAG(peer1->flags, PEER_FLAG_CONFIG_NODE)
                           == CHECK_FLAG(peer2->flags, PEER_FLAG_CONFIG_NODE));
@@ -1761,7 +1762,8 @@ void bgp_recalculate_all_bestpaths(struct bgp *bgp)
  */
 struct peer *peer_create(union sockunion *su, const char *conf_if,
                         struct bgp *bgp, as_t local_as, as_t remote_as,
-                        int as_type, struct peer_group *group)
+                        int as_type, struct peer_group *group,
+                        bool config_node)
 {
        int active;
        struct peer *peer;
@@ -1799,6 +1801,10 @@ struct peer *peer_create(union sockunion *su, const char *conf_if,
        peer = peer_lock(peer); /* bgp peer list reference */
        peer->group = group;
        listnode_add_sort(bgp->peer, peer);
+
+       if (config_node)
+               SET_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE);
+
        (void)hash_get(bgp->peerhash, peer, hash_alloc_intern);
 
        /* Adjust update-group coalesce timer heuristics for # peers. */
@@ -1826,8 +1832,6 @@ struct peer *peer_create(union sockunion *su, const char *conf_if,
        /* Default configured keepalives count for shutdown rtt command */
        peer->rtt_keepalive_conf = 1;
 
-       SET_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE);
-
        /* If 'bgp default <afi>-<safi>' is configured, then activate the
         * neighbor for the corresponding address family. IPv4 Unicast is
         * the only address family enabled by default without expliict
@@ -2034,7 +2038,8 @@ int peer_remote_as(struct bgp *bgp, union sockunion *su, const char *conf_if,
                else
                        local_as = bgp->as;
 
-               peer_create(su, conf_if, bgp, local_as, *as, as_type, NULL);
+               peer_create(su, conf_if, bgp, local_as, *as, as_type, NULL,
+                           true);
        }
 
        return 0;
@@ -3136,7 +3141,7 @@ int peer_group_bind(struct bgp *bgp, union sockunion *su, struct peer *peer,
                }
 
                peer = peer_create(su, NULL, bgp, bgp->as, group->conf->as,
-                                  group->conf->as_type, group);
+                                  group->conf->as_type, group, true);
 
                peer = peer_lock(peer); /* group->peer list reference */
                listnode_add(group->peer, peer);
@@ -3158,8 +3163,6 @@ int peer_group_bind(struct bgp *bgp, union sockunion *su, struct peer *peer,
                                peer_deactivate(peer, afi, safi);
                }
 
-               SET_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE);
-
                /* Set up peer's events and timers. */
                if (peer_active(peer))
                        bgp_timer_set(peer);
@@ -4032,7 +4035,7 @@ struct peer *peer_create_bind_dynamic_neighbor(struct bgp *bgp,
 
        /* Create peer first; we've already checked group config is valid. */
        peer = peer_create(su, NULL, bgp, bgp->as, group->conf->as,
-                          group->conf->as_type, group);
+                          group->conf->as_type, group, true);
        if (!peer)
                return NULL;
 
@@ -4061,7 +4064,6 @@ struct peer *peer_create_bind_dynamic_neighbor(struct bgp *bgp,
        /* Mark as dynamic, but also as a "config node" for other things to
         * work. */
        SET_FLAG(peer->flags, PEER_FLAG_DYNAMIC_NEIGHBOR);
-       SET_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE);
 
        return peer;
 }

diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index b7a329fdcdeb3c82c9bdfca6f7a1ed8a5af70515..a75bfdf7465def994ccb076083a7836984484155 100644 (file)
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -2177,8 +2177,10 @@ extern bool peer_active_nego(struct peer *);
 extern bool peer_afc_received(struct peer *peer);
 extern bool peer_afc_advertised(struct peer *peer);
 extern void bgp_recalculate_all_bestpaths(struct bgp *bgp);
-extern struct peer *peer_create(union sockunion *, const char *, struct bgp *,
-                               as_t, as_t, int, struct peer_group *);
+extern struct peer *peer_create(union sockunion *su, const char *conf_if,
+                               struct bgp *bgp, as_t local_as, as_t remote_as,
+                               int as_type, struct peer_group *group,
+                               bool config_node);
 extern struct peer *peer_create_accept(struct bgp *);
 extern void peer_xfer_config(struct peer *dst, struct peer *src);
 extern char *peer_uptime(time_t uptime2, char *buf, size_t len, bool use_json,