[9.18] fix: dev: Prevent a reference leak when using plugins

The `NS_QUERY_DONE_BEGIN` and `NS_QUERY_DONE_SEND` plugin hooks could cause a reference leak if they returned `NS_HOOK_RETURN` without cleaning up the query context properly.

Closes #2094

Backport of MR !9971

Merge branch 'backport-2094-plugin-reference-leak-9.18' into 'bind-9.18'

See merge request isc-projects/bind9!10171
This commit is contained in:
Evan Hunt 2025-02-26 00:59:10 +00:00
commit 8d0d08ec00
4 changed files with 50 additions and 4 deletions

View file

@ -159,6 +159,7 @@ struct query_ctx {
ns_client_t *client; /* client object */
bool detach_client; /* client needs detaching */
bool async; /* asynchronous hook running */
dns_fetchevent_t *event; /* recursion event */

View file

@ -5308,6 +5308,9 @@ qctx_clean(query_ctx_t *qctx) {
if (qctx->db != NULL && qctx->node != NULL) {
dns_db_detachnode(qctx->db, &qctx->node);
}
if (qctx->client != NULL && qctx->client->query.gluedb != NULL) {
dns_db_detach(&qctx->client->query.gluedb);
}
}
/*%
@ -7037,6 +7040,9 @@ ns_query_hookasync(query_ctx_t *qctx, ns_query_starthookasync_t runasync,
goto cleanup_and_detach_from_quota;
}
/* Record that an asynchronous copy of the qctx has been started */
qctx->async = true;
/*
* Typically the runasync() function will trigger recursion, but
* there is no need to set NS_QUERYATTR_RECURSING. The calling hook
@ -11972,10 +11978,6 @@ ns_query_done(query_ctx_t *qctx) {
qctx_clean(qctx);
qctx_freedata(qctx);
if (qctx->client->query.gluedb != NULL) {
dns_db_detach(&qctx->client->query.gluedb);
}
/*
* Clear the AA bit if we're not authoritative.
*/
@ -12112,6 +12114,17 @@ ns_query_done(query_ctx_t *qctx) {
return qctx->result;
cleanup:
/*
* We'd only get here if one of the hooks above
* (NS_QUERY_DONE_BEGIN or NS_QUERY_DONE_SEND) returned
* NS_HOOK_RETURN. Some housekeeping may be needed.
*/
qctx_clean(qctx);
qctx_freedata(qctx);
if (!qctx->async) {
qctx->detach_client = true;
query_error(qctx->client, DNS_R_SERVFAIL, __LINE__);
}
return result;
}

View file

@ -20,6 +20,10 @@ check_PROGRAMS = \
plugin_test \
query_test
query_test_SOURCES = \
query_test.c \
netmgr_wrap.c
EXTRA_DIST = testdata
include $(top_srcdir)/Makefile.tests

28
tests/ns/netmgr_wrap.c Normal file
View file

@ -0,0 +1,28 @@
/*
* Copyright (C) Internet Systems Consortium, Inc. ("ISC")
*
* SPDX-License-Identifier: MPL-2.0
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, you can obtain one at https://mozilla.org/MPL/2.0/.
*
* See the COPYRIGHT file distributed with this work for additional
* information regarding copyright ownership.
*/
/*! \file */
#include <isc/atomic.h>
#include <isc/netmgr.h>
#include <isc/util.h>
#include <dns/view.h>
#include <ns/client.h>
void
ns_client_error(ns_client_t *client ISC_ATTR_UNUSED,
isc_result_t result ISC_ATTR_UNUSED) {
return;
}