From 32ff134eeb62dfb11a11824bccafbedf6a97d809 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 16 Dec 2022 21:46:50 +0100 Subject: [PATCH] Fix reference counting in get_attached_entry (again) When get_attached_entry() encounters entry that would be expired, it needs to get reference to the entry before calling maybe_expire_entry(), so the ADB entry doesn't get destroyed inside the its own lock. This creeped into the code base again during review, so I am adding an extra comment to prevent this. --- lib/dns/adb.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/dns/adb.c b/lib/dns/adb.c index 0454105c14..d105107f95 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -1368,12 +1368,14 @@ get_attached_and_locked_name(dns_adb_t *adb, const dns_name_t *name, adbname); INSIST(result == ISC_R_SUCCESS); + dns_adbname_ref(adbname); LOCK(&adbname->lock); /* Must be unlocked by the caller */ adbname->last_used = now; ISC_LIST_PREPEND(adb->names_lru, adbname, link); break; case ISC_R_SUCCESS: + dns_adbname_ref(adbname); LOCK(&adbname->lock); /* Must be unlocked by the caller */ if (adbname->last_used + ADB_CACHE_MINIMUM <= last_update) { adbname->last_used = now; @@ -1386,7 +1388,6 @@ get_attached_and_locked_name(dns_adb_t *adb, const dns_name_t *name, UNREACHABLE(); } - dns_adbname_ref(adbname); /* * The refcount is now 2 and the final detach will happen in * expire_name() - the unused adbname stored in the hashtable and lru @@ -1436,6 +1437,7 @@ get_attached_and_locked_entry(dns_adb_t *adb, isc_stdtime_t now, sizeof(adbentry->sockaddr), adbentry); INSIST(result == ISC_R_SUCCESS); + dns_adbentry_ref(adbentry); LOCK(&adbentry->lock); /* Must be unlocked by the caller */ adbentry->last_used = now; @@ -1443,6 +1445,11 @@ get_attached_and_locked_entry(dns_adb_t *adb, isc_stdtime_t now, break; } case ISC_R_SUCCESS: + /* + * The dns_adbentry_ref() must stay here before trying to expire + * the ADB entry, so it is not destroyed under the lock. + */ + dns_adbentry_ref(adbentry); LOCK(&adbentry->lock); /* Must be unlocked by the caller */ if (maybe_expire_entry(adbentry, now)) { UNLOCK(&adbentry->lock); @@ -1460,8 +1467,6 @@ get_attached_and_locked_entry(dns_adb_t *adb, isc_stdtime_t now, UNREACHABLE(); } - dns_adbentry_ref(adbentry); - UNLOCK(&adb->entries_lock); return (adbentry);