Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bgpd: Prevent USE case for conditional advertisements with set actions #14845

Merged

Conversation

ton31337
Copy link
Member

No description provided.

Just to make sure we don't crash bgpd with double-free if an existing route
already exists.

Signed-off-by: Donatas Abraitis <[email protected]>
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 <[email protected]>
@frrbot frrbot bot added bgp tests Topotests, make check, etc labels Nov 21, 2023
@ton31337
Copy link
Member Author

ton31337 commented Nov 21, 2023

@Mergifyio backport stable/9.1 stable/9.0

Copy link

mergify bot commented Nov 21, 2023

backport stable/9.1

🟠 Waiting for conditions to match

  • merged [:pushpin: backport requirement]

@ton31337
Copy link
Member Author

ci:rerun not all tests started up

Copy link

mergify bot commented Nov 21, 2023

backport stable/9.1 stable/9.0

✅ Backports have been created

@donaldsharp donaldsharp merged commit b41f7b1 into FRRouting:master Nov 21, 2023
79 checks passed
donaldsharp added a commit that referenced this pull request Nov 21, 2023
bgpd: Prevent USE case for conditional advertisements with `set` actions (backport #14845)
ton31337 added a commit that referenced this pull request Nov 22, 2023
bgpd: Prevent USE case for conditional advertisements with `set` actions (backport #14845)
@ton31337 ton31337 deleted the fix/bgp_conditional_double_free branch November 22, 2023 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants