upstream/bind9
1
0
Fork 0
mirror of https://github.com/isc-projects/bind9.git synced 2026-06-11 03:40:00 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Fix a lock-order-inversion bug in resolver.c

Browse source 
There is a lock-order-inversion (potential deadlock) in resolver.c,
because in dns_resolver_shutdown() a resolver bucket lock is locked
while the resolver lock itself is already locked, while in
fctx_sendevents() the resolver lock is locked while a bucket lock
is locked before calling that function in fctx__done_detach().

The resolver lock/unlock in dns_resolver_shutdown() was added back in
the 317e36d47e commit to make sure that
the function is finished before the resolver object is destroyed.

Since res->exiting is atomic, it should be possible to remove the
resolver locking in dns_resolver_shutdown() and add it to the
send_shutdown_events() function which requires it.

Also, since 'res->exiting' is now set while unlocked, the 'INSIST'
in spillattimer_countdown() is wrong, and is removed.
This commit is contained in:
Aram Sargsyan 2023-06-05 14:35:42 +00:00
parent ff3d25a47f
commit db45cab546
1 changed files with 2 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
lib/dns/resolver.c
View file

@ -4441,9 +4441,7 @@ fctx_destroy(fetchctx_t *fctx, bool exiting) {
if (bucket_empty && exiting &&
isc_refcount_decrement(&res->activebuckets) == 1)
{
LOCK(&res->lock);
send_shutdown_events(res);
UNLOCK(&res->lock);
}
isc_refcount_destroy(&fctx->references);
@ -10260,10 +10258,7 @@ send_shutdown_events(dns_resolver_t *res) {
isc_event_t *event, *next_event;
isc_task_t *etask;
/*
* Caller must be holding the resolver lock.
*/
LOCK(&res->lock);
for (event = ISC_LIST_HEAD(res->whenshutdown); event != NULL;
event = next_event)
{
@ -10273,6 +10268,7 @@ send_shutdown_events(dns_resolver_t *res) {
event->ev_sender = res;
isc_task_sendanddetach(&etask, &event);
}
UNLOCK(&res->lock);
}
static void
@ -10287,7 +10283,6 @@ spillattimer_countdown(isc_task_t *task, isc_event_t *event) {
UNUSED(task);
LOCK(&res->lock);
INSIST(!atomic_load_acquire(&res->exiting));
if (res->spillat > res->spillatmin) {
res->spillat--;
logit = true;
@ -10644,7 +10639,6 @@ dns_resolver_shutdown(dns_resolver_t *res) {
RTRACE("shutdown");
LOCK(&res->lock);
if (atomic_compare_exchange_strong(&res->exiting, &is_false, true)) {
RTRACE("exiting");
@ -10673,7 +10667,6 @@ dns_resolver_shutdown(dns_resolver_t *res) {
true);
RUNTIME_CHECK(result == ISC_R_SUCCESS);
}
UNLOCK(&res->lock);
}
void