Commit Graph

36748 Commits

Author SHA1 Message Date
Ondřej Surý
b19d932262 Increase the BUFSIZ-long buffers
The BUFSIZ value varies between platforms, it could be 8K on Linux and
512 bytes on mingw.  Make sure the buffers are always big enough for the
output data to prevent truncation of the output by appropriately
enlarging or sizing the buffers.
2022-07-15 10:33:46 +00:00
Michał Kępień
d0f6d309e3 Merge branch '3443-memory-related-cleanups' into 'main'
Memory-related cleanups

Closes #3443

See merge request isc-projects/bind9!6567
2022-07-15 08:31:23 +00:00
Michał Kępień
f0c31ceb3b Update documentation for named's -M option
Remove "external" from the list of legal values for the -M command-line
option as it has not been allowed since the internal memory allocator
was removed by commit 55ace5d3aa.

Make the style of the relevant paragraph more in line with the next one
and split its contents up into an unordered list of options for improved
readability.
2022-07-15 10:23:03 +02:00
Evan Hunt
f9620d8b94 Merge branch '3456-dispatch-connect-race' into 'main'
remove unnecessary assertion in dns_dispatch_connect()

Closes #3456

See merge request isc-projects/bind9!6573
2022-07-15 02:26:41 +00:00
Evan Hunt
25ddec8a0a remove unnecessary assertion in dns_dispatch_connect()
When a thread calls dns_dispatch_connect() on an unconnected TCP socket
it sets `tcpstate` from `DNS_DISPATCHSTATE_NONE` to `_CONNECTING`.
Previously, it then INSISTed that there were no pending connections
before calling isc_nm_tcpdnsconnect().

If a second thread called dns_dispatch_connect() during that window
of time, it could add a pending connection to the list, and trigger
an assertion failure.

This commit removes the INSIST since the condition is actually
harmless.
2022-07-14 16:31:01 -07:00
Ondřej Surý
2488dbbb33 Merge branch 'ondrej-fix-timing-error-in-statistics-system-test' into 'main'
Wait for TCP connection refused in the statistics system test

See merge request isc-projects/bind9!6580
2022-07-14 20:33:16 +00:00
Ondřej Surý
0227d82dc8 Wait for TCP connection refused in the statistics system test
The statistics system test makes a query to foo.info to check for the
pending connections because the ans4 doesn't respond to the query.

This might or might not (depending on exact timing) increment the failed
TCP connection counter when the query is retried over TCP because ans4
doesn't listen on the TCP.

Wait for the 'connection refused' in the ns3 log file to be able to
count the exactly 1 failed TCP connection.
2022-07-14 13:08:29 -07:00
Ondřej Surý
deb0c27dbc Merge branch '3451-handle-transient-TCP-connect-EADDRINUSE-on-BSDs' into 'main'
Handle the transient TCP connect() failures on FreeBSD

Closes #3451 and #3452

See merge request isc-projects/bind9!6562
2022-07-14 19:38:33 +00:00
Ondřej Surý
a9c54ecdc6 Add CHANGES note for [GL #3451] 2022-07-14 14:34:53 +02:00
Ondřej Surý
59e1703b50 Cleanup the STATID_CONNECT and STATID_CONNECTFAIL stat counters
The STATID_CONNECT and STATID_CONNECTFAIL statistics were used
incorrectly. The STATID_CONNECT was incremented twice (once in
the *_connect_direct() and once in the callback) and STATID_CONNECTFAIL
would not be incremented at all if the failure happened in the callback.

Closes: #3452
2022-07-14 14:34:53 +02:00
Ondřej Surý
b21f507c0a Handle the transient TCP connect() failures on FreeBSD
On FreeBSD (and perhaps other *BSD) systems, the TCP connect() call (via
uv_tcp_connect()) can fail with transient UV_EADDRINUSE error.  The UDP
code already handles this by trying three times (is a charm) before
giving up.  Add a code for the TCP, TCPDNS and TLSDNS layers to also try
three times before giving up by calling uv_tcp_connect() from the
callback two more time on UV_EADDRINUSE error.

Additionally, stop the timer only if we succeed or on hard error via
isc__nm_failed_connect_cb().
2022-07-14 14:20:10 +02:00
Mark Andrews
b8f4e19366 Merge branch '3448-redundant-assignment-of-clistenon-in-bin-named-server-c' into 'main'
Resolve "Redundant assignment of clistenon in bin/named/server.c"

Closes #3448

See merge request isc-projects/bind9!6560
2022-07-14 01:14:14 +00:00
Mark Andrews
73fb350989 Remove redundant assignment of 'clistenon = NULL' 2022-07-14 00:24:37 +00:00
Evan Hunt
5ac79f500e Merge branch '3454-check-putstr' into 'main'
check putstr return values

Closes #3454

See merge request isc-projects/bind9!6574
2022-07-14 00:23:17 +00:00
Evan Hunt
f6653d0cf2 check putstr return values
The calls to putstr() in named_server_fetchlimit() were not checked
for failure.
2022-07-14 00:04:39 +00:00
Mark Andrews
56c4480e3e Merge branch '3447-lib-dns-tkey-c-free_namelist-should-be-disassociating-associated-rdatatsets' into 'main'
Resolve "lib/dns/tkey.c:free_namelist should be disassociating associated rdatatsets"

Closes #3447

See merge request isc-projects/bind9!6556
2022-07-14 00:03:38 +00:00
Mark Andrews
745d5edc3a disassociate rdatasets when cleaning up
free_namelist could be passed names with associated rdatasets
when handling errors.  These need to be disassociated before
calling dns_message_puttemprdataset.
2022-07-13 23:43:39 +00:00
Mark Andrews
0e0012af0a Merge branch '3449-kasp-system-test-failed-to-log-some-zones-during-setup' into 'main'
Resolve "kasp system test failed to log some zones during setup"

Closes #3449

See merge request isc-projects/bind9!6561
2022-07-13 23:42:38 +00:00
Mark Andrews
04627997eb kasp: add missing logging during setup
Some zones where not being logged when just DNSSEC keys where being
generated in system test setup phase.  Add logging for these zones.
2022-07-13 23:19:39 +00:00
Michał Kępień
4be5bd24d2 Merge branch '3054-3-improve-reporting-for-pthreads-errors' into 'main'
[3/3] Improve reporting for pthreads errors

Closes #3054

See merge request isc-projects/bind9!6572
2022-07-13 13:05:11 +00:00
Michał Kępień
a5346854b6 Merge branch '3054-2-enable-tracking-pthreads-objects' into 'main'
[2/3] Enable tracking pthreads objects

See merge request isc-projects/bind9!6571
2022-07-13 13:03:59 +00:00
Michał Kępień
16fc0a8379 Merge branch '3054-1-misc-pthreads-cleanups' into 'main'
[1/3] Miscellaneous pthreads cleanups

See merge request isc-projects/bind9!6570
2022-07-13 13:01:32 +00:00
Michał Kępień
a73ab08e75 Improve reporting for barrier errors
uv_barrier_init() errors are currently ignored.  Use UV_RUNTIME_CHECK()
to catch them and to improve error reporting for any uv_barrier_init()
run-time failures (by augmenting error messages with file/line
information and the error string corresponding to the value returned).
-
Michał Kępień
90d6bd062d Detect pthreads object leaks during respdiff tests
Set the ISC_TRACK_PTHREADS_OBJECTS preprocessor macro when preparing a
build of BIND 9 for respdiff testing and pass the -m command-line option
to respdiff.sh in order to enable automatic identification of memory
leaks during respdiff tests.
-
Ondřej Surý
ce3b3c1d80 Directly cause assertion failure on pthreads primitives failure
Instead of returning error values from isc_rwlock_*(), isc_mutex_*(),
and isc_condition_*() macros/functions and subsequently carrying out
runtime assertion checks on the return values in the calling code,
trigger assertion failures directly in those macros/functions whenever
any pthread function returns an error, as there is no point in
continuing execution in such a case anyway.
-
Michał Kępień
56638c94b5 Improve reporting for read-write lock errors
Replace direct uses of implementation-specific rwlock functions in
lib/isc/include/isc/rwlock.h with preprocessor macros that use
ERRNO_CHECK(), in order to augment rwlock-related error messages with
file/line/caller information and the error string corresponding to
errno.  Adjust the implementation-specific functions for pthreads-based
rwlocks so that they return any errors encountered to the caller instead
of aborting execution immediately using RUNTIME_CHECK().

To keep code modifications simple, make the non-pthreads-based
implementation-specific rwlock functions always return 0; these
functions continue to handle errors using less verbose run-time
assertions as they do not set errno anyway.
-
Michał Kępień
4b4a41b07e Enable tracking of pthreads barriers
Some POSIX threads implementations (e.g. FreeBSD's libthr) allocate
memory on the heap when pthread_barrier_init() is called.  Every call to
that function must be accompanied by a corresponding call to
pthread_barrier_destroy() or else the memory allocated for the barrier
will leak.

jemalloc can be used for detecting memory allocations which are not
released by a process when it exits.  Unfortunately, since jemalloc is
also the system allocator on FreeBSD and a special (profiling-enabled)
build of jemalloc is required for memory leak detection, this method
cannot be used for detecting leaked memory allocated by libthr on a
stock FreeBSD installation.

However, libthr's behavior can be emulated on any platform by
implementing alternative versions of libisc functions for creating and
destroying barriers that allocate memory using malloc() and release it
using free().  This enables using jemalloc for detecting missing
pthread_barrier_destroy() calls on any platform on which it works
reliably.

When the newly introduced ISC_TRACK_PTHREADS_OBJECTS preprocessor macro
is set, allocate isc_barrier_t structures on the heap in
isc_barrier_init() and free them in isc_barrier_destroy().  Reuse
existing barrier macros (after renaming them appropriately) for other
operations.
-
Ondřej Surý
aadb5e95a2 Use library constructor to create default mutex attr once
Instead of using isc_once_do() on every isc_mutex_init() call, use the
global library constructor to initialize the default mutex attr
object (optionally with PTHREAD_MUTEX_ADAPTIVE_NP if supported) just
once when the library is loaded.
-
Michał Kępień
bad0f6d49d Improve reporting for condition variable errors
Replace all uses of RUNTIME_CHECK() in lib/isc/include/isc/condition.h
with ERRNO_CHECK(), in order to improve error reporting for any
condition-variable-related run-time failures (by augmenting error
messages with file/line/caller information and the error string
corresponding to errno).
-
Ondřej Surý
89ce327266 Enable tracking of pthreads rwlocks
Some POSIX threads implementations (e.g. FreeBSD's libthr) allocate
memory on the heap when pthread_rwlock_init() is called.  Every call to
that function must be accompanied by a corresponding call to
pthread_rwlock_destroy() or else the memory allocated for the rwlock
will leak.

jemalloc can be used for detecting memory allocations which are not
released by a process when it exits.  Unfortunately, since jemalloc is
also the system allocator on FreeBSD and a special (profiling-enabled)
build of jemalloc is required for memory leak detection, this method
cannot be used for detecting leaked memory allocated by libthr on a
stock FreeBSD installation.

However, libthr's behavior can be emulated on any platform by
implementing alternative versions of libisc functions for creating and
destroying rwlocks that allocate memory using malloc() and release it
using free().  This enables using jemalloc for detecting missing
pthread_rwlock_destroy() calls on any platform on which it works
reliably.

When the newly introduced ISC_TRACK_PTHREADS_OBJECTS preprocessor macro
is set (and --enable-pthread-rwlock is used), allocate isc_rwlock_t
structures on the heap in isc_rwlock_init() and free them in
isc_rwlock_destroy().  Reuse existing functions defined in
lib/isc/rwlock.c for other operations, but rename them first, so that
they contain triple underscores (to indicate that these functions are
implementation-specific, unlike their mutex and condition variable
counterparts, which always use the pthreads implementation).  Define the
isc__rwlock_init() macro so that it is a logical counterpart of
isc__mutex_init() and isc__condition_init(); adjust isc___rwlock_init()
accordingly.  Remove a redundant function prototype for
isc__rwlock_lock() and rename that (static) function to rwlock_lock() in
order to avoid having to use quadruple underscores.
-
Michał Kępień
76d2a97ed2 Handle pthread_*_init() failures consistently
isc_rwlock_init() currently detects pthread_rwlock_init() failures using
a REQUIRE() assertion.  Use the ERRNO_CHECK() macro for that purpose
instead, so that read-write lock initialization failures are handled
identically as condition variable (pthread_cond_init()) and mutex
(pthread_mutex_init()) initialization failures.
-
Michał Kępień
3506a22fb7 Improve reporting for mutex errors
Replace all uses of RUNTIME_CHECK() in lib/isc/include/isc/mutex.h with
ERRNO_CHECK(), in order to improve error reporting for any mutex-related
run-time failures (by augmenting error messages with file/line/caller
information and the error string corresponding to errno).
-
Ondřej Surý
59b8a064e9 Enable tracking of pthreads condition variables
Some POSIX threads implementations (e.g. FreeBSD's libthr) allocate
memory on the heap when pthread_cond_init() is called.  Every call to
that function must be accompanied by a corresponding call to
pthread_cond_destroy() or else the memory allocated for the condition
variable will leak.

jemalloc can be used for detecting memory allocations which are not
released by a process when it exits.  Unfortunately, since jemalloc is
also the system allocator on FreeBSD and a special (profiling-enabled)
build of jemalloc is required for memory leak detection, this method
cannot be used for detecting leaked memory allocated by libthr on a
stock FreeBSD installation.

However, libthr's behavior can be emulated on any platform by
implementing alternative versions of libisc functions for creating and
destroying condition variables that allocate memory using malloc() and
release it using free().  This enables using jemalloc for detecting
missing pthread_cond_destroy() calls on any platform on which it works
reliably.

When the newly introduced ISC_TRACK_PTHREADS_OBJECTS preprocessor macro
is set, allocate isc_condition_t structures on the heap in
isc_condition_init() and free them in isc_condition_destroy().  Reuse
existing condition variable macros (after renaming them appropriately)
for other operations.
-
Michał Kępień
3608abc8fa Add an ERRNO_CHECK() preprocessor macro
In a number of situations in pthreads-related code, a common sequence of
steps is taken: if the value returned by a library function is not 0,
pass errno to strerror_r(), log the string returned by the latter, and
immediately abort execution.  Add an ERRNO_CHECK() preprocessor macro
which takes those exact steps and use it wherever (conveniently)
possible.

Notes:

 1. The "log the return value of strerror_r() and abort" pattern is used
    in a number of other places that this commit does not touch; only
    "!= 0" checks followed by isc_error_fatal() calls with
    non-customized error messages are replaced here.

 2. This change temporarily breaks file name & line number reporting for
    isc__mutex_init() errors, to prevent breaking the build.  This issue
    will be rectified in a subsequent change.
-
Ondřej Surý
3fd019ea9e Enable tracking of pthreads mutexes
Some POSIX threads implementations (e.g. FreeBSD's libthr) allocate
memory on the heap when pthread_mutex_init() is called.  Every call to
that function must be accompanied by a corresponding call to
pthread_mutex_destroy() or else the memory allocated for the mutex will
leak.

jemalloc can be used for detecting memory allocations which are not
released by a process when it exits.  Unfortunately, since jemalloc is
also the system allocator on FreeBSD and a special (profiling-enabled)
build of jemalloc is required for memory leak detection, this method
cannot be used for detecting leaked memory allocated by libthr on a
stock FreeBSD installation.

However, libthr's behavior can be emulated on any platform by
implementing alternative versions of libisc functions for creating and
destroying mutexes that allocate memory using malloc() and release it
using free().  This enables using jemalloc for detecting missing
pthread_mutex_destroy() calls on any platform on which it works
reliably.

Introduce a new ISC_TRACK_PTHREADS_OBJECTS preprocessor macro, which
causes isc_mutex_t structures to be allocated on the heap by
isc_mutex_init() and freed by isc_mutex_destroy().  Reuse existing mutex
macros (after renaming them appropriately) for other operations.
-
Ondřej Surý
9968a6292d Merge branch 'ondrej-update-dir-locals-for-libtest' into 'main'
Update the .dir-locals.el for libtest

See merge request isc-projects/bind9!6565
2022-07-13 10:21:35 +00:00
Ondřej Surý
80fbd849d5 Update the .dir-locals.el for libtest
The tests/libtest directory is missing from the .dir-locals.el, so the
emacs flycheck would not work for the unit tests.  Add it to the
configuration.
2022-07-13 12:17:34 +02:00
Michał Kępień
5415ecbd7c Merge branch '3439-stop-resolving-invalid-names-in-resume_dslookup' into 'main'
Stop resolving invalid names in resume_dslookup()

Closes #3439

See merge request isc-projects/bind9!6563
2022-07-13 08:59:30 +00:00
Michał Kępień
cfa398ad37 Add CHANGES entry and release note for GL #3439 2022-07-13 10:31:16 +02:00
Michał Kępień
1a79aeab44 Stop resolving invalid names in resume_dslookup()
Commit 7b2ea97e46 introduced a logic bug
in resume_dslookup(): that function now only conditionally checks
whether DS chasing can still make progress.  Specifically, that check is
only performed when the previous resume_dslookup() call invokes
dns_resolver_createfetch() with the 'nameservers' argument set to
something else than NULL, which may not always be the case.  Failing to
perform that check may trigger assertion failures as a result of
dns_resolver_createfetch() attempting to resolve an invalid name.

Example scenario that leads to such outcome:

 1. A validating resolver is configured to forward all queries to
    another resolver.  The latter returns broken DS responses that
    trigger DS chasing.

 2. rctx_chaseds() calls dns_resolver_createfetch() with the
    'nameservers' argument set to NULL.

 3. The fetch fails, so resume_dslookup() is called.  Due to
    fevent->result being set to e.g. DNS_R_SERVFAIL, the default branch
    is taken in the switch statement.

 4. Since 'nameservers' was set to NULL for the fetch which caused the
    resume_dslookup() callback to be invoked
    (fctx->nsfetch->private->nameservers), resume_dslookup() chops off
    one label off fctx->nsname and calls dns_resolver_createfetch()
    again, for a name containing one label less than before.

 5. Steps 3-4 are repeated (i.e. all attempts to find the name servers
    authoritative for the DS RRset being chased fail) until fctx->nsname
    becomes stripped down the the root name.

 6. Since resume_dslookup() does not check whether DS chasing can still
    make progress, it strips off a label off the root name and continues
    its attempts at finding the name servers authoritative for the DS
    RRset being chased, passing an invalid name to
    dns_resolver_createfetch().

Fix by ensuring resume_dslookup() always checks whether DS chasing can
still make progress when a name server fetch fails.  Update code
comments to ensure the purpose of the relevant dns_name_equal() check is
clear.
2022-07-13 10:31:16 +02:00
Mark Andrews
75027bc6ce Merge branch '3446-autosign-s-checking-revoked-key-with-duplicate-key-id-test-was-incomplete' into 'main'
Resolve "Autosign's 'checking revoked key with duplicate key ID' test was incomplete"

Closes #3446

See merge request isc-projects/bind9!6555
2022-07-13 00:48:09 +00:00
Mark Andrews
513cb24b55 Make "checking revoked key with duplicate key ID" work
There should be 2 keys with the same key id after the numerically
lower one is revoked (serial space arithmetic).  The DS points
at the non-revoked key so validation should still succeed.
2022-07-13 00:47:49 +00:00
Mark Andrews
e28492dc04 Merge branch '3445-update-libdns_la_libadd-instead-of-libdns_la_ldflags' into 'main'
Resolve "Update libdns_la_LIBADD instead of libdns_la_LDFLAGS"

Closes #3445

See merge request isc-projects/bind9!6553
2022-07-13 00:44:51 +00:00
Mark Andrews
5b51610174 Update libdns_la_LIBADD rather than libdns_la_LDFLAGS
the wrong macro was being update with MAXMINDDB_LIBS making
it difficult to adjust link order.
2022-07-13 00:14:13 +00:00
Evan Hunt
47bf1e95ad Merge branch '2962-invalid-fctx' into 'main'
clear fctx->magic and fetch->magic when destroying

See merge request isc-projects/bind9!6543
2022-07-12 23:54:25 +00:00
Evan Hunt
5ec077e6aa clear fctx->magic and fetch->magic when destroying
fctx_destroy() and dns_resolver_destroyfetch() did not clear the
'magic' field during destruction.
2022-07-12 23:40:47 +00:00
Evan Hunt
24e52a623f Merge branch '2683-ixfr-logging' into 'main'
log the reason for falling back to AXFR from IXFR at level info

Closes #2683

See merge request isc-projects/bind9!6511
2022-07-12 23:25:34 +00:00
Evan Hunt
df1d81cf96 log the reason for falling back to AXFR from IXFR at level info
messages indicating the reason for a fallback to AXFR (i.e, because
the requested serial number is not present in the journal, or because
the size of the IXFR response would exceeed "max-ixfr-ratio") are now
logged at level info instead of debug(4).
2022-07-12 16:02:54 -07:00
Artem Boldariev
0b221bf326 Merge branch 'artem-backport-tls-related-fixes-from-loopmgr-branch' into 'main'
Backport TLS-related fixes from the loopmgr-branch

See merge request isc-projects/bind9!6501
2022-07-12 12:11:21 +00:00
Artem Boldariev
ffcb54211e TLS: do not ignore accept callback result
Before this change the TLS code would ignore the accept callback result,
and would not try to gracefully close the connection. This had not been
noticed, as it is not really required for DoH. Now the code tries to
shut down the TLS connection gracefully when accepting it is not
successful.
2022-07-12 14:40:22 +03:00