bgpd: Fix several use after free's in bgp for the peer

Three fixes:

a) When calling bgp_fsm_change_status with `Deleted` do
not add a new event to the peer's t_event because
we are already in the process of deleting everything

b) When bgp_stop decides to delete a peer return a notification
that it is happening to bgp_event_update so that it does not
set the peer state back to idle or do other processing.

c) bgp_event_update can cause a peer deletion, because
the peer can be deleted in the fsm function but the peer
data structure is still pointed to and used after words.
So lock the peer before entering and prevent a use after
free.

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

diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c
index fcb7275c34100533562fdb43afe960d7818db320..db68c918c05ce6a62d9a7084eace21e5cf3c2cdc 100644 (file)
--- a/bgpd/bgp_fsm.c
+++ b/bgpd/bgp_fsm.c
@@ -1293,7 +1293,8 @@ void bgp_fsm_change_status(struct peer *peer, int status)
                 * Clearing
                 * (or Deleted).
                 */
-               if (!work_queue_is_scheduled(peer->clear_node_queue))
+               if (!work_queue_is_scheduled(peer->clear_node_queue) &&
+                   status != Deleted)
                        BGP_EVENT_ADD(peer, Clearing_Completed);
        }
 
@@ -1375,7 +1376,7 @@ int bgp_stop(struct peer *peer)
                        zlog_debug("%s (dynamic neighbor) deleted (%s)",
                                   peer->host, __func__);
                peer_delete(peer);
-               return -1;
+               return -2;
        }
 
        /* Can't do this in Clearing; events are used for state transitions */
@@ -1584,7 +1585,7 @@ int bgp_stop(struct peer *peer)
        if (!CHECK_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE)
            && !(CHECK_FLAG(peer->flags, PEER_FLAG_DELETE))) {
                peer_delete(peer);
-               ret = -1;
+               ret = -2;
        } else {
                bgp_peer_conf_if_to_su_update(peer);
        }
@@ -2571,7 +2572,9 @@ void bgp_event(struct thread *thread)
        peer = THREAD_ARG(thread);
        event = THREAD_VAL(thread);
 
+       peer_lock(peer);
        bgp_event_update(peer, event);
+       peer_unlock(peer);
 }
 
 int bgp_event_update(struct peer *peer, enum bgp_fsm_events event)
@@ -2641,7 +2644,7 @@ int bgp_event_update(struct peer *peer, enum bgp_fsm_events event)
                 * we need to indicate that the peer was stopped in the return
                 * code.
                 */
-               if (!dyn_nbr && !passive_conn && peer->bgp) {
+               if (!dyn_nbr && !passive_conn && peer->bgp && ret != -2) {
                        flog_err(
                                EC_BGP_FSM,
                                "%s [FSM] Failure handling event %s in state %s, prior events %s, %s, fd %d",