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.

Sign up for GitHub

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

Jump to bottom

heap-use-after-free when processing an RPC with no input and no outputs #1695

Open
jktjkt opened this issue Jan 29, 2025 · 4 comments
Open

heap-use-after-free when processing an RPC with no input and no outputs #1695

jktjkt opened this issue Jan 29, 2025 · 4 comments
Labels
is:bug Bug description. status:completed From the developer perspective, the issue was solved (bug fixed, question answered,...)

Comments

@jktjkt
Copy link
Contributor

jktjkt commented Jan 29, 2025

Under ASAN+UBSAN, with the latest devel of everything (CESNET/libyang@2766cdf96, CESNET/libnetconf2@c0bac65, sysrepo/sysrepo@5f1fd923 and d95c2e63) and the appropriate versions of the C++ bindings (CESNET/libyang-cpp@31131f2, CESNET/libnetconf2-cpp@0374395, sysrepo/sysrepo-cpp@b34fb20 aka refs/changes/32/8132/4), the unit tests of netconf-cli fail. The test in question, test_datastore_access_netconf, launches the netopeer2-server process in the background, connects via a local socket, and tests the CLI against that NETCONF server. Some sysrepo-level callbacks are in place.

The CTest process appears to take a looooooong time, and when it eventually fails, the following is at the end of test_netopeer_outputs/netconf-cli_test_datastore_access_netconf.out which is where the launched netopeer2-server logs its output:

==412260==ERROR: AddressSanitizer: heap-use-after-free on address 0x507000012938 at pc 0x7f9cdb6bbeb4 bp 0x7fff4f194a90 sp 0x7fff4f194a88
READ of size 8 at 0x507000012938 thread T0
    #0 0x7f9cdb6bbeb3 in nc_write_msg_io github/CESNET/libnetconf2/src/io.c:967:13
    #1 0x7f9cdb710230 in nc_server_send_reply_io github/CESNET/libnetconf2/src/session_server.c:1808:9
    #2 0x7f9cdb710230 in nc_ps_poll github/CESNET/libnetconf2/src/session_server.c:2191:20
    #3 0x55a34cdb836b in worker_thread github/CESNET/Netopeer2/src/main.c:988:14
    #4 0x55a34cdb72e7 in main github/CESNET/Netopeer2/src/main.c:1336:5
    #5 0x7f9cd9e3127d in __libc_start_call_main (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b)
    #6 0x7f9cd9e31338 in __libc_start_main@GLIBC_2.2.5 (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a338) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b)
    #7 0x55a34cc3e1a4 in _start (/home/jkt/work/prog/_build/czechlight-clang19-asan-ubsan/target/sbin/netopeer2-server+0x661a4)

0x507000012938 is located 8 bytes inside of 72-byte region [0x507000012930,0x507000012978)
freed by thread T0 here:
    #0 0x55a34cd2e5f8 in free.part.0 asan_malloc_linux.cpp.o
    #1 0x7f9cdaae2077 in lyd_free_ github/CESNET/libyang/src/tree_data_free.c:287:9
    #2 0x7f9cda158b06 in sr_release_data github/sysrepo/sysrepo/src/sysrepo.c:3363:5
    #3 0x55a34cdb89f8 in np2srv_rpc_cb github/CESNET/Netopeer2/src/main.c:320:5
    #4 0x7f9cdb710199 in nc_server_send_reply_io github/CESNET/libnetconf2/src/session_server.c
    #5 0x7f9cdb710199 in nc_ps_poll github/CESNET/libnetconf2/src/session_server.c:2191:20
    #6 0x55a34cdb836b in worker_thread github/CESNET/Netopeer2/src/main.c:988:14
    #7 0x55a34cdb72e7 in main github/CESNET/Netopeer2/src/main.c:1336:5
    #8 0x7f9cd9e3127d in __libc_start_call_main (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b)

previously allocated by thread T0 here:
    #0 0x55a34cd2ff0f in calloc (/home/jkt/work/prog/_build/czechlight-clang19-asan-ubsan/target/sbin/netopeer2-server+0x157f0f)
    #1 0x7f9cdab00f3d in lyd_create_inner github/CESNET/libyang/src/tree_data_new.c:118:10
    #2 0x7f9cdab59c47 in lyb_parse_node_inner github/CESNET/libyang/src/parser_lyb.c:1308:11
    #3 0x7f9cdab59c47 in lyb_parse_node github/CESNET/libyang/src/parser_lyb.c:1541:15
    #4 0x7f9cdab59c47 in lyb_parse_siblings github/CESNET/libyang/src/parser_lyb.c:1573:9
    #5 0x7f9cdab56ab1 in lyd_parse_lyb github/CESNET/libyang/src/parser_lyb.c:1718:10
    #6 0x7f9cdaa9ef4a in lyd_parse_op_ github/CESNET/libyang/src/tree_data.c:399:14
    #7 0x7f9cdaa9e86b in lyd_parse_op github/CESNET/libyang/src/tree_data.c:446:12
    #8 0x7f9cda1d1177 in sr_lyd_parse_op github/sysrepo/sysrepo/src/ly_wrap.c:517:9
    #9 0x7f9cda2c0b13 in sr_shmsub_rpc_notify github/sysrepo/sysrepo/src/shm_sub.c:2818:25
    #10 0x7f9cda187929 in _sr_rpc_send_tree github/sysrepo/sysrepo/src/sysrepo.c:6807:21
    #11 0x7f9cda18508c in sr_rpc_send_tree github/sysrepo/sysrepo/src/sysrepo.c:7083:20
    #12 0x55a34cdba5f8 in np2srv_rpc_cb github/CESNET/Netopeer2/src/main.c:307:14
    #13 0x7f9cdb710199 in nc_server_send_reply_io github/CESNET/libnetconf2/src/session_server.c
    #14 0x7f9cdb710199 in nc_ps_poll github/CESNET/libnetconf2/src/session_server.c:2191:20
    #15 0x55a34cdb836b in worker_thread github/CESNET/Netopeer2/src/main.c:988:14
    #16 0x55a34cdb72e7 in main github/CESNET/Netopeer2/src/main.c:1336:5
    #17 0x7f9cd9e3127d in __libc_start_call_main (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b)

SUMMARY: AddressSanitizer: heap-use-after-free github/CESNET/libnetconf2/src/io.c:967:13 in nc_write_msg_io

This used to work with 6d1cb61.
@jktjkt
Copy link
Contributor Author

jktjkt commented Jan 31, 2025

Here's how to reproduce with no C++ code:

  1. install https://github.com/CESNET/netconf-cli/blob/master/tests/example-schema.yang (it also requires a submodule other-module from the same path). The RPC has no input and no output:
rpc noop {}
  1. ./sysrepo/examples/rpc_subscribe_example /example-schema:noop
  2. run netopeer2-server -d -c MSG -U np.sock -p np.pid
  3. trigger that RPC via NETCONF:
<rpc xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" message-id="46"><noop xmlns="http://example.com"/></rpc>
  1. observe the crash:
[DBG]: LN: Session 1: Received message:
<rpc xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" message-id="46"><noop xmlns="http://example.com"/></rpc>

=================================================================
==1010861==ERROR: AddressSanitizer: heap-use-after-free on address 0x5070000d9568 at pc 0x7efef6cbbeb4 bp 0x7efeeeae96f0 sp 0x7efeeeae96e8
READ of size 8 at 0x5070000d9568 thread T4
    #0 0x7efef6cbbeb3 in nc_write_msg_io github/CESNET/libnetconf2/src/io.c:967:13
    #1 0x7efef6d10230 in nc_server_send_reply_io github/CESNET/libnetconf2/src/session_server.c:1808:9
    #2 0x7efef6d10230 in nc_ps_poll github/CESNET/libnetconf2/src/session_server.c:2191:20
    #3 0x562e6c32036b in worker_thread github/CESNET/Netopeer2/src/main.c:988:14
    #4 0x562e6c1ce663 in asan_thread_start(void*) asan_interceptors.cpp.o
    #5 0x7efef5497d01 in start_thread (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x90d01) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b)
    #6 0x7efef55173ab in __GI___clone3 (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x1103ab) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b)

0x5070000d9568 is located 8 bytes inside of 72-byte region [0x5070000d9560,0x5070000d95a8)
freed by thread T4 here:
    #0 0x562e6c2965f8 in free.part.0 asan_malloc_linux.cpp.o
    #1 0x7efef60e2077 in lyd_free_ github/CESNET/libyang/src/tree_data_free.c:287:9
    #2 0x7efef5758b16 in sr_release_data github/sysrepo/sysrepo/src/sysrepo.c:3363:5
    #3 0x562e6c3209f8 in np2srv_rpc_cb github/CESNET/Netopeer2/src/main.c:320:5
    #4 0x7efef6d10199 in nc_server_send_reply_io github/CESNET/libnetconf2/src/session_server.c
    #5 0x7efef6d10199 in nc_ps_poll github/CESNET/libnetconf2/src/session_server.c:2191:20
    #6 0x562e6c32036b in worker_thread github/CESNET/Netopeer2/src/main.c:988:14
    #7 0x562e6c1ce663 in asan_thread_start(void*) asan_interceptors.cpp.o

previously allocated by thread T4 here:
    #0 0x562e6c297f0f in calloc (/home/jkt/work/prog/_build/czechlight-clang19-asan-ubsan/target/sbin/netopeer2-server+0x157f0f)
    #1 0x7efef6100f3d in lyd_create_inner github/CESNET/libyang/src/tree_data_new.c:118:10
    #2 0x7efef6159e47 in lyb_parse_node_inner github/CESNET/libyang/src/parser_lyb.c:1308:11
    #3 0x7efef6159e47 in lyb_parse_node github/CESNET/libyang/src/parser_lyb.c:1541:15
    #4 0x7efef6159e47 in lyb_parse_siblings github/CESNET/libyang/src/parser_lyb.c:1573:9
    #5 0x7efef6156cb1 in lyd_parse_lyb github/CESNET/libyang/src/parser_lyb.c:1718:10
    #6 0x7efef609ef4a in lyd_parse_op_ github/CESNET/libyang/src/tree_data.c:399:14
    #7 0x7efef609e86b in lyd_parse_op github/CESNET/libyang/src/tree_data.c:446:12
    #8 0x7efef57d1197 in sr_lyd_parse_op github/sysrepo/sysrepo/src/ly_wrap.c:517:9
    #9 0x7efef58c0f57 in sr_shmsub_rpc_notify github/sysrepo/sysrepo/src/shm_sub.c:2842:25
    #10 0x7efef5787939 in _sr_rpc_send_tree github/sysrepo/sysrepo/src/sysrepo.c:6807:21
    #11 0x7efef578509c in sr_rpc_send_tree github/sysrepo/sysrepo/src/sysrepo.c:7083:20
    #12 0x562e6c3225f8 in np2srv_rpc_cb github/CESNET/Netopeer2/src/main.c:307:14
    #13 0x7efef6d10199 in nc_server_send_reply_io github/CESNET/libnetconf2/src/session_server.c
    #14 0x7efef6d10199 in nc_ps_poll github/CESNET/libnetconf2/src/session_server.c:2191:20
    #15 0x562e6c32036b in worker_thread github/CESNET/Netopeer2/src/main.c:988:14
    #16 0x562e6c1ce663 in asan_thread_start(void*) asan_interceptors.cpp.o

Thread T4 created by T0 here:
    #0 0x562e6c28f01d in pthread_create (/home/jkt/work/prog/_build/czechlight-clang19-asan-ubsan/target/sbin/netopeer2-server+0x14f01d)
    #1 0x562e6c31f25d in main github/CESNET/Netopeer2/src/main.c:1329:9
    #2 0x7efef543127d in __libc_start_call_main (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b)

SUMMARY: AddressSanitizer: heap-use-after-free github/CESNET/libnetconf2/src/io.c:967:13 in nc_write_msg_io
Shadow bytes around the buggy address:
  0x5070000d9280: fd fd fd fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x5070000d9300: fd fa fa fa fa fa fd fd fd fd fd fd fd fd fd fd
  0x5070000d9380: fa fa fa fa 00 00 00 00 00 00 00 00 00 fa fa fa
  0x5070000d9400: fa fa fd fd fd fd fd fd fd fd fd fd fa fa fa fa
  0x5070000d9480: fd fd fd fd fd fd fd fd fd fd fa fa fa fa fd fd
=>0x5070000d9500: fd fd fd fd fd fd fd fd fa fa fa fa fd[fd]fd fd
  0x5070000d9580: fd fd fd fd fd fa fa fa fa fa fd fd fd fd fd fd
  0x5070000d9600: fd fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa
  0x5070000d9680: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x5070000d9700: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x5070000d9780: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
==1010861==ABORTING

@jktjkt jktjkt changed the title heap-use-after-free via netconf-cli's test suite heap-use-after-free when processing an RPC with no input and no outputs Jan 31, 2025
Roytak added a commit that referenced this issue Jan 31, 2025
@Roytak
main BUGFIX invalid memory access on rpc
2efb81d 
Fixes #1695
@Roytak
Copy link
Collaborator

Roytak commented Jan 31, 2025

Hi, should be fixed.

@Roytak Roytak added is:bug Bug description. status:completed From the developer perspective, the issue was solved (bug fixed, question answered,...) labels Jan 31, 2025
@jktjkt
Copy link
Contributor Author

jktjkt commented Jan 31, 2025

Your change in sysrepo/sysrepo@927fd522 introduced a regression which was caught by sysrepo-cpp's unit tests. We can adapt to these changes, but please keep in mind that this is definitely a very visible change in behavior and of the API contract. If you change a function to suddenly allow returning a NULL, your users will have to adapt their code.

test-subscriptions: sysrepo-cpp/src/Session.cpp:352: libyang::DataNode sysrepo::Session::sendRPC(libyang::DataNode, std::chrono::milliseconds): Assertion `output' failed.

@michalvasko
Copy link
Member

We were aware of the exact effect and while causing all the issues you mentioned, we think this is how it was supposed to always work and so the change is a positive one in the long run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
is:bug Bug description. status:completed From the developer perspective, the issue was solved (bug fixed, question answered,...)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jktjkt @michalvasko @Roytak