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>
(cherry picked from commit f83431c7e8767abc01b3bc2c9a98bd712b55b67f)

diff --git a/lib/bfd.c b/lib/bfd.c
index 29b8d85d8d3a28c042af2d6336d5647eb6ab711a..ec135245d716adf2cd8a4ca1a13200250added22 100644 (file)
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -508,13 +508,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);