bgpd: fix bgp_best_selection heap-use-after-free

Fix bgp_best_selection heap-use-after-free

> ==2521540==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d000032810 at pc 0x000000716f45 bp 0x7ffedc6229d0 sp 0x7ffedc6229c8
> READ of size 8 at 0x60d000032810 thread T0
>     #0 0x716f44 in bgp_best_selection /home/lscalber/git/frr/bgpd/bgp_route.c:2834:5
>     #1 0x71a05e in bgp_process_main_one /home/lscalber/git/frr/bgpd/bgp_route.c:3344:2
>     #2 0x71c265 in bgp_process_wq /home/lscalber/git/frr/bgpd/bgp_route.c:3622:3
>     #3 0x7fe630a6669c in work_queue_run /home/lscalber/git/frr/lib/workqueue.c:282:10
>     #4 0x7fe630a294e2 in event_call /home/lscalber/git/frr/lib/event.c:1974:2
>     #5 0x7fe630898f3f in frr_run /home/lscalber/git/frr/lib/libfrr.c:1214:3
>     #6 0x4f4ace in main /home/lscalber/git/frr/bgpd/bgp_main.c:510:2
>     #7 0x7fe63018bd09 in __libc_start_main csu/../csu/libc-start.c:308:16
>     #8 0x449629 in _start (/usr/lib/frr/bgpd+0x449629)
>
> 0x60d000032810 is located 48 bytes inside of 144-byte region [0x60d0000327e0,0x60d000032870)
> freed by thread T0 here:
>     #0 0x4c341d in free (/usr/lib/frr/bgpd+0x4c341d)
>     #1 0x7fe6308d7420 in qfree /home/lscalber/git/frr/lib/memory.c:130:2
>     #2 0x702632 in bgp_path_info_free_with_caller /home/lscalber/git/frr/bgpd/bgp_route.c:300:2
>     #3 0x702023 in bgp_path_info_unlock /home/lscalber/git/frr/bgpd/bgp_route.c:315:3
>     #4 0x703bc6 in bgp_path_info_reap /home/lscalber/git/frr/bgpd/bgp_route.c:461:2
>     #5 0x716e5d in bgp_best_selection /home/lscalber/git/frr/bgpd/bgp_route.c:2829:12
>     #6 0x71a05e in bgp_process_main_one /home/lscalber/git/frr/bgpd/bgp_route.c:3344:2
>     #7 0x71c265 in bgp_process_wq /home/lscalber/git/frr/bgpd/bgp_route.c:3622:3
>     #8 0x7fe630a6669c in work_queue_run /home/lscalber/git/frr/lib/workqueue.c:282:10
>     #9 0x7fe630a294e2 in event_call /home/lscalber/git/frr/lib/event.c:1974:2
>     #10 0x7fe630898f3f in frr_run /home/lscalber/git/frr/lib/libfrr.c:1214:3
>     #11 0x4f4ace in main /home/lscalber/git/frr/bgpd/bgp_main.c:510:2
>     #12 0x7fe63018bd09 in __libc_start_main csu/../csu/libc-start.c:308:16
>
> previously allocated by thread T0 here:
>     #0 0x4c3812 in calloc (/usr/lib/frr/bgpd+0x4c3812)
>     #1 0x7fe6308d7178 in qcalloc /home/lscalber/git/frr/lib/memory.c:105:27
>     #2 0x71f5b4 in info_make /home/lscalber/git/frr/bgpd/bgp_route.c:3985:8
>     #3 0x725293 in bgp_update /home/lscalber/git/frr/bgpd/bgp_route.c:4881:8
>     #4 0x73083d in bgp_nlri_parse_ip /home/lscalber/git/frr/bgpd/bgp_route.c:6230:4
>     #5 0x6ba980 in bgp_nlri_parse /home/lscalber/git/frr/bgpd/bgp_packet.c:341:10
>     #6 0x6cca2a in bgp_update_receive /home/lscalber/git/frr/bgpd/bgp_packet.c:2412:15
>     #7 0x6c6788 in bgp_process_packet /home/lscalber/git/frr/bgpd/bgp_packet.c:3887:11
>     #8 0x7fe630a294e2 in event_call /home/lscalber/git/frr/lib/event.c:1974:2
>     #9 0x7fe630898f3f in frr_run /home/lscalber/git/frr/lib/libfrr.c:1214:3
>     #10 0x4f4ace in main /home/lscalber/git/frr/bgpd/bgp_main.c:510:2
>     #11 0x7fe63018bd09 in __libc_start_main csu/../csu/libc-start.c:308:16

Fixes: ddb5b4880b ("bgpd: vpn-vrf route leaking")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 02aee9c81cafbb9030ac65c0f9d318799bfb5170..0885c9f1a9d682efadee478f88bfb5f6c0134f6e 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -2758,16 +2758,16 @@ void bgp_best_selection(struct bgp *bgp, struct bgp_dest *dest,
                        /* reap REMOVED routes, if needs be
                         * selected route must stay for a while longer though
                         */
-                       if (CHECK_FLAG(pi->flags, BGP_PATH_REMOVED)
-                           && (pi != old_select))
-                               bgp_path_info_reap(dest, pi);
-
                        if (debug)
                                zlog_debug(
                                        "%s: %pBD(%s) pi from %s in holddown",
                                        __func__, dest, bgp->name_pretty,
                                        pi->peer->host);
 
+                       if (CHECK_FLAG(pi->flags, BGP_PATH_REMOVED) &&
+                           (pi != old_select))
+                               bgp_path_info_reap(dest, pi);
+
                        continue;
                }