AddressSanitizer: heap-use-after-free lib/timer.h:61 in tm_active

[Zagorski, Michal]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3556 bytes
Desc: not available
URL: <http://trubka.network.cz/pipermail/bird-users/attachments/20230526/d0200220/attachment.p7s>