From 534db980a20763a7c66ab7b0e2451d615af01396 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 2 Dec 2022 12:59:30 -0500 Subject: [PATCH] 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 --- bgpd/bgp_network.c | 4 +--- bgpd/bgp_vty.c | 2 +- bgpd/bgpd.c | 20 +++++++++++--------- bgpd/bgpd.h | 6 ++++-- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index fa618232f6..7186a50711 100644 --- 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 1f532d4990..90182be85f 100644 --- 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 992306773b..36ff8866e6 100644 --- 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 -' 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 b7a329fdcd..a75bfdf746 100644 --- 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, -- 2.39.5