From 567832f2abd39a4a71252850e9cbad1f2c91b9e6 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sat, 1 Jul 2023 11:18:06 -0400 Subject: [PATCH] ospf6d: Fix crash because neighbor structure was freed The loading_done event needs a event pointer to prevent use after free's. Testing found this: ERROR: AddressSanitizer: heap-use-after-free on address 0x613000035130 at pc 0x55ad42d54e5f bp 0x7ffff1e942a0 sp 0x7ffff1e94290 READ of size 1 at 0x613000035130 thread T0 #0 0x55ad42d54e5e in loading_done ospf6d/ospf6_neighbor.c:447 #1 0x55ad42ed7be4 in event_call lib/event.c:1995 #2 0x55ad42e1df75 in frr_run lib/libfrr.c:1213 #3 0x55ad42cf332e in main ospf6d/ospf6_main.c:250 #4 0x7f5798133c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86) #5 0x55ad42cf2b19 in _start (/usr/lib/frr/ospf6d+0x248b19) 0x613000035130 is located 48 bytes inside of 384-byte region [0x613000035100,0x613000035280) freed by thread T0 here: #0 0x7f57998d77a8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7a8) #1 0x55ad42e3b4b6 in qfree lib/memory.c:130 #2 0x55ad42d5d049 in ospf6_neighbor_delete ospf6d/ospf6_neighbor.c:180 #3 0x55ad42d1e1ea in interface_down ospf6d/ospf6_interface.c:930 #4 0x55ad42ed7be4 in event_call lib/event.c:1995 #5 0x55ad42ed84fe in _event_execute lib/event.c:2086 #6 0x55ad42d26d7b in ospf6_interface_clear ospf6d/ospf6_interface.c:2847 #7 0x55ad42d73f16 in ospf6_process_reset ospf6d/ospf6_top.c:755 #8 0x55ad42d7e98c in clear_router_ospf6_magic ospf6d/ospf6_top.c:778 #9 0x55ad42d7e98c in clear_router_ospf6 ospf6d/ospf6_top_clippy.c:42 #10 0x55ad42dc2665 in cmd_execute_command_real lib/command.c:994 #11 0x55ad42dc2b32 in cmd_execute_command lib/command.c:1053 #12 0x55ad42dc2fa9 in cmd_execute lib/command.c:1221 #13 0x55ad42ee3cd6 in vty_command lib/vty.c:591 #14 0x55ad42ee4170 in vty_execute lib/vty.c:1354 #15 0x55ad42eec94f in vtysh_read lib/vty.c:2362 #16 0x55ad42ed7be4 in event_call lib/event.c:1995 #17 0x55ad42e1df75 in frr_run lib/libfrr.c:1213 #18 0x55ad42cf332e in main ospf6d/ospf6_main.c:250 #19 0x7f5798133c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86) previously allocated by thread T0 here: #0 0x7f57998d7d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28) #1 0x55ad42e3ab22 in qcalloc lib/memory.c:105 #2 0x55ad42d5c8ff in ospf6_neighbor_create ospf6d/ospf6_neighbor.c:119 #3 0x55ad42d4c86a in ospf6_hello_recv ospf6d/ospf6_message.c:464 #4 0x55ad42d4c86a in ospf6_read_helper ospf6d/ospf6_message.c:1884 #5 0x55ad42d4c86a in ospf6_receive ospf6d/ospf6_message.c:1925 #6 0x55ad42ed7be4 in event_call lib/event.c:1995 #7 0x55ad42e1df75 in frr_run lib/libfrr.c:1213 #8 0x55ad42cf332e in main ospf6d/ospf6_main.c:250 #9 0x7f5798133c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86) Add an actual event pointer and just track it appropriately. Signed-off-by: Donald Sharp --- ospf6d/ospf6_message.c | 3 ++- ospf6d/ospf6_neighbor.c | 4 +++- ospf6d/ospf6_neighbor.h | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ospf6d/ospf6_message.c b/ospf6d/ospf6_message.c index c17d38713241..fe70b2499d4f 100644 --- a/ospf6d/ospf6_message.c +++ b/ospf6d/ospf6_message.c @@ -2533,7 +2533,8 @@ void ospf6_lsreq_send(struct thread *thread) /* schedule loading_done if request list is empty */ if (on->request_list->count == 0) { - thread_add_event(master, loading_done, on, 0, NULL); + thread_add_event(master, loading_done, on, 0, + &on->event_loading_done); return; } diff --git a/ospf6d/ospf6_neighbor.c b/ospf6d/ospf6_neighbor.c index 06fccdb14736..6de0b086ebed 100644 --- a/ospf6d/ospf6_neighbor.c +++ b/ospf6d/ospf6_neighbor.c @@ -188,6 +188,7 @@ void ospf6_neighbor_delete(struct ospf6_neighbor *on) THREAD_OFF(on->thread_send_lsack); THREAD_OFF(on->thread_exchange_done); THREAD_OFF(on->thread_adj_ok); + THREAD_OFF(on->event_loading_done); THREAD_OFF(on->gr_helper_info.t_grace_timer); @@ -443,7 +444,8 @@ void ospf6_check_nbr_loading(struct ospf6_neighbor *on) if ((on->state == OSPF6_NEIGHBOR_LOADING) || (on->state == OSPF6_NEIGHBOR_EXCHANGE)) { if (on->request_list->count == 0) - thread_add_event(master, loading_done, on, 0, NULL); + thread_add_event(master, loading_done, on, 0, + &on->event_loading_done); else if (on->last_ls_req == NULL) { THREAD_OFF(on->thread_send_lsreq); thread_add_event(master, ospf6_lsreq_send, on, 0, diff --git a/ospf6d/ospf6_neighbor.h b/ospf6d/ospf6_neighbor.h index ea4d4241528f..c78c759b513a 100644 --- a/ospf6d/ospf6_neighbor.h +++ b/ospf6d/ospf6_neighbor.h @@ -138,6 +138,7 @@ struct ospf6_neighbor { struct thread *thread_send_lsack; struct thread *thread_exchange_done; struct thread *thread_adj_ok; + struct event *event_loading_done; /* BFD information */ struct bfd_session_params *bfd_session;