From 5a2196f0494bc7fcefcace4a4f5bc69da6cd511d Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 1 Dec 2022 13:12:40 -0500 Subject: [PATCH] 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 --- bgpd/bgp_fsm.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 3b49f4f93607..09cdfb558a53 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -1292,7 +1292,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); } @@ -1372,7 +1373,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 */ @@ -1581,7 +1582,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); } @@ -2565,7 +2566,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) @@ -2635,7 +2638,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",