lib: on bfd peer shutdown actually stop event

When deleting a bfd peer during shutdown, let's ensure
that any scheduled events are actually stopped.

==7759== Invalid read of size 4
==7759==    at 0x48BF700: _bfd_sess_valid (bfd.c:419)
==7759==    by 0x48BF700: _bfd_sess_send (bfd.c:470)
==7759==    by 0x492F79C: thread_call (thread.c:2008)
==7759==    by 0x48E9BD7: frr_run (libfrr.c:1223)
==7759==    by 0x1C739B: main (bgp_main.c:550)
==7759==  Address 0xfb687a4 is 4 bytes inside a block of size 272 free'd
==7759==    at 0x48369AB: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==7759==    by 0x48BFA5A: bfd_sess_free (bfd.c:535)
==7759==    by 0x2B7034: bgp_peer_remove_bfd (bgp_bfd.c:339)
==7759==    by 0x29FF8A: peer_free (bgpd.c:1160)
==7759==    by 0x29FF8A: peer_unlock_with_caller (bgpd.c:1192)
==7759==    by 0x2A0506: peer_delete (bgpd.c:2633)
==7759==    by 0x208190: bgp_stop (bgp_fsm.c:1639)
==7759==    by 0x20C082: bgp_event_update (bgp_fsm.c:2751)
==7759==    by 0x492F79C: thread_call (thread.c:2008)
==7759==    by 0x48E9BD7: frr_run (libfrr.c:1223)
==7759==    by 0x1C739B: main (bgp_main.c:550)
==7759==  Block was alloc'd at
==7759==    at 0x4837B65: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==7759==    by 0x48F53AF: qcalloc (memory.c:116)
==7759==    by 0x48BF98D: bfd_sess_new (bfd.c:397)
==7759==    by 0x2B76DC: bgp_peer_configure_bfd (bgp_bfd.c:298)
==7759==    by 0x2B76DC: bgp_peer_configure_bfd (bgp_bfd.c:279)
==7759==    by 0x29BA06: peer_group2peer_config_copy (bgpd.c:2803)
==7759==    by 0x2A3D96: peer_create_bind_dynamic_neighbor (bgpd.c:4107)
==7759==    by 0x2A4195: peer_lookup_dynamic_neighbor (bgpd.c:4239)
==7759==    by 0x21AB72: bgp_accept (bgp_network.c:422)
==7759==    by 0x492F79C: thread_call (thread.c:2008)
==7759==    by 0x48E9BD7: frr_run (libfrr.c:1223)
==7759==    by 0x1C739B: main (bgp_main.c:550)

tl;dr -> Effectively, in this test setup we have 300 dynamic bgp
sessions all of which are using bfd.  When a peer collision is detected
or we remove the peers, if an event has been scheduled but not actually
executed yet the event event was not actually being stopped, leaving
the bsp pointer on the thread->arg and causing a crash when it is
executed.

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

diff --git a/lib/bfd.c b/lib/bfd.c
index 0d77c5c822de8ba620d86ec6565ce0932398443b..c1e0fff7f578ed58ef95b1e934f5377dd6a66350 100644 (file)
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -532,13 +532,13 @@ static void _bfd_sess_send(struct thread *t)
 
 static void _bfd_sess_remove(struct bfd_session_params *bsp)
 {
+       /* Cancel any pending installation request. */
+       THREAD_OFF(bsp->installev);
+
        /* Not installed, nothing to do. */
        if (!bsp->installed)
                return;
 
-       /* Cancel any pending installation request. */
-       THREAD_OFF(bsp->installev);
-
        /* Send request to remove any session. */
        bsp->lastev = BSE_UNINSTALL;
        thread_execute(bsglobal.tm, _bfd_sess_send, bsp, 0);