1. 01 Oct, 2020 3 commits
    • Mark Andrews's avatar
      Add missing rwlock calls when access keynode.initial and keynode.managed · 840cf7ad
      Mark Andrews authored
          WARNING: ThreadSanitizer: data race
          Write of size 1 at 0x000000000001 by thread T1 (mutexes: write M1):
          #0 dns_keynode_trust lib/dns/keytable.c:836
          #1 keyfetch_done lib/dns/zone.c:10187
          #2 dispatch lib/isc/task.c:1152
          #3 run lib/isc/task.c:1344
          #4 <null> <null>
      
          Previous read of size 1 at 0x000000000001 by thread T2 (mutexes: read M2):
          #0 keynode_dslist_totext lib/dns/keytable.c:682
          #1 dns_keytable_totext lib/dns/keytable.c:732
          #2 named_server_dumpsecroots bin/named/server.c:11357
          #3 named_control_docommand bin/named/control.c:264
          #4 control_command bin/named/controlconf.c:390
          #5 dispatch lib/isc/task.c:1152
          #6 run lib/isc/task.c:1344
          #7 <null> <null>
      
          Location is heap block of size 241 at 0x000000000010 allocated by thread T3:
          #0 malloc <null>
          #1 default_memalloc lib/isc/mem.c:713
          #2 mem_get lib/isc/mem.c:622
          #3 mem_allocateunlocked lib/isc/mem.c:1268
          #4 isc___mem_allocate lib/isc/mem.c:1288
          #5 isc__mem_allocate lib/isc/mem.c:2453
          #6 isc___mem_get lib/isc/mem.c:1037
          #7 isc__mem_get lib/isc/mem.c:2432
          #8 new_keynode lib/dns/keytable.c:346
          #9 insert lib/dns/keytable.c:393
          #10 dns_keytable_add lib/dns/keytable.c:421
          #11 process_key bin/named/server.c:955
          #12 load_view_keys bin/named/server.c:983
          #13 configure_view_dnsseckeys bin/named/server.c:1140
          #14 configure_view bin/named/server.c:5371
          #15 load_configuration bin/named/server.c:9110
          #16 loadconfig bin/named/server.c:10310
          #17 named_server_reconfigcommand bin/named/server.c:10693
          #18 named_control_docommand bin/named/control.c:250
          #19 control_command bin/named/controlconf.c:390
          #20 dispatch lib/isc/task.c:1152
          #21 run lib/isc/task.c:1344
          #22 <null> <null>
      
          Mutex M1 is already destroyed.
      
          Mutex M2 is already destroyed.
      
          Thread T1 (running) created by main thread at:
          #0 pthread_create <null>
          #1 isc_thread_create pthreads/thread.c:73
          #2 isc_taskmgr_create lib/isc/task.c:1434
          #3 create_managers bin/named/main.c:915
          #4 setup bin/named/main.c:1223
          #5 main bin/named/main.c:1523
      
          Thread T2 (running) created by main thread at:
          #0 pthread_create <null>
          #1 isc_thread_create pthreads/thread.c:73
          #2 isc_taskmgr_create lib/isc/task.c:1434
          #3 create_managers bin/named/main.c:915
          #4 setup bin/named/main.c:1223
          #5 main bin/named/main.c:1523
      
          Thread T3 (running) created by main thread at:
          #0 pthread_create <null>
          #1 isc_thread_create pthreads/thread.c:73
          #2 isc_taskmgr_create lib/isc/task.c:1434
          #3 create_managers bin/named/main.c:915
          #4 setup bin/named/main.c:1223
          #5 main bin/named/main.c:1523
      
          SUMMARY: ThreadSanitizer: data race lib/dns/keytable.c:836 in dns_keynode_trust
      840cf7ad
    • Mark Andrews's avatar
      Merge branch '2192-tsan-error-accessing-listener-connections' into 'main' · 061fb5e0
      Mark Andrews authored
      Resolve "TSAN error accessing listener->connections"
      
      Closes #2192
      
      See merge request isc-projects/bind9!4206
      061fb5e0
    • Mark Andrews's avatar
      Lock access to listener->connections · bea1326c
      Mark Andrews authored
      as it is accessed from multiple threads with libuv.
      
          WARNING: ThreadSanitizer: data race
          Write of size 8 at 0x000000000001 by thread T1:
          #0 conn_reset bin/named/controlconf.c:574
          #1 isc_nmhandle_detach netmgr/netmgr.c:1257
          #2 isc__nm_uvreq_put netmgr/netmgr.c:1389
          #3 tcp_send_cb netmgr/tcp.c:1030
          #4 <null> <null>
          #5 <null> <null>
      
          Previous read of size 8 at 0x000000000001 by thread T2:
          #0 conn_reset bin/named/controlconf.c:574
          #1 isc_nmhandle_detach netmgr/netmgr.c:1257
          #2 control_recvmessage bin/named/controlconf.c:556
          #3 recv_data lib/isccc/ccmsg.c:110
          #4 isc__nm_tcp_shutdown netmgr/tcp.c:1161
          #5 shutdown_walk_cb netmgr/netmgr.c:1511
          #6 uv_walk <null>
          #7 process_queue netmgr/netmgr.c:656
          #8 process_normal_queue netmgr/netmgr.c:582
          #9 process_queues netmgr/netmgr.c:590
          #10 async_cb netmgr/netmgr.c:548
          #11 <null> <null>
          #12 <null> <null>
      
          Location is heap block of size 265 at 0x000000000017 allocated by thread T3:
          #0 malloc <null>
          #1 default_memalloc lib/isc/mem.c:713
          #2 mem_get lib/isc/mem.c:622
          #3 isc___mem_get lib/isc/mem.c:1044
          #4 isc__mem_get lib/isc/mem.c:2432
          #5 add_listener bin/named/controlconf.c:1127
          #6 named_controls_configure bin/named/controlconf.c:1324
          #7 load_configuration bin/named/server.c:9181
          #8 run_server bin/named/server.c:9819
          #9 dispatch lib/isc/task.c:1152
          #10 run lib/isc/task.c:1344
          #11 <null> <null>
      
          Thread T1 (running) created by main thread at:
          #0 pthread_create <null>
          #1 isc_thread_create pthreads/thread.c:73
          #2 isc_nm_start netmgr/netmgr.c:232
          #3 create_managers bin/named/main.c:909
          #4 setup bin/named/main.c:1223
          #5 main bin/named/main.c:1523
      
          Thread T2 (running) created by main thread at:
          #0 pthread_create <null>
          #1 isc_thread_create pthreads/thread.c:73
          #2 isc_nm_start netmgr/netmgr.c:232
          #3 create_managers bin/named/main.c:909
          #4 setup bin/named/main.c:1223
          #5 main bin/named/main.c:1523
      
          Thread T3 (running) created by main thread at:
          #0 pthread_create <null>
          #1 isc_thread_create pthreads/thread.c:73
          #2 isc_taskmgr_create lib/isc/task.c:1434
          #3 create_managers bin/named/main.c:915
          #4 setup bin/named/main.c:1223
          #5 main bin/named/main.c:1523
      
          SUMMARY: ThreadSanitizer: data race bin/named/controlconf.c:574 in conn_reset
      bea1326c
  2. 30 Sep, 2020 15 commits
  3. 29 Sep, 2020 9 commits
    • Ondřej Surý's avatar
      Merge branch '2124-fix-assertion-failure-in-dns-message' into 'main' · 6e156834
      Ondřej Surý authored
      Resolve "Bind 9.16.6 Assertion failure message.c:4733: REQUIRE(msg->state == (-1)) failed"
      
      Closes #2124
      
      See merge request isc-projects/bind9!4194
      6e156834
    • Ondřej Surý's avatar
      Add CHANGES and release note for GL #2124 · 6179a388
      Ondřej Surý authored
      6179a388
    • Ondřej Surý's avatar
      The dns_message_create() cannot fail, change the return to void · 33eefe9f
      Ondřej Surý authored
      The dns_message_create() function cannot soft fail (as all memory
      allocations either succeed or cause abort), so we change the function to
      return void and cleanup the calls.
      33eefe9f
    • Diego dos Santos Fronza's avatar
      cocci: Add semantic patch to refactor dns_message_destroy() · 7deaf9a9
      Diego dos Santos Fronza authored
      dns_message_t objects are now being handled using reference counting
      semantics, so now dns_message_destroy() is not called directly anymore,
      dns_message_detach must be called instead.
      7deaf9a9
    • Diego dos Santos Fronza's avatar
      Properly handling dns_message_t shared references · cde6227a
      Diego dos Santos Fronza authored
      This commit fix the problems that arose when moving the dns_message_t
      object from fetchctx_t to the query structure.
      
      Since the lifetime of query objects are different than that of a fetchctx
      and the dns_message_t object held by the query may be being used by some
      external module, e.g. validator, even after the query may have been destroyed,
      propery handling of the references to the message were added in this commit to
      avoid accessing an already destroyed object.
      
      Specifically, in rctx_done(), a reference to the message is attached at
      the beginning of the function and detached at the end, since a possible call
      to fctx_cancelquery() would release the dns_message_t object, and in the next
      lines of code a call to rctx_nextserver() or rctx_chaseds() would require
      a valid pointer to the same object.
      
      In valcreate() a new reference is attached to the message object, this
      ensures that if the corresponding query object is destroyed before the
      validator attempts to access it, no invalid pointer access occurs.
      
      In validated() we have to attach a new reference to the message, since
      we destroy the validator object at the beginning of the function,
      and we need access to the message in the next lines of the same function.
      
      rctx_nextserver() and rctx_chaseds() functions were adapted to receive
      a new parameter of dns_message_t* type, this was so they could receive a
      valid reference to a dns_message_t since using the response context respctx_t
      to access the message through rctx->query->rmessage could lead to an already
      released reference due to the query being canceled.
      cde6227a
    • Diego dos Santos Fronza's avatar
      Fix invalid dns message state in resolver's logic · 02f9e125
      Diego dos Santos Fronza authored
      The assertion failure REQUIRE(msg->state == DNS_SECTION_ANY),
      caused by calling dns_message_setclass within function resquery_response()
      in resolver.c, was happening due to wrong management of dns message_t
      objects used to process responses to the queries issued by the resolver.
      
      Before the fix, a resolver's fetch context (fetchctx_t) would hold
      a pointer to the message, this same reference would then be used over all
      the attempts to resolve the query, trying next server, etc... for this to work
      the message object would have it's state reset between each iteration, marking
      it as ready for a new processing.
      
      The problem arose in a scenario with many different forwarders configured,
      managing the state of the dns_message_t object was lacking better
      synchronization, which have led it to a invalid dns_message_t state in
      resquery_response().
      
      Instead of adding unnecessarily complex code to synchronize the object,
      the dns_message_t object was moved from fetchctx_t structure to the
      query structure, where it better belongs to, since each query will produce
      a response, this way whenever a new query is created an associated
      dns_messate_t is also created.
      
      This commit deals mainly with moving the dns_message_t object from fetchctx_t
      to the query structure.
      02f9e125
    • Diego dos Santos Fronza's avatar
      Refactored dns_message_t for using attach/detach semantics · 12d6d131
      Diego dos Santos Fronza authored
      This commit will be used as a base for the next code updates in order
      to have a better control of dns_message_t objects' lifetime.
      12d6d131
    • Mark Andrews's avatar
      Merge branch... · e6f2f79f
      Mark Andrews authored
      Merge branch '2189-some-comments-in-lib-dns-stats-c-use-incorrect-notation-for-bit-values' into 'main'
      
      Resolve "some comments in lib/dns/stats.c use incorrect notation for bit values"
      
      Closes #2189
      
      See merge request isc-projects/bind9!4191
      e6f2f79f
    • Mark Andrews's avatar
      Update comments to have binary notation · 6727e23a
      Mark Andrews authored
      6727e23a
  4. 28 Sep, 2020 9 commits
    • Michał Kępień's avatar
      Merge branch '114-out-of-tree-system-tests' into 'main' · 90e1acfd
      Michał Kępień authored
      Add out-of-tree system test job
      
      Closes #114
      
      See merge request isc-projects/bind9!3895
      90e1acfd
    • Michal Nowak's avatar
      Do not remove $systest for out-of-tree builds · 47075f64
      Michal Nowak authored
      Previously, the $systest directory was being removed for out-of-tree
      builds at the end of each system test.  Because of that, running tests
      which depend on compiled objects was breaking subsequent "make check"
      invocations:
      
          make: Target 'check' not remade because of errors.
          Making all in dyndb/driver
          /bin/bash: line 20: cd: dyndb/driver: No such file or directory
          Making all in dlzexternal/driver
          /bin/bash: line 20: cd: dlzexternal/driver: No such file or directory
      
      Address by first removing build/test artifacts for a given test and then
      removing empty directories inside (and potentially including) $systest.
      47075f64
    • Michal Nowak's avatar
      Add an out-of-tree system test job to GitLab CI · 483e5af5
      Michal Nowak authored
      Make sure the new job does not get run for every pipeline as it is not
      expected to break often and it is similar enough to other system test
      jobs.  Change the name of the variable holding the path to the
      out-of-tree build directory to a more generic one.
      483e5af5
    • Ondřej Surý's avatar
      Merge branch 'ondrej/clear-the-uv-event-loop-before-exiting' into 'main' · 36195cf7
      Ondřej Surý authored
      Clear the libuv event loop before exiting
      
      See merge request isc-projects/bind9!4181
      36195cf7
    • Ondřej Surý's avatar
      Refactor the pausing/unpausing and finishing the nm_thread · e5ab137b
      Ondřej Surý authored
      The isc_nm_pause(), isc_nm_resume() and finishing the nm_thread() from
      nm_destroy() has been refactored, so all use the netievents instead of
      directly touching the worker structure members.  This allows us to
      remove most of the locking as the .paused and .finished members are
      always accessed from the matching nm_thread.
      
      When shutting down the nm_thread(), instead of issuing uv_stop(), we
      just shutdown the .async handler, so all uv_loop_t events are properly
      finished first and uv_run() ends gracefully with no outstanding active
      handles in the loop.
      e5ab137b
    • Michał Kępień's avatar
      Merge branch '1725-drop-function-wrapping' into 'main' · b8b3ddb0
      Michał Kępień authored
      Drop function wrapping as it is redundant for now
      
      Closes #1725
      
      See merge request isc-projects/bind9!4174
      b8b3ddb0
    • Michał Kępień's avatar
      Fix function overrides in unit tests on macOS · b60d7345
      Michał Kępień authored
      Since Mac OS X 10.1, Mach-O object files are by default built with a
      so-called two-level namespace which prevents symbol lookups in BIND unit
      tests that attempt to override the implementations of certain library
      functions from working as intended.  This feature can be disabled by
      passing the "-flat_namespace" flag to the linker.  Fix unit tests
      affected by this issue on macOS by adding "-flat_namespace" to LDFLAGS
      used for building all object files on that operating system (it is not
      enough to only set that flag for the unit test executables).
      b60d7345
    • Michał Kępień's avatar
      Drop function wrapping as it is redundant for now · 8bdba2ed
      Michał Kępień authored
      As currently used in the BIND source tree, the --wrap linker option is
      redundant because:
      
        - static builds are no longer supported,
      
        - there is no need to wrap around existing functions - what is
          actually required (at least for now) is to replace them altogether
          in unit tests,
      
        - only functions exposed by shared libraries linked into unit test
          binaries are currently being replaced.
      
      Given the above, providing the alternative implementations of functions
      to be overridden in lib/ns/tests/nstest.c is a much simpler alternative
      to using the --wrap linker option.  Drop the code detecting support for
      the latter from configure.ac, simplify the relevant Makefile.am, and
      remove lib/ns/tests/wrap.c, updating lib/ns/tests/nstest.c accordingly
      (it is harmless for unit tests which are not calling the overridden
      functions).
      8bdba2ed
    • Mark Andrews's avatar
      Merge branch '2185-nsdname-wait-recurse-speed-test-fails-under-tsan' into 'main' · aee29706
      Mark Andrews authored
      Resolve "nsdname-wait-recurse speed test fails under tsan"
      
      Closes #2185
      
      See merge request isc-projects/bind9!4184
      aee29706
  5. 27 Sep, 2020 2 commits
  6. 25 Sep, 2020 2 commits