Conversation
connection_or_change_state() saved an old_state to pass to channel_tls_handle_state_change_on_orconn(), which promptly cast it to void. Remove this unused variable and parameter.
Add a publish-subscribe subsystem to publish messages about changes to OR connections. connection_or_change_state() in connection_or.c and control_event_or_conn_event() in control.c publish messages to this subsystem via helper functions. Move state constants from connection_or.h to orconn_state.h so that subscribers don't have to include all of connection_or.h to take actions based on changes in OR connection state. Move event constants from control.h for similar reasons. Part of ticket 27167.
Add a publish-subscribe subsystem to publish messages about changes to origin circuits. Functions in circuitbuild.c and circuitlist.c publish messages to this subsystem. Move circuit event constants out of control.h so that subscribers don't have to include all of control.h to take actions based on messages they receive. Part of ticket 27167.
Part of ticket 27167.
Add a tracker for bootstrap progress, tracking events related to origin circuit and ORCONN states. This uses the ocirc_event and orconn_event publish-subscribe subsystems. Part of ticket 27167.
Replace a few invocations of control_event_bootstrap() with calls from the bootstrap tracker subsystem. This mostly leaves behavior unchanged. The actual behavior changes come in the next commit. Part of ticket 27167.
Redefine the set of bootstrap phases to allow display of finer-grained progress in the early connection stages of connecting to a relay. This includes adding intermediate phases for proxy and PT connections. Also add a separate new phase to indicate obtaining enough directory info to build circuits so we can report that independently of actually initiating an ORCONN to build the first application circuit. Previously, we would claim to be connecting to a relay when we had merely finished obtaining directory info. Part of ticket 27167.
Part of ticket 27617.
Pull Request Test Coverage Report for Build 2393
💛 - Coveralls |
tlyu
pushed a commit
that referenced
this pull request
Sep 3, 2020
When accessing the last_resolved_address cache we always need to convert the AF family value to an index value else we are out of bound and thus overflowing if we write to it. This fix is on code that has not been released. GeKo reported the following libasan crash using Tor Browser alpha with tor 0.4.5.0-alpha-dev (3c884bc): ==4240==ERROR: AddressSanitizer: global-buffer-overflow on address 0x55888490e388 at pc 0x5588842cc216 bp 0x7ffc8c421b00 sp 0x7ffc8c421af8 READ of size 2 at 0x55888490e388 thread T0 #0 0x5588842cc215 in tor_addr_compare_masked (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x5a6215) #1 0x558884203210 in is_local_to_resolve_addr (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x4dd210) #2 0x558883f7e252 in channel_tls_connect (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x258252) #3 0x558883f87ff7 in channel_connect_for_circuit (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x261ff7) #4 0x558883f8bc90 in circuit_handle_first_hop (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x265c90) #5 0x558883f8c891 in circuit_establish_circuit (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x266891) #6 0x558883fc3bbc in circuit_launch_by_extend_info (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x29dbbc) #7 0x558883fc5900 (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x29f900) #8 0x558883fc6988 in connection_ap_handshake_attach_circuit (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x2a0988) #9 0x558883fd0d3f in connection_ap_attach_pending (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x2aad3f) #10 0x7f4d50110885 (TorBrowser/Tor/libevent-2.1.so.7+0x22885) #11 0x7f4d501110de in event_base_loop (TorBrowser/Tor/libevent-2.1.so.7+0x230de) #12 0x558883f69b3c in do_main_loop (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x243b3c) #13 0x558883f3f70c in tor_run_main (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x21970c) #14 0x558883f3c2f7 in tor_main (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x2162f7) #15 0x558883f3531b in main (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x20f31b) #16 0x7f4d4f76acc9 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26cc9) #17 0x558883f3ba00 (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x215a00) 0x55888490e388 is located 24 bytes to the left of global variable 'state_mgr' defined in 'src/app/config/statefile.c:184:22' (0x55888490e3a0) of size 8 0x55888490e388 is located 32 bytes to the right of global variable 'global_state' defined in 'src/app/config/statefile.c:204:20' (0x55888490e360) of size 8 SUMMARY: AddressSanitizer: global-buffer-overflow (/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x5a6215) in tor_addr_compare_masked Shadow bytes around the buggy address: 0x0ab190919c20: 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9 0x0ab190919c30: 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 03 f9 f9 f9 0x0ab190919c40: f9 f9 f9 f9 03 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 0x0ab190919c50: 00 00 00 04 f9 f9 f9 f9 00 00 00 00 00 00 00 04 0x0ab190919c60: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 =>0x0ab190919c70: f9[f9]f9 f9 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 0x0ab190919c80: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 0x0ab190919c90: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 0x0ab190919ca0: 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 0x0ab190919cb0: 01 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9 0x0ab190919cc0: 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9 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 Shadow gap: cc ==4240==ABORTING Signed-off-by: David Goulet <dgoulet@torproject.org>
tlyu
pushed a commit
that referenced
this pull request
Feb 22, 2021
This bug made the pipeline fail. It basically tries to access a service we just
freed because it's still on the service list.
It only occurs about once every 10 tests and it looks like this:
$ ./src/test/test hs_control/hs_control_add_onion_helper_add_service
hs_control/hs_control_add_onion_helper_add_service: [forking] =================================================================
==354311==ERROR: AddressSanitizer: heap-use-after-free on address 0x613000000940 at pc 0x55a159251b03 bp 0x7ffc6abb5b30 sp 0x7ffc6abb5b28
READ of size 8 at 0x613000000940 thread T0
^[[A
#0 0x55a159251b02 in hs_service_ht_HT_FIND_P_ src/feature/hs/hs_service.c:153
#1 0x55a159251b02 in hs_service_ht_HT_FIND src/feature/hs/hs_service.c:153
#2 0x55a159251b02 in find_service src/feature/hs/hs_service.c:175
#3 0x55a159251c2c in register_service src/feature/hs/hs_service.c:188
#4 0x55a159262379 in hs_service_add_ephemeral src/feature/hs/hs_service.c:3811
#5 0x55a158e865e6 in test_hs_control_add_onion_helper_add_service src/test/test_hs_control.c:847
#6 0x55a1590fe77b in testcase_run_bare_ src/ext/tinytest.c:107
#7 0x55a1590fee98 in testcase_run_forked_ src/ext/tinytest.c:201
#8 0x55a1590fee98 in testcase_run_one src/ext/tinytest.c:267
#9 0x55a1590ffb06 in tinytest_main src/ext/tinytest.c:454
#10 0x55a158b1b1a4 in main src/test/testing_common.c:420
#11 0x7f7f06f8dd09 in __libc_start_main ../csu/libc-start.c:308
#12 0x55a158b21f69 in _start (/home/f/Computers/tor/mytor/src/test/test+0x372f69)
0x613000000940 is located 64 bytes inside of 344-byte region [0x613000000900,0x613000000a58)
freed by thread T0 here:
#0 0x7f7f0774ab6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
#1 0x55a158e86508 in test_hs_control_add_onion_helper_add_service src/test/test_hs_control.c:838
#2 0x55a1590fe77b in testcase_run_bare_ src/ext/tinytest.c:107
#3 0x55a1590fee98 in testcase_run_forked_ src/ext/tinytest.c:201
#4 0x55a1590fee98 in testcase_run_one src/ext/tinytest.c:267
#5 0x55a1590ffb06 in tinytest_main src/ext/tinytest.c:454
#6 0x55a158b1b1a4 in main src/test/testing_common.c:420
#7 0x7f7f06f8dd09 in __libc_start_main ../csu/libc-start.c:308
previously allocated by thread T0 here:
#0 0x7f7f0774ae8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
#1 0x55a15948b728 in tor_malloc_ src/lib/malloc/malloc.c:45
#2 0x55a15948b7c0 in tor_malloc_zero_ src/lib/malloc/malloc.c:71
#3 0x55a159261bb5 in hs_service_new src/feature/hs/hs_service.c:4290
#4 0x55a159261f49 in hs_service_add_ephemeral src/feature/hs/hs_service.c:3758
#5 0x55a158e8619f in test_hs_control_add_onion_helper_add_service src/test/test_hs_control.c:832
#6 0x55a1590fe77b in testcase_run_bare_ src/ext/tinytest.c:107
#7 0x55a1590fee98 in testcase_run_forked_ src/ext/tinytest.c:201
#8 0x55a1590fee98 in testcase_run_one src/ext/tinytest.c:267
#9 0x55a1590ffb06 in tinytest_main src/ext/tinytest.c:454
#10 0x55a158b1b1a4 in main src/test/testing_common.c:420
#11 0x7f7f06f8dd09 in __libc_start_main ../csu/libc-start.c:308
SUMMARY: AddressSanitizer: heap-use-after-free src/feature/hs/hs_service.c:153 in hs_service_ht_HT_FIND_P_
Shadow bytes around the buggy address:
0x0c267fff80d0: 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa fa
0x0c267fff80e0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
0x0c267fff80f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c267fff8100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c267fff8110: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
=>0x0c267fff8120: fd fd fd fd fd fd fd fd[fd]fd fd fd fd fd fd fd
0x0c267fff8130: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c267fff8140: fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa
0x0c267fff8150: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
0x0c267fff8160: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c267fff8170: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
Shadow gap: cc
==354311==ABORTING
[Lost connection!]
[hs_control_add_onion_helper_add_service FAILED]
1/1 TESTS FAILED. (0 skipped)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.