mirror of
https://github.com/isc-projects/bind9.git
synced 2026-05-28 04:34:54 -04:00
[9.20] fix: dev: Take dns_dtenv_t reference before an async function call
A 'dns_dtenv_t' pointer is passed to an async function without taking a reference first, which can potentially cause a use-after-free error. Take a reference, then detach in the async function. Closes #5820 Backport of MR !11705 Merge branch 'backport-5820-dns_dtenv-reference-bug-fix-9.20' into 'bind-9.20' See merge request isc-projects/bind9!11714
This commit is contained in:
Arаm Sаrgsyаn
commit
be7b811fff
5 changed files with 38 additions and 62 deletions
|
|
@ -4085,7 +4085,7 @@ configure_dnstap(const cfg_obj_t **maps, dns_view_t *view) {
|
|||
cfg_obj_asstring(obj));
|
||||
}
|
||||
|
||||
dns_dt_attach(named_g_server->dtenv, &view->dtenv);
|
||||
dns_dtenv_attach(named_g_server->dtenv, &view->dtenv);
|
||||
view->dttypes = dttypes;
|
||||
|
||||
result = ISC_R_SUCCESS;
|
||||
|
|
@ -10613,7 +10613,7 @@ named_server_destroy(named_server_t **serverp) {
|
|||
|
||||
#ifdef HAVE_DNSTAP
|
||||
if (server->dtenv != NULL) {
|
||||
dns_dt_detach(&server->dtenv);
|
||||
dns_dtenv_detach(&server->dtenv);
|
||||
}
|
||||
#endif /* HAVE_DNSTAP */
|
||||
|
||||
|
|
|
|||
|
|
@ -100,7 +100,7 @@ struct dns_dthandle {
|
|||
|
||||
struct dns_dtenv {
|
||||
unsigned int magic;
|
||||
isc_refcount_t refcount;
|
||||
isc_refcount_t references;
|
||||
|
||||
isc_mem_t *mctx;
|
||||
isc_loop_t *loop;
|
||||
|
|
@ -126,10 +126,19 @@ typedef struct ioq {
|
|||
struct fstrm_iothr_queue *ioq;
|
||||
} dt__ioq_t;
|
||||
|
||||
static void
|
||||
destroy(dns_dtenv_t *env);
|
||||
|
||||
static thread_local dt__ioq_t dt_ioq = { 0 };
|
||||
|
||||
static atomic_uint_fast32_t global_generation;
|
||||
|
||||
#if DNS_DTENV_TRACE
|
||||
ISC_REFCOUNT_TRACE_IMPL(dns_dtenv, destroy);
|
||||
#else
|
||||
ISC_REFCOUNT_IMPL(dns_dtenv, destroy);
|
||||
#endif /* DNS_DTENV_TRACE */
|
||||
|
||||
isc_result_t
|
||||
dns_dt_create(isc_mem_t *mctx, dns_dtmode_t mode, const char *path,
|
||||
struct fstrm_iothr_options **foptp, isc_loop_t *loop,
|
||||
|
|
@ -160,7 +169,7 @@ dns_dt_create(isc_mem_t *mctx, dns_dtmode_t mode, const char *path,
|
|||
isc_mem_attach(mctx, &env->mctx);
|
||||
isc_mutex_init(&env->reopen_lock);
|
||||
env->path = isc_mem_strdup(env->mctx, path);
|
||||
isc_refcount_init(&env->refcount, 1);
|
||||
isc_refcount_init(&env->references, 1);
|
||||
isc_stats_create(env->mctx, &env->stats, dns_dnstapcounter_max);
|
||||
|
||||
fwopt = fstrm_writer_options_init();
|
||||
|
|
@ -444,15 +453,6 @@ dt_queue(dns_dtenv_t *env) {
|
|||
return dt_ioq.ioq;
|
||||
}
|
||||
|
||||
void
|
||||
dns_dt_attach(dns_dtenv_t *source, dns_dtenv_t **destp) {
|
||||
REQUIRE(VALID_DTENV(source));
|
||||
REQUIRE(destp != NULL && *destp == NULL);
|
||||
|
||||
isc_refcount_increment(&source->refcount);
|
||||
*destp = source;
|
||||
}
|
||||
|
||||
isc_result_t
|
||||
dns_dt_getstats(dns_dtenv_t *env, isc_stats_t **statsp) {
|
||||
REQUIRE(VALID_DTENV(env));
|
||||
|
|
@ -498,18 +498,6 @@ destroy(dns_dtenv_t *env) {
|
|||
isc_mem_putanddetach(&env->mctx, env, sizeof(*env));
|
||||
}
|
||||
|
||||
void
|
||||
dns_dt_detach(dns_dtenv_t **envp) {
|
||||
REQUIRE(envp != NULL && VALID_DTENV(*envp));
|
||||
dns_dtenv_t *env = *envp;
|
||||
*envp = NULL;
|
||||
|
||||
if (isc_refcount_decrement(&env->refcount) == 1) {
|
||||
isc_refcount_destroy(&env->refcount);
|
||||
destroy(env);
|
||||
}
|
||||
}
|
||||
|
||||
static isc_result_t
|
||||
pack_dt(const Dnstap__Dnstap *d, void **buf, size_t *sz) {
|
||||
ProtobufCBufferSimple sbuf;
|
||||
|
|
@ -697,6 +685,8 @@ perform_reopen(void *arg) {
|
|||
LOCK(&env->reopen_lock);
|
||||
env->reopen_queued = false;
|
||||
UNLOCK(&env->reopen_lock);
|
||||
|
||||
dns_dtenv_detach(&env);
|
||||
}
|
||||
|
||||
/*%
|
||||
|
|
@ -728,6 +718,7 @@ check_file_size_and_maybe_reopen(dns_dtenv_t *env) {
|
|||
* Send an event to roll the output file, then disallow output file
|
||||
* rolling until the roll we queue is completed.
|
||||
*/
|
||||
dns_dtenv_ref(env);
|
||||
isc_async_run(env->loop, perform_reopen, env);
|
||||
env->reopen_queued = true;
|
||||
|
||||
|
|
|
|||
|
|
@ -118,6 +118,21 @@ struct dns_dtdata {
|
|||
};
|
||||
#endif /* HAVE_DNSTAP */
|
||||
|
||||
#if DNS_DTENV_TRACE
|
||||
#define dns_dtenv_ref(ptr) dns_dtenv__ref(ptr, __func__, __FILE__, __LINE__)
|
||||
#define dns_dtenv_unref(ptr) dns_dtenv__unref(ptr, __func__, __FILE__, __LINE__)
|
||||
#define dns_dtenv_attach(ptr, ptrp) \
|
||||
dns_dtenv__attach(ptr, ptrp, __func__, __FILE__, __LINE__)
|
||||
#define dns_dtenv_detach(ptrp) \
|
||||
dns_dtenv__detach(ptrp, __func__, __FILE__, __LINE__)
|
||||
ISC_REFCOUNT_TRACE_DECL(dns_dtenv);
|
||||
#else
|
||||
ISC_REFCOUNT_DECL(dns_dtenv);
|
||||
#endif /* DNS_DTENV_TRACE */
|
||||
/*%
|
||||
* Reference counting for dns_dtenv
|
||||
*/
|
||||
|
||||
isc_result_t
|
||||
dns_dt_create(isc_mem_t *mctx, dns_dtmode_t mode, const char *path,
|
||||
struct fstrm_iothr_options **foptp, isc_loop_t *loop,
|
||||
|
|
@ -215,36 +230,6 @@ dns_dt_setversion(dns_dtenv_t *env, const char *version);
|
|||
*\li 'env' is a valid dnstap environment.
|
||||
*/
|
||||
|
||||
void
|
||||
dns_dt_attach(dns_dtenv_t *source, dns_dtenv_t **destp);
|
||||
/*%<
|
||||
* Attach '*destp' to 'source', incrementing the reference counter.
|
||||
*
|
||||
* Requires:
|
||||
*
|
||||
*\li 'source' is a valid dnstap environment.
|
||||
*
|
||||
*\li 'destp' is not NULL and '*destp' is NULL.
|
||||
*
|
||||
*\li *destp is attached to source.
|
||||
*/
|
||||
|
||||
void
|
||||
dns_dt_detach(dns_dtenv_t **envp);
|
||||
/*%<
|
||||
* Detach '*envp', decrementing the reference counter.
|
||||
*
|
||||
* Requires:
|
||||
*
|
||||
*\li '*envp' is a valid dnstap environment.
|
||||
*
|
||||
* Ensures:
|
||||
*
|
||||
*\li '*envp' will be destroyed when the number of references reaches zero.
|
||||
*
|
||||
*\li '*envp' is NULL.
|
||||
*/
|
||||
|
||||
isc_result_t
|
||||
dns_dt_getstats(dns_dtenv_t *env, isc_stats_t **statsp);
|
||||
/*%<
|
||||
|
|
|
|||
|
|
@ -377,7 +377,7 @@ destroy(dns_view_t *view) {
|
|||
}
|
||||
#ifdef HAVE_DNSTAP
|
||||
if (view->dtenv != NULL) {
|
||||
dns_dt_detach(&view->dtenv);
|
||||
dns_dtenv_detach(&view->dtenv);
|
||||
}
|
||||
#endif /* HAVE_DNSTAP */
|
||||
dns_view_setnewzones(view, false, NULL, NULL, 0ULL);
|
||||
|
|
|
|||
|
|
@ -90,7 +90,7 @@ ISC_LOOP_TEST_IMPL(dns_dt_create) {
|
|||
&dtenv);
|
||||
assert_int_equal(result, ISC_R_SUCCESS);
|
||||
if (dtenv != NULL) {
|
||||
dns_dt_detach(&dtenv);
|
||||
dns_dtenv_detach(&dtenv);
|
||||
}
|
||||
if (fopt != NULL) {
|
||||
fstrm_iothr_options_destroy(&fopt);
|
||||
|
|
@ -106,7 +106,7 @@ ISC_LOOP_TEST_IMPL(dns_dt_create) {
|
|||
&dtenv);
|
||||
assert_int_equal(result, ISC_R_SUCCESS);
|
||||
if (dtenv != NULL) {
|
||||
dns_dt_detach(&dtenv);
|
||||
dns_dtenv_detach(&dtenv);
|
||||
}
|
||||
if (fopt != NULL) {
|
||||
fstrm_iothr_options_destroy(&fopt);
|
||||
|
|
@ -123,7 +123,7 @@ ISC_LOOP_TEST_IMPL(dns_dt_create) {
|
|||
assert_int_equal(result, ISC_R_FAILURE);
|
||||
assert_null(dtenv);
|
||||
if (dtenv != NULL) {
|
||||
dns_dt_detach(&dtenv);
|
||||
dns_dtenv_detach(&dtenv);
|
||||
}
|
||||
if (fopt != NULL) {
|
||||
fstrm_iothr_options_destroy(&fopt);
|
||||
|
|
@ -169,7 +169,7 @@ ISC_LOOP_TEST_IMPL(dns_dt_send) {
|
|||
&dtenv);
|
||||
assert_int_equal(result, ISC_R_SUCCESS);
|
||||
|
||||
dns_dt_attach(dtenv, &view->dtenv);
|
||||
dns_dtenv_attach(dtenv, &view->dtenv);
|
||||
view->dttypes = DNS_DTTYPE_ALL;
|
||||
|
||||
/*
|
||||
|
|
@ -258,8 +258,8 @@ ISC_LOOP_TEST_IMPL(dns_dt_send) {
|
|||
m);
|
||||
}
|
||||
|
||||
dns_dt_detach(&view->dtenv);
|
||||
dns_dt_detach(&dtenv);
|
||||
dns_dtenv_detach(&view->dtenv);
|
||||
dns_dtenv_detach(&dtenv);
|
||||
dns_view_detach(&view);
|
||||
|
||||
result = dns_dt_open(TAPFILE, dns_dtmode_file, mctx, &handle);
|
||||
|
|
|
|||
Loading…
Reference in a new issue