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

Dispatch ratelimiter events under the lock

Browse source 
isc__ratelimiter_tick() and isc_ratelimiter_shutdown() each pulled
events out of rl->pending into a function-local list, dropped the
mutex, and then iterated.  ISC_LIST_APPEND leaves the link in the
LINKED state, so a concurrent isc_ratelimiter_dequeue() saw an
event as still queued, called ISC_LIST_UNLINK against rl->pending —
which patched the prev/next of the local list — and freed the
event before dispatch finished, producing either an INSIST in the
unlink macro or a use-after-free in the dispatch loop.

isc_async_run() is a non-blocking wfcq enqueue, so there is no
benefit to dropping the mutex around it.  Unlink each event and
hand it to isc_async_run() while still holding rl->lock; the
existing ISC_LINK_LINKED check in dequeue then correctly
distinguishes "still queued and cancellable" from "already taken".

Assisted-by: Claude:claude-opus-4-7
This commit is contained in:
Ondřej Surý 2026-04-30 07:39:23 +02:00
parent bf7ee390ba
commit 4d465f4fa5
1 changed files with 15 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

39
lib/isc/ratelimiter.c
View file

@ -219,12 +219,9 @@ static void
isc__ratelimiter_tick(void *arg) {
isc_ratelimiter_t *rl = (isc_ratelimiter_t *)arg;
uint32_t pertic;
ISC_LIST(isc_rlevent_t) pending;
REQUIRE(VALID_RATELIMITER(rl));
ISC_LIST_INIT(pending);
LOCK(&rl->lock);
REQUIRE(rl->timer != NULL);
@ -237,11 +234,7 @@ isc__ratelimiter_tick(void *arg) {
pertic = rl->pertic;
while (pertic != 0) {
isc_rlevent_t *rle = ISC_LIST_HEAD(rl->pending);
if (rle != NULL) {
/* There is work to do. Let's do it after unlocking. */
ISC_LIST_UNLINK(rl->pending, rle, link);
ISC_LIST_APPEND(pending, rle, link);
} else {
if (rle == NULL) {
/*
* We processed all the scheduled work, but there's a
* room for at least one more event (we haven't consumed
@ -252,6 +245,15 @@ isc__ratelimiter_tick(void *arg) {
rl->state = isc_ratelimiter_idle;
break;
}
/*
* Unlink and dispatch under the lock: isc_async_run() is a
* non-blocking enqueue, so this stays cheap, and once the
* link is TOMBSTONEd a concurrent isc_ratelimiter_dequeue()
* sees ISC_LINK_LINKED == false and returns ISC_R_NOTFOUND
* cleanly instead of racing with our dispatch.
*/
ISC_LIST_UNLINK(rl->pending, rle, link);
isc_async_run(rle->loop, rle->cb, rle->arg);
pertic--;
}
@ -261,11 +263,6 @@ isc__ratelimiter_tick(void *arg) {
}
unlock:
UNLOCK(&rl->lock);
ISC_LIST_FOREACH(pending, rle, link) {
ISC_LIST_UNLINK(pending, rle, link);
isc_async_run(rle->loop, rle->cb, rle->arg);
}
}
void
@ -287,26 +284,20 @@ isc__ratelimiter_doshutdown(void *arg) {
void
isc_ratelimiter_shutdown(isc_ratelimiter_t *restrict rl) {
ISC_LIST(isc_rlevent_t) pending;
REQUIRE(VALID_RATELIMITER(rl));
ISC_LIST_INIT(pending);
LOCK(&rl->lock);
if (rl->state != isc_ratelimiter_shuttingdown) {
rl->state = isc_ratelimiter_shuttingdown;
ISC_LIST_MOVE(pending, rl->pending);
ISC_LIST_FOREACH(rl->pending, rle, link) {
ISC_LIST_UNLINK(rl->pending, rle, link);
rle->canceled = true;
isc_async_run(rl->loop, rle->cb, rle->arg);
}
isc_ratelimiter_ref(rl);
isc_async_run(rl->loop, isc__ratelimiter_doshutdown, rl);
}
UNLOCK(&rl->lock);
ISC_LIST_FOREACH(pending, rle, link) {
ISC_LIST_UNLINK(pending, rle, link);
rle->canceled = true;
isc_async_run(rl->loop, rle->cb, rle->arg);
}
}
static void