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

Don't remove ADB entry from LRU before trying to expire it

Browse Source 
There was a code flow error that would remove the expired ADB entry from
the LRU list and then a check in the expire_entry() would cause
assertion error because it expect the ADB entry to be linked.

Additionally, the expire mechanism would loop for cases when we would
held only a read rwlock; in such case we need to upgrade the lock and
try again, not just try again.
This commit is contained in:
Ondřej Surý
2023-02-16 12:26:01 +01:00
parent 51e0b8994e
commit 668d287d01
1 changed files with 43 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

77
lib/dns/adb.c
View File

@@ -1143,6 +1143,8 @@ destroy_adbentry(dns_adbentry_t *entry) {
entry->magic = 0;
INSIST(!ISC_LINK_LINKED(entry, link));
INSIST(ISC_LIST_EMPTY(entry->nhs));
active = atomic_load_acquire(&entry->active);
@@ -1381,6 +1383,16 @@ get_attached_and_locked_name(dns_adb_t *adb, const dns_name_t *name,
return (adbname);
}
static void
upgrade_entries_lock(dns_adb_t *adb, isc_rwlocktype_t *locktypep,
isc_stdtime_t now) {
if (*locktypep == isc_rwlocktype_read) {
UPGRADELOCK(&adb->entries_lock, *locktypep);
purge_stale_entries(adb, now);
adb->entries_last_update = now;
}
}
/*
* Find the entry in the adb->entries hashtable.
*/
@@ -1405,20 +1417,17 @@ get_attached_and_locked_entry(dns_adb_t *adb, isc_stdtime_t now,
{
last_update = now;
UPGRADELOCK(&adb->entries_lock, locktype);
purge_stale_entries(adb, now);
adb->entries_last_update = now;
upgrade_entries_lock(adb, &locktype, now);
}
again:
result = isc_hashmap_find(adb->entries, &hashval,
(const unsigned char *)addr, sizeof(*addr),
(void **)&adbentry);
switch (result) {
case ISC_R_NOTFOUND: {
if (result == ISC_R_NOTFOUND) {
upgrade_entries_lock(adb, &locktype, now);
create:
UPGRADELOCK(&adb->entries_lock, locktype);
INSIST(locktype == isc_rwlocktype_write);
/* Allocate a new entry and add it to the hash table. */
adbentry = new_adbentry(adb, addr);
@@ -1426,26 +1435,18 @@ again:
result = isc_hashmap_add(adb->entries, &hashval,
&adbentry->sockaddr,
sizeof(adbentry->sockaddr), adbentry);
if (result == ISC_R_EXISTS) {
if (result == ISC_R_SUCCESS) {
ISC_LIST_PREPEND(adb->entries_lru, adbentry, link);
} else if (result == ISC_R_EXISTS) {
dns_adbentry_detach(&adbentry);
result = isc_hashmap_find(adb->entries, &hashval,
(const unsigned char *)addr,
sizeof(*addr),
(void **)&adbentry);
ISC_LIST_UNLINK(adb->entries_lru, adbentry, link);
}
INSIST(result == ISC_R_SUCCESS);
break;
}
case ISC_R_SUCCESS:
if (locktype == isc_rwlocktype_write) {
ISC_LIST_UNLINK(adb->entries_lru, adbentry, link);
}
break;
default:
UNREACHABLE();
}
INSIST(result == ISC_R_SUCCESS);
/*
* The dns_adbentry_ref() must stay here before trying to expire
@@ -1455,14 +1456,17 @@ again:
LOCK(&adbentry->lock); /* Must be unlocked by the caller */
switch (locktype) {
case isc_rwlocktype_read:
if (entry_expired(adbentry, now)) {
UNLOCK(&adbentry->lock);
dns_adbentry_detach(&adbentry);
goto again;
if (!entry_expired(adbentry, now)) {
break;
}
break;
/* We need to upgrade the LRU lock */
UNLOCK(&adbentry->lock);
upgrade_entries_lock(adb, &locktype, now);
LOCK(&adbentry->lock);
FALLTHROUGH;
case isc_rwlocktype_write:
if (maybe_expire_entry(adbentry, now)) {
if (ENTRY_DEAD(adbentry) || maybe_expire_entry(adbentry, now)) {
UNLOCK(&adbentry->lock);
dns_adbentry_detach(&adbentry);
goto create;
@@ -1472,11 +1476,13 @@ again:
UNREACHABLE();
}
/* Did enough time pass to update the LRU? */
if (adbentry->last_used + ADB_CACHE_MINIMUM <= last_update) {
adbentry->last_used = now;
}
if (locktype == isc_rwlocktype_write) {
ISC_LIST_PREPEND(adb->entries_lru, adbentry, link);
if (locktype == isc_rwlocktype_write) {
ISC_LIST_UNLINK(adb->entries_lru, adbentry, link);
ISC_LIST_PREPEND(adb->entries_lru, adbentry, link);
}
}
RWUNLOCK(&adb->entries_lock, locktype);
@@ -1653,12 +1659,15 @@ expire_entry(dns_adbentry_t *adbentry) {
isc_result_t result;
dns_adb_t *adb = adbentry->adb;
adbentry->flags |= ENTRY_IS_DEAD;
if (!ENTRY_DEAD(adbentry)) {
adbentry->flags |= ENTRY_IS_DEAD;
result = isc_hashmap_delete(adb->entries, NULL, &adbentry->sockaddr,
sizeof(adbentry->sockaddr));
RUNTIME_CHECK(result == ISC_R_SUCCESS);
ISC_LIST_UNLINK(adb->entries_lru, adbentry, link);
result = isc_hashmap_delete(adb->entries, NULL,
&adbentry->sockaddr,
sizeof(adbentry->sockaddr));
RUNTIME_CHECK(result == ISC_R_SUCCESS);
ISC_LIST_UNLINK(adb->entries_lru, adbentry, link);
}
dns_adbentry_detach(&adbentry);
}