]> git.puffer.fish Git - mirror/frr.git/commitdiff
bgpd: Flush attrs only if we don't have to announce a conditional route 14854/head
authorDonatas Abraitis <donatas@opensourcerouting.org>
Tue, 21 Nov 2023 08:40:58 +0000 (10:40 +0200)
committerMergify <37929162+mergify[bot]@users.noreply.github.com>
Tue, 21 Nov 2023 15:50:13 +0000 (15:50 +0000)
To avoid USE:

```
==587645==ERROR: AddressSanitizer: heap-use-after-free on address 0x604000074050 at pc 0x55b34337d96c bp 0x7ffda59bb4c0 sp 0x7ffda59bb4b0
READ of size 8 at 0x604000074050 thread T0
    0 0x55b34337d96b in bgp_attr_flush bgpd/bgp_attr.c:1289
    1 0x55b34368ef85 in bgp_conditional_adv_routes bgpd/bgp_conditional_adv.c:111
    2 0x55b34368ff58 in bgp_conditional_adv_timer bgpd/bgp_conditional_adv.c:301
    3 0x7f7d41cdf81c in event_call lib/event.c:1980
    4 0x7f7d41c1da37 in frr_run lib/libfrr.c:1214
    5 0x55b343371e22 in main bgpd/bgp_main.c:510
    6 0x7f7d41517082 in __libc_start_main ../csu/libc-start.c:308
    7 0x55b3433769fd in _start (/usr/lib/frr/bgpd+0x2e29fd)

0x604000074050 is located 0 bytes inside of 40-byte region [0x604000074050,0x604000074078)
freed by thread T0 here:
    #0 0x7f7d4207540f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:122
    1 0x55b343396afd in community_free bgpd/bgp_community.c:41
    2 0x55b343396afd in community_free bgpd/bgp_community.c:28
    3 0x55b343397373 in community_intern bgpd/bgp_community.c:458
    4 0x55b34337bed4 in bgp_attr_intern bgpd/bgp_attr.c:967
    5 0x55b34368165b in bgp_advertise_attr_intern bgpd/bgp_advertise.c:106
    6 0x55b3435277d7 in bgp_adj_out_set_subgroup bgpd/bgp_updgrp_adv.c:587
    7 0x55b34368f36b in bgp_conditional_adv_routes bgpd/bgp_conditional_adv.c:125
    8 0x55b34368ff58 in bgp_conditional_adv_timer bgpd/bgp_conditional_adv.c:301
    9 0x7f7d41cdf81c in event_call lib/event.c:1980
    10 0x7f7d41c1da37 in frr_run lib/libfrr.c:1214
    11 0x55b343371e22 in main bgpd/bgp_main.c:510
    12 0x7f7d41517082 in __libc_start_main ../csu/libc-start.c:308

previously allocated by thread T0 here:
    #0 0x7f7d42075a06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153
    1 0x7f7d41c3c28e in qcalloc lib/memory.c:105
    2 0x55b3433976e8 in community_dup bgpd/bgp_community.c:514
    3 0x55b34350273a in route_set_community bgpd/bgp_routemap.c:2589
    4 0x7f7d41c96c06 in route_map_apply_ext lib/routemap.c:2690
    5 0x55b34368f2d8 in bgp_conditional_adv_routes bgpd/bgp_conditional_adv.c:107
    6 0x55b34368ff58 in bgp_conditional_adv_timer bgpd/bgp_conditional_adv.c:301
    7 0x7f7d41cdf81c in event_call lib/event.c:1980
    8 0x7f7d41c1da37 in frr_run lib/libfrr.c:1214
    9 0x55b343371e22 in main bgpd/bgp_main.c:510
    10 0x7f7d41517082 in __libc_start_main ../csu/libc-start.c:308
```

And also a crash:

```
(gdb) bt
0  raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
1  0x00007ff3b7048ce0 in core_handler (signo=6, siginfo=0x7ffc8cf724b0, context=<optimized out>)
    at lib/sigevent.c:246
2  <signal handler called>
3  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
4  0x00007ff3b6bb8859 in __GI_abort () at abort.c:79
5  0x00007ff3b6c2326e in __libc_message (action=action@entry=do_abort,
    fmt=fmt@entry=0x7ff3b6d4d298 "%s\n") at ../sysdeps/posix/libc_fatal.c:155
6  0x00007ff3b6c2b2fc in malloc_printerr (
    str=str@entry=0x7ff3b6d4f628 "double free or corruption (fasttop)") at malloc.c:5347
7  0x00007ff3b6c2cc65 in _int_free (av=0x7ff3b6d82b80 <main_arena>, p=0x555c8fa70a10, have_lock=0)
    at malloc.c:4266
8  0x0000555c8da94bd3 in community_free (com=0x7ffc8cf72e70) at bgpd/bgp_community.c:41
9  community_free (com=com@entry=0x7ffc8cf72e70) at bgpd/bgp_community.c:28
10 0x0000555c8da8afc1 in bgp_attr_flush (attr=attr@entry=0x7ffc8cf73040) at bgpd/bgp_attr.c:1290
11 0x0000555c8dbc0760 in bgp_conditional_adv_routes (peer=peer@entry=0x555c8fa627c0,
    afi=afi@entry=AFI_IP, safi=SAFI_UNICAST, table=table@entry=0x555c8fa510b0, rmap=0x555c8fa71cb0,
    update_type=UPDATE_TYPE_ADVERTISE) at bgpd/bgp_conditional_adv.c:111
12 0x0000555c8dbc0b75 in bgp_conditional_adv_timer (t=<optimized out>)
    at bgpd/bgp_conditional_adv.c:301
13 0x00007ff3b705b84c in event_call (thread=thread@entry=0x7ffc8cf73440) at lib/event.c:1980
14 0x00007ff3b700bf98 in frr_run (master=0x555c8f27c090) at lib/libfrr.c:1214
15 0x0000555c8da85f05 in main (argc=<optimized out>, argv=0x7ffc8cf736a8) at bgpd/bgp_main.c:510
```

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
(cherry picked from commit d410587bab67d1d492f724c6111ff7238a78e7b2)

bgpd/bgp_conditional_adv.c

index 6685a4f447877ed6459a5f7c3805f5f4b05b01e3..ff805cb70a97cec8b94c491275ad69d0c90ff457 100644 (file)
@@ -141,8 +141,9 @@ static void bgp_conditional_adv_routes(struct peer *peer, afi_t afi,
                                        bgp_addpath_id_for_peer(
                                                peer, afi, safi,
                                                &pi->tx_addpath));
+
+                               bgp_attr_flush(&advmap_attr);
                        }
-                       bgp_attr_flush(&advmap_attr);
                }
        }
        UNSET_FLAG(subgrp->sflags, SUBGRP_STATUS_TABLE_REPARSING);