Skip to content

Commit

Permalink
bgpd: Fix several use after free's in bgp for the peer
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
donaldsharp committed Feb 23, 2023
1 parent 1747e97 commit 5a2196f
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions bgpd/bgp_fsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 5a2196f

Please sign in to comment.