From 1aaf031b33a799aeab9ff2f87353b9bf7e3b624d Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 20 Mar 2023 16:07:20 -0400 Subject: [PATCH] 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 (cherry picked from commit f83431c7e8767abc01b3bc2c9a98bd712b55b67f) --- lib/bfd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/bfd.c b/lib/bfd.c index 29b8d85d8d..ec135245d7 100644 --- 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); -- 2.39.5