The SRTT update loaded the old value, computed a new one, and stored it
back as separate operations. Two concurrent callers could each read the
same old value and one update would be silently lost.
Use a CAS loop for the read-modify-write on entry->srtt. For the aging
path, also CAS on entry->lastage to prevent multiple threads from aging
the same entry in the same second.
(cherry picked from commit 4d15494b94)
A 'dns_dtenv_t' pointer is passed to an async function without taking
a reference first, which can potentially cause a use-after-free error.
Take a reference, then detach in the async function.
Closes#5820
Backport of MR !11705
Merge branch 'backport-5820-dns_dtenv-reference-bug-fix-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11714
The 'env' pointer is passed to an async function without taking
a reference first, which can potentially cause a use-after-free
error. Take a reference, then detach in the async function.
(cherry picked from commit 48d7401f0d)
Change the convention for system test directory names to always use an
underscore rather than a hyphen. Names using underscore are valid python
package names and can be used with standard `import` facilities in
python, which allows easier code reuse.
Backport of MR !11710
Merge branch 'backport-nicki/system-test-dir-underscore-names-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11711
All system tests previously using a hyphen have been renamed to use
underscore instead. A couple of symlinks were corrected and one path in
`nsec3-answer` adjusted accordingly.
(cherry picked from commit 67aca1f8c6)
Change the convention for system test directory names to always use an
underscore rather than a hyphen. Names using underscore are valid python
package names and can be used with standard `import` facilities in
python, which allows easier code reuse.
The temporary directories for test execution and their convenience
symlinks have been switched to using hyphens rather than underscores to
keep the pytest collection, filtering and .gitignore working as
expected.
(cherry picked from commit 9f4c1d1993)
isc_buffer_init() is given MAX_DNS_MESSAGE_SIZE (65535) as capacity but
only h2->content_length bytes are allocated. This makes the buffer
believe it has more space than actually allocated. A secondary bounds
check (new_bufsize <= h2->content_length) prevents actual overflow, but
the buffer invariant is violated.
Pass h2->content_length as the capacity to match the allocation.
Backport of MR !11662
Merge branch 'backport-ondrej/fix-isc_buffer_init-capacity-mismatch-in-DoH-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11709
isc_buffer_init() is given MAX_DNS_MESSAGE_SIZE (65535) as capacity but
only h2->content_length bytes are allocated. This makes the buffer
believe it has more space than actually allocated. A secondary bounds
check (new_bufsize <= h2->content_length) prevents actual overflow, but
the buffer invariant is violated.
Pass h2->content_length as the capacity to match the allocation.
(cherry picked from commit 8e240bbb5f)
Under specific error conditions during query processing, resources were not
being properly released, which could eventually lead to unnecessary memory
consumption for the server. The a potential resource leak in the resolver
has been fixed.
Backport of MR !11658
Merge branch 'backport-ondrej/fix-pthread-primitives-usage-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11706
The keylist_lock rwlock is initialized at startup but never destroyed
on exit, unlike the sibling namelock mutex which is properly cleaned up.
(cherry picked from commit 5dc19a7d92)
The error cleanup in fctx_create() was missing isc_mutex_destroy() and
dns_ede_invalidate() calls. When error paths (cleanup_nameservers,
cleanup_fcount, cleanup_qmessage, cleanup_adb) were taken after the
mutex and edectx were initialized, the fctx memory was freed without
properly destroying these resources first.
(cherry picked from commit 5b1750f15f)
The lock is acquired for reading but the error path from
dns_rdata_fromstruct() incorrectly unlocks it as a write lock.
(cherry picked from commit 96a22451d7)
Fail with a specific error code if we detect a deadlock in the validator.
Closes#5769
Backport of MR !11622
Merge branch 'backport-5769-deadlock-validator-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11702
We return DNS_R_NOVALIDSIG if we detected a deadlock. Then in
'validate_async_done()', this result value is used to check if we
need to fall back to insecure. As part of that we create a new fetch
but that fails because of the detected deadlock. This results in a loop
of deadlock detected, fallback to insecure, deadlock detected, ...
Add a new result value, ISC_R_DEADLOCK, and return this instead when
we have detected a deadlock. This will be treated as a generic error,
as there is no special handling for this result value.
(cherry picked from commit bc1d177cc2)
Calling `rndc modzone` on a zone that was configured in the configuration file caused a crash. This has been fixed.
ISC would like to thank Nathan Reilly for reporting this.
Closes#5800
Backport of MR !11683
Merge branch 'backport-5800-rndc-modzone-non-dynamic-zone-crash-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11698
'rndc modzone' deletes the old configuration. If we don't store the
new zone config, when we do a 'rndc showzone' it will be a failure.
This is not an issue in the 9.21 version, because of the effective
config behavior.
If we are modifiying the zone, the zone must have been added before.
Don't overwrite this value on modifications.
Also it feels cleaner to pass added=false to configure_zone() in
do_modzone().
(cherry picked from commit 780872e07e)
Some code paths try to lock an already locked view->newzone.lock.
For example, do_modzone() aqcuires the lock and then calls
delete_zoneconf(), that wants to acquire the same lock.
Add a parameter to delete_zoneconf() that informs the function if the
lock has already been acquired.
(cherry picked from commit 71587b0816)
Add test cases for 'rndc showzone' and 'rndc modzone' on a zone
that was configured in named.conf. This should not crash.
(cherry picked from commit 900127c460)
Zero errno before calling strtol.
Closes#5773
Backport of MR !11625
Merge branch 'backport-5773-clear-errno-correctly-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11703
The previous code was incorrectly clearing errno after calling
strtol but before testing the result rather than clearing it and
then calling strtol so that changes to errno can be correctly
determined.
(cherry picked from commit d3ffa1f007)
The key identifiers in the comments did not match those in the
output.
Closes#5785
Backport of MR !11634
Merge branch 'backport-5785-dnssec-guide-patch-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11700
Move the LIBDNS_XFRIN_RECV_DONE probe execution before dns_xfrin_detach
in xfrin_recv_done.
Previously, dns_xfrin_detach was called before the trace probe, which
could free the xfr object. Because the accessed member xfr->info is an
embedded array, the expression evaluates via pointer arithmetic rather
than a direct memory dereference. Although this prevents a reliable
crash in practice, it technically remains a use-after-free issue.
Reorder the statements to ensure the transfer context is fully valid
when the probe executes.
Closes#5786
Backport of MR !11632
Merge branch 'backport-5786-fix-dtrace-after-free-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11694
Move the LIBDNS_XFRIN_RECV_DONE probe execution before dns_xfrin_detach
in xfrin_recv_done.
Previously, dns_xfrin_detach was called before the trace probe, which
could free the xfr object. Because the accessed member xfr->info is an
embedded array, the expression evaluates via pointer arithmetic rather
than a direct memory dereference. Although this prevents a reliable
crash in practice, it technically remains a use-after-free issue.
Reorder the statements to ensure the transfer context is fully valid
when the probe executes.
(cherry picked from commit e57245ee81)
Exclude named.args.j2 files from license header checks so named.args can
be generated from Jinja templates. Also exclude system test README files
from the license header checks.
Backport of MR !11690
Merge branch 'backport-colin/reuse-namedargs-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11696
Starting from OpenSSL 4 the the X509_get_subject_name() function
returns a 'const' pointer to a name instead of a regular pointer.
Duplicate the name before operating on it, then free it.
Closes#5807
Backport of MR !11676
Merge branch 'backport-5807-openssl-4-X509_get_subject_name-compat-fix-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11692
Starting from OpenSSL 4 the the X509_get_subject_name() function
returns a 'const' pointer to a name instead of a regular pointer.
Duplicate the name before operating on it, then free it.
(cherry picked from commit 336c523b79)
Fix missing detach/free on error paths.
Backport of MR !11666
Merge branch 'backport-ondrej/fix-reference-counting-errors-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11691
In both cfg_kasp_fromconfig() and cfg_kasp_builtinconfig(), the
newly allocated KASP key was not destroyed when the keystore
lookup failed.
(cherry picked from commit df1993611b)
When TLS creation fails in tlslisten_acceptcb(), tlssock->server
was not detached before detaching tlssock itself.
(cherry picked from commit 2ab3d7c075)
Since memory allocation never fails in BIND 9, checkds_create() cannot
fail. Change it to return void and use designated initializers,
removing error handling at all call sites.
(cherry picked from commit 63d3c1f58a)
Initialize cb_args to NULL and free it in the cleanup path so it
is not leaked when the function fails after allocation.
(cherry picked from commit d7e1013741)
Two 'goto next' paths in zone_notify() skipped detaching the TSIG
key and transport, leaking them on TLS configuration failure and
when the destination address is disabled.
(cherry picked from commit 1505cb1c24)
Use isc_mem_putanddetach() instead of isc_mem_put() to properly
detach the attached memory context stored in resarg->mctx.
(cherry picked from commit d0165070c7)
In fctx_query(), resquery_ref(query) is called before
dns_dispatch_connect() in anticipation of the resquery_connected()
callback consuming the reference. When dns_dispatch_connect() fails
synchronously on TCP (e.g. from dns_transport_get_tlsctx() failing
in tcp_dispatch_connect()), the connect callback is never scheduled,
so the extra reference is never consumed. This has been fixed.
Backport of MR !11640
Merge branch 'backport-ondrej/fix-resquery-refcount-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11656
In fctx_query(), resquery_ref(query) is called before
dns_dispatch_connect() in anticipation of the resquery_connected()
callback consuming the reference.
When dns_dispatch_connect() fails synchronously on TCP (e.g. from
dns_transport_get_tlsctx() failing in tcp_dispatch_connect()), the
connect callback is never scheduled, so the extra reference is never
consumed. The error path then tears down the query via manual cleanup
(isc_mem_put) without going through the refcount destructor, leaving
the reference imbalanced.
Fix by dropping the extra reference on the error path, just after
dns_dispatch_done() which cleans up the dispatch entry.
(cherry picked from commit 2da669490c)
The :iscman:`named` process could hang when processing a
maliciously crafted update for a response policy zone (RPZ).
This has been fixed.
Closes#5775
Backport of MR !11659
Merge branch 'backport-5775-rpz-del_name-deadlock-fix-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11687
When the dns_qp_getname() call returns an error the del_name() function
just returns without cleaning up the trasnaction.
Instead of returning, jump to a new label 'done:' similar to the code
written in the add_nm() function.
(cherry picked from commit 4df5b9ac32)
When defaults->zonedir is set, opts->zonedir is unconditionally
overwritten without freeing the previous value. This leaks memory
on every catalog zone update when zonedir defaults are configured.
Free the existing opts->zonedir before replacing it.
Backport of MR !11660
Merge branch 'backport-ondrej/fix-memory-leak-in-dns_catz_options_setdefault-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11685
When defaults->zonedir is set, opts->zonedir is unconditionally
overwritten without freeing the previous value. This leaks memory
on every catalog zone update when zonedir defaults are configured.
Free the existing opts->zonedir before replacing it.
(cherry picked from commit 5cd17c8adc)
Asynchronous zone loading and dumping operations occasionally dispatched tasks
to the wrong internal event loop. This threading violation triggered internal
safety assertions that abruptly terminated named. Strict loop affinity is now
enforced for these tasks, ensuring they execute on their designated threads
and preventing the crashes.
Closes#4882
Backport of MR !11655
Merge branch 'backport-4882-run-rndc-zone-commands-on-correct-loop-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11684
Refactor dns_loadctx_t and dns_dumpctx_t to use standard
ISC_REFCOUNT_DECL and ISC_REFCOUNT_IMPL macros, retiring the
redundant manual attach and detach implementations.
Introduce dns_loadctx_enqueue() and dns_dumpctx_enqueue() to
ensure compliance with the new strict loop affinity in
isc_work_enqueue(). If the current loop does not match the
target loop, the enqueue operation is safely bounced to the
correct thread via isc_async_run().
(cherry picked from commit e7c550730a)
Add a REQUIRE(isc_loop() == loop) assertion to isc_work_enqueue()
to strictly enforce that work is enqueued from the loop it is
assigned to. This loudly prohibits cross-thread queue manipulation
before it inevitably turns into a concurrency debugging nightmare.
(cherry picked from commit f1311d2d19)
