Hello,
During stress testing I found invalid read within bgp.
I use gcc's Address Sanitizer, building with config:
CFLAGS="-D_FORTIFY_SOURCE=2 -fsanitize=address -fno-omit-frame-pointer
-ggdb3 -g3 -O0" LDFLAGS="-fsanitize=address -fno-omit-frame-pointer"
LIBS="-lasan" ./configure --with-protocols="bgp static"
When breaking session links and querying "show protocols all" after expired
hold timer, it hits with report:
=================================================================
==3476100==ERROR: AddressSanitizer: heap-use-after-free on address
0x606000245508 at pc 0x56497e7af450 bp 0x7ffcae09e400 sp 0x7ffcae09e3f8
READ of size 8 at 0x606000245508 thread T0
#0 0x56497e7af44f in tm_active lib/timer.h:61
#1 0x56497e7c4e12 in bgp_show_proto_info proto/bgp/bgp.c:2624
#2 0x56497e76cb40 in proto_cmd_show nest/proto.c:2082
#3 0x56497e76d9bb in proto_apply_cmd_patt nest/proto.c:2232
#4 0x56497e76db9c in proto_apply_cmd nest/proto.c:2248
#5 0x56497e6762e6 in cf_parse nest/config.Y:623
#6 0x56497e69089c in cli_parse conf/conf.c:180
#7 0x56497e751001 in cli_command nest/cli.c:273
#8 0x56497e751273 in cli_event nest/cli.c:302
#9 0x56497e71f7a6 in ev_run lib/event.c:86
#10 0x56497e71f9af in ev_run_list lib/event.c:159
#11 0x56497e813e34 in io_loop sysdep/unix/io.c:2216
#12 0x56497e820b45 in main sysdep/unix/main.c:958
#13 0x7fa940846189 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
#14 0x7fa940846244 in __libc_start_main_impl ../csu/libc-start.c:381
#15 0x56497e65c940 in _start (/home/mzagorsk/git/cz-bird/bird+0x48940)
0x606000245508 is located 40 bytes inside of 64-byte region
[0x6060002454e0,0x606000245520)
freed by thread T0 here:
#0 0x7fa940ab76a8 in __interceptor_free
../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
#1 0x56497e7376a5 in pool_free lib/resource.c:97
#2 0x56497e737c5f in rfree lib/resource.c:187
#3 0x56497e76b2bc in proto_do_down nest/proto.c:1885
#4 0x56497e76b56e in proto_notify_state nest/proto.c:1951
#5 0x56497e7b1b71 in bgp_down proto/bgp/bgp.c:475
#6 0x56497e7b1d8d in bgp_decision proto/bgp/bgp.c:493
#7 0x56497e71f7a6 in ev_run lib/event.c:86
#8 0x56497e71f9af in ev_run_list lib/event.c:159
#9 0x56497e813e34 in io_loop sysdep/unix/io.c:2216
#10 0x56497e820b45 in main sysdep/unix/main.c:958
#11 0x7fa940846189 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
previously allocated by thread T0 here:
#0 0x7fa940ab89cf in __interceptor_malloc
../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x56497e7461d4 in bird_xmalloc lib/xmalloc.c:29
#2 0x56497e73800b in ralloc lib/resource.c:245
#3 0x56497e7434df in tm_new lib/timer.c:133
#4 0x56497e7af4e6 in tm_new_init lib/timer.h:74
#5 0x56497e7bcad7 in bgp_channel_start proto/bgp/bgp.c:1796
#6 0x56497e7632da in channel_do_start nest/proto.c:560
#7 0x56497e763c89 in channel_set_state nest/proto.c:658
#8 0x56497e760cb6 in proto_start_channels nest/proto.c:217
#9 0x56497e76b081 in proto_do_up nest/proto.c:1853
#10 0x56497e76b504 in proto_notify_state nest/proto.c:1938
#11 0x56497e7b427c in bgp_conn_enter_established_state
proto/bgp/bgp.c:691
#12 0x56497e7e217a in bgp_rx_keepalive proto/bgp/packets.c:3412
#13 0x56497e7e2445 in bgp_rx_packet proto/bgp/packets.c:3445
#14 0x56497e7e2683 in bgp_rx proto/bgp/packets.c:3488
#15 0x56497e8122e2 in call_rx_hook sysdep/unix/io.c:1800
#16 0x56497e8127cb in sk_read_noflush sysdep/unix/io.c:1888
#17 0x56497e812a2b in sk_read sysdep/unix/io.c:1923
#18 0x56497e8148dd in io_loop sysdep/unix/io.c:2323
#19 0x56497e820b45 in main sysdep/unix/main.c:958
#20 0x7fa940846189 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
SUMMARY: AddressSanitizer: heap-use-after-free lib/timer.h:61 in tm_active
Shadow bytes around the buggy address:
0x0c0c80040a50: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
0x0c0c80040a60: fd fd fd fd fd fd fd fa fa fa fa fa fd fd fd fd
0x0c0c80040a70: fd fd fd fa fa fa fa fa fd fd fd fd fd fd fd fa
0x0c0c80040a80: fa fa fa fa fd fd fd fd fd fd fd fa fa fa fa fa
0x0c0c80040a90: fd fd fd fd fd fd fd fa fa fa fa fa fd fd fd fd
=>0x0c0c80040aa0: fd[fd]fd fd fa fa fa fa fd fd fd fd fd fd fd fa
0x0c0c80040ab0: fa fa fa fa fd fd fd fd fd fd fd fa fa fa fa fa
0x0c0c80040ac0: fd fd fd fd fd fd fd fa fa fa fa fa fd fd fd fd
0x0c0c80040ad0: fd fd fd fa fa fa fa fa fd fd fd fd fd fd fd fd
0x0c0c80040ae0: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
0x0c0c80040af0: fd fd fd fd fd fd fd fd fa fa fa fa fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==3476100==ABORTING
Without sanitizers, similar scenario wouldn't lead to visible crashes or
invalid output.
If I understand it correctly, after hold timers expired, protocol's memory
pool (p->pool) is freed from event with bgp_down.
On other event, "show protocols all" read channels' structures and got valid
pointer c->staled_timer, as timer is mostly deactivated, second
condition would fail.
--
Thanks,
Michał Zagórski
smime.p7s
Description: S/MIME cryptographic signature
