ret->link.next is tested to see if it is NULL unlocked
to avoid obtaining taillock when it is not nexessary then
retested once the taillock is obtained.
While
if (isc_refcount_decrement() == 1) { // memory_order_release
isc_refcount_destroy(); // memory_order_acquire
...
}
is theoretically the most efficent in practice, using
memory_order_acq_rel produces the same code on x86_64 and doesn't
trigger tsan data races (which use a idealistic model) if
isc_refcount_destroy() is not called immediately. In fact
isc_refcount_destroy() could be removed if we didn't want
to check for the count being 0 when isc_refcount_destroy() is
called.
https://stackoverflow.com/questions/49112732/memory-order-in-shared-pointer-destructor
(cherry picked from commit 6278899a38)
When pk11_numbits() is passed a user provided input that contains all
zeroes (via crafted DNS message), it would crash with assertion
failure. Fix that by properly handling such input.
cppcheck 1.89 enabled certain value flow analysis mechanisms [1] which
trigger null pointer dereference false positives that were previously
not reported. It seems that cppcheck no longer treats at least some
REQUIRE() assertion failures as fatal, so add extra assertion macro
definitions to lib/isc/include/isc/util.h that are only used when the
CPPCHECK preprocessor macro is defined; these definitions make cppcheck
1.89 behave as expected.
There is an important requirement for these custom definitions to work:
cppcheck must properly treat abort() as a function which does not
return. In order for that to happen, the __GNUC__ macro must be set to
a high enough number (because system include directories are used and
system headers compile attributes away if __GNUC__ is not high enough).
__GNUC__ is thus set to the major version number of the GCC compiler
used, which is what that latter does itself during compilation.
[1] https://github.com/danmar/cppcheck/commit/aaeec462e6d96bb70c2b1cf030979d09e2d7c959
(cherry picked from commit abfde3d543)
The OASIS pkcs11.h header has a restrictive license. Replace the
pkcs11.h pkcs11f.h and pkcs11t.h headers with pkcs11.h from p11-kit.
For source distribution, the license for the OASIS headers itself
doesn't pose any licensing problem when combined with MPL license, but
it possibly creates problem for downstream distributors of BIND 9.
(cherry picked from commit c47fad2431)
The memory ordering in the rwlock was all wrong, I am copying excerpts
from the https://en.cppreference.com/w/c/atomic/memory_order#Relaxed_ordering
for the convenience of the reader:
Relaxed ordering
Atomic operations tagged memory_order_relaxed are not synchronization
operations; they do not impose an order among concurrent memory
accesses. They only guarantee atomicity and modification order
consistency.
Sequentially-consistent ordering
Atomic operations tagged memory_order_seq_cst not only order memory
the same way as release/acquire ordering (everything that
happened-before a store in one thread becomes a visible side effect in
the thread that did a load), but also establish a single total
modification order of all atomic operations that are so tagged.
Which basically means that we had no or weak synchronization between
threads using the same variables in the rwlock structure. There should
not be a significant performance drop because the critical sections were
already protected by:
while(1) {
if (relaxed_atomic_operation) {
break;
}
LOCK(lock);
if (!relaxed_atomic_operation) {
WAIT(sem, lock);
}
UNLOCK(lock)l
}
I would add one more thing to "Don't do your own crypto, folks.":
- Also don't do your own locking, folks.
As part of this commit, I have also cleaned up the #ifdef spaghetti,
and fixed the isc_atomic API usage.
The ThreadSanitizer found several possible data races in our rwlock
implementation. This commit convert .spins and .write_granted fields
to atomic.
(cherry picked from commit 1da0994ea4)
Add {isc,ns}_stats_{update_if_greater,get_counter}() functions that
are used to set and collect high-water type of statistics.
(cherry picked from commit a544e2e300)
For TCP high-water work, we need to keep the used integer types widths
in sync.
Note: int_fast32_t is used on WIN32 platform
(cherry picked from commit 0fc98ef2d5)
The native implementation's conversion from the uint8_t buffers to uint64_t now
follows the reference implementation that doesn't require aligned buffers.
Move the macOS section of <isc/endian.h> to a lower spot as it is
believed not to be the most popular platform for running BIND. Add a
comment and remove redundant definitions.
(cherry picked from commit c727a31eab)
Instead of only supporting Linux, try making <isc/endian.h> support
other GNU platforms as well. Since some compilers define __GNUC__ on
BSDs (e.g. Clang on FreeBSD), move the relevant section to the bottom of
the platform-specific part of <isc/endian.h>, so that it only gets
evaluated when more specific platform determination criteria are not
met. Also include <byteswap.h> so that any byte-swapping macros which
may be defined in that file on older platforms are used in the fallback
definitions of the nonstandard hto[bl]e{16,32,64}() and
[bl]e{16,32,64}toh() conversion functions.
(cherry picked from commit a98c7408fc)
While Solaris does not support the nonstandard hto[bl]e{16,32,64}() and
[bl]e{16,32,64}toh() conversion functions, it does have some
byte-swapping macros available in <sys/byteorder.h>. Ensure these
macros are used in the fallback definitions of the aforementioned
nonstandard functions.
(cherry picked from commit 5b0f81e549)
Since the hto[bl]e{16,32,64}() and [bl]e{16,32,64}toh() conversion
functions are nonstandard, add fallback definitions of these functions
to <isc/endian.h>, so that their unavailability does not prevent
compilation from succeeding.
(cherry picked from commit 973d2991a0)
Current versions of DragonFly BSD, FreeBSD, NetBSD, and OpenBSD all
support the modern variants of functions converting values between host
and big-endian/little-endian byte order while older ones might not.
Ensure <isc/endian.h> works properly in both cases.
(cherry picked from commit 588c14d5c9)
- if the TCP quota has been exceeded but there are no clients listening
for new connections on the interface, we can now force attachment to the
quota using isc_quota_force(), instead of carrying on with the quota not
attached.
- the TCP client quota is now referenced via a reference-counted
'ns_tcpconn' object, one of which is created whenever a client begins
listening for new connections, and attached to by members of that
client's pipeline group. when the last reference to the tcpconn
object is detached, it is freed and the TCP quota slot is released.
- reduce code duplication by adding mark_tcp_active() function.
- convert counters to atomic.
(cherry picked from commit 7e8222378ca24f1302a0c1c638565050ab04681b)
(cherry picked from commit 4939451275722bfda490ea86ca13e84f6bc71e46)
(cherry picked from commit 13f7c918b8)
The ATOMIC_*_LOCK_FREE can evalutate either 0, 1, or 2 which indicate the
lock-free property of the corresponding atomic types (both signed and unsigned).
Value Explanation
----- --------------------------------------
0 The atomic type is never lock-free
1 The atomic type is sometimes lock-free
2 The atomic type is always lock-free
----- --------------------------------------
(cherry picked from commit a5e7901eb9)
Evan Hunt
Mark Andrews
Ondřej SurýandMichał Kępień
Ondřej SurýandOndřej Surý
Samuel ThibaultandOndřej Surý
Diego FronzaandOndřej Surý