From 63fec93665d57c886840b6eaedc66fb323e5562b Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 14 Feb 2025 07:55:09 -0500 Subject: [PATCH] bgpd: When removing the prefix list drop the pointer We are very very rarely seeing this crash: 0 0x7f36ba48e389 in prefix_list_apply_ext lib/plist.c:789 1 0x55eff3fa4126 in subgroup_announce_check bgpd/bgp_route.c:2334 2 0x55eff3fa858e in subgroup_process_announce_selected bgpd/bgp_route.c:3440 3 0x55eff4016488 in subgroup_announce_table bgpd/bgp_updgrp_adv.c:808 4 0x55eff401664e in subgroup_announce_route bgpd/bgp_updgrp_adv.c:861 5 0x55eff40111df in peer_af_announce_route bgpd/bgp_updgrp.c:2223 6 0x55eff3f884cb in bgp_announce_route_timer_expired bgpd/bgp_route.c:5892 7 0x7f36ba4ec239 in event_call lib/event.c:2019 8 0x7f36ba41a22a in frr_run lib/libfrr.c:1295 9 0x55eff3e668b7 in main bgpd/bgp_main.c:557 10 0x7f36b9e2d249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 11 0x7f36b9e2d304 in __libc_start_main_impl ../csu/libc-start.c:360 12 0x55eff3e64a30 in _start (/home/ci/cibuild.1407/frr-source/bgpd/.libs/bgpd+0x2fda30) 0x608000037038 is located 24 bytes inside of 88-byte region [0x608000037020,0x608000037078) freed by thread T0 here: 0 0x7f36ba8b76a8 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52 1 0x7f36ba439bd7 in qfree lib/memory.c:131 2 0x7f36ba48d3a3 in prefix_list_free lib/plist.c:156 3 0x7f36ba48d3a3 in prefix_list_delete lib/plist.c:247 4 0x7f36ba48fbef in prefix_bgp_orf_remove_all lib/plist.c:1516 5 0x55eff3f679c4 in bgp_route_refresh_receive bgpd/bgp_packet.c:2841 6 0x55eff3f70bab in bgp_process_packet bgpd/bgp_packet.c:4069 7 0x7f36ba4ec239 in event_call lib/event.c:2019 8 0x7f36ba41a22a in frr_run lib/libfrr.c:1295 9 0x55eff3e668b7 in main bgpd/bgp_main.c:557 10 0x7f36b9e2d249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 previously allocated by thread T0 here: 0 0x7f36ba8b83b7 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77 1 0x7f36ba4392e4 in qcalloc lib/memory.c:106 2 0x7f36ba48d0de in prefix_list_new lib/plist.c:150 3 0x7f36ba48d0de in prefix_list_insert lib/plist.c:186 4 0x7f36ba48d0de in prefix_list_get lib/plist.c:204 5 0x7f36ba48f9df in prefix_bgp_orf_set lib/plist.c:1479 6 0x55eff3f67ba6 in bgp_route_refresh_receive bgpd/bgp_packet.c:2920 7 0x55eff3f70bab in bgp_process_packet bgpd/bgp_packet.c:4069 8 0x7f36ba4ec239 in event_call lib/event.c:2019 9 0x7f36ba41a22a in frr_run lib/libfrr.c:1295 10 0x55eff3e668b7 in main bgpd/bgp_main.c:557 11 0x7f36b9e2d249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 Let's just stop trying to save the pointer around in the peer->orf_plist data structure. There are other design problems but at least lets stop the crash from possibly happening. Fixes: #18138 Signed-off-by: Donald Sharp (cherry picked from commit 3d43d7b78971520854903c11b6aec23754fdca34) --- bgpd/bgp_packet.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index a7004725e598..d6e6afd90628 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -2599,6 +2599,8 @@ static int bgp_route_refresh_receive(struct peer_connection *connection, "%pBP rcvd Remove-All pfxlist ORF request", peer); prefix_bgp_orf_remove_all(afi, name); + peer->orf_plist[afi][safi] = prefix_bgp_orf_lookup(afi, + name); break; }