ComputerSurge/bind9
3
0
Fork 0
Code Pull Requests 1 Activity

Fix RCU API usage in acl.c

Browse Source 
The rcu_xchg_pointer() function can be used outside of a critical
section, and usually must be followed by a synchronize_rcu() or
call_rcu() call to detach from the resource, unless if there are
some guarantees in place because of our own reference counting.
This commit is contained in:
Aram Sargsyan
2024-09-04 16:19:44 +00:00
parent 0d2482c62e
commit 0367c60759
1 changed files with 55 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

90
lib/dns/acl.c
View File

@@ -459,13 +459,25 @@ dns_aclelement_match(const isc_netaddr_t *reqaddr, const dns_name_t *reqsigner,
}
static void
dns__acl_destroy(dns_acl_t *dacl) {
unsigned int i;
dns_acl_port_transports_t *port_proto;
dns__acl_destroy_port_transports(dns_acl_t *acl) {
dns_acl_port_transports_t *port_proto = NULL;
dns_acl_port_transports_t *next = NULL;
ISC_LIST_FOREACH_SAFE (acl->ports_and_transports, port_proto, link,
next)
{
ISC_LIST_DEQUEUE(acl->ports_and_transports, port_proto, link);
isc_mem_put(acl->mctx, port_proto, sizeof(*port_proto));
}
}
static void
dns__acl_destroy(dns_acl_t *dacl) {
INSIST(!ISC_LINK_LINKED(dacl, nextincache));
for (i = 0; i < dacl->length; i++) {
isc_refcount_destroy(&dacl->references);
dacl->magic = 0;
for (size_t i = 0; i < dacl->length; i++) {
dns_aclelement_t *de = &dacl->elements[i];
if (de->type == dns_aclelementtype_keyname) {
dns_name_free(&de->keyname, dacl->mctx);
@@ -484,18 +496,8 @@ dns__acl_destroy(dns_acl_t *dacl) {
dns_iptable_detach(&dacl->iptable);
}
port_proto = ISC_LIST_HEAD(dacl->ports_and_transports);
while (port_proto != NULL) {
dns_acl_port_transports_t *next = NULL;
dns__acl_destroy_port_transports(dacl);
next = ISC_LIST_NEXT(port_proto, link);
ISC_LIST_DEQUEUE(dacl->ports_and_transports, port_proto, link);
isc_mem_put(dacl->mctx, port_proto, sizeof(*port_proto));
port_proto = next;
}
isc_refcount_destroy(&dacl->references);
dacl->magic = 0;
isc_mem_putanddetach(&dacl->mctx, dacl, sizeof(*dacl));
}
@@ -659,10 +661,21 @@ dns_aclenv_set(dns_aclenv_t *env, dns_acl_t *localhost, dns_acl_t *localnets) {
REQUIRE(DNS_ACL_VALID(localhost));
REQUIRE(DNS_ACL_VALID(localnets));
rcu_read_lock();
localhost = rcu_xchg_pointer(&env->localhost, dns_acl_ref(localhost));
localnets = rcu_xchg_pointer(&env->localnets, dns_acl_ref(localnets));
rcu_read_unlock();
/*
* This function is called only during interface scanning, so blocking
* a bit is acceptable. Wait until all ongoing attachments to old
* 'localhost' and 'localnets' are finished before we can detach and
* possibly destroy them.
*
* The problem here isn't the memory reclamation per se, but
* the reference counting race - we need to wait for the
* critical section to end before we decrement the value and
* possibly destroy the acl objects.
*/
synchronize_rcu();
dns_acl_detach(&localhost);
dns_acl_detach(&localnets);
@@ -675,24 +688,34 @@ dns_aclenv_copy(dns_aclenv_t *target, dns_aclenv_t *source) {
rcu_read_lock();
dns_acl_t *localhost = rcu_dereference(source->localhost);
/*
* We need to acquire the reference inside the critical section.
*/
dns_acl_t *localhost = dns_acl_ref(rcu_dereference(source->localhost));
INSIST(DNS_ACL_VALID(localhost));
dns_acl_t *localnets = rcu_dereference(source->localnets);
dns_acl_t *localnets = dns_acl_ref(rcu_dereference(source->localnets));
INSIST(DNS_ACL_VALID(localnets));
localhost = rcu_xchg_pointer(&target->localhost,
dns_acl_ref(localhost));
localnets = rcu_xchg_pointer(&target->localnets,
dns_acl_ref(localnets));
rcu_read_unlock();
localhost = rcu_xchg_pointer(&target->localhost, localhost);
localnets = rcu_xchg_pointer(&target->localnets, localnets);
/*
* This function is called only during (re)configuration, so blocking
* a bit is acceptable.
*
* See the comment above in dns_aclenv_set() for more detail.
*/
synchronize_rcu();
target->match_mapped = source->match_mapped;
#if defined(HAVE_GEOIP2)
target->geoip = source->geoip;
#endif /* if defined(HAVE_GEOIP2) */
rcu_read_unlock();
dns_acl_detach(&localhost);
dns_acl_detach(&localnets);
}
@@ -703,17 +726,14 @@ dns__aclenv_destroy(dns_aclenv_t *aclenv) {
aclenv->magic = 0;
rcu_read_lock();
dns_acl_t *localhost = rcu_xchg_pointer(&aclenv->localhost, NULL);
INSIST(DNS_ACL_VALID(localhost));
/*
* The last reference to the aclenv has been detached, so nobody should
* be reading from this aclenv. We can destroy the localhost and
* localnet directly without swapping the pointers.
*/
dns_acl_t *localnets = rcu_xchg_pointer(&aclenv->localnets, NULL);
INSIST(DNS_ACL_VALID(localnets));
rcu_read_unlock();
dns_acl_detach(&localhost);
dns_acl_detach(&localnets);
dns_acl_detach(&aclenv->localhost);
dns_acl_detach(&aclenv->localnets);
isc_mem_putanddetach(&aclenv->mctx, aclenv, sizeof(*aclenv));
}