upstream/bind9
1
0
Fork 0
mirror of https://github.com/isc-projects/bind9.git synced 2026-05-28 04:34:54 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Merge branch '3178-dispatch-race-v9_18' into 'v9_18'

Browse source 
[9.18] Fix the thread safety in the dns_dispatch unit

See merge request isc-projects/bind9!7251
This commit is contained in:
Ondřej Surý 2022-12-21 12:41:22 +00:00
commit 6fd6686865
7 changed files with 1091 additions and 765 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CHANGES
View file

@ -1,3 +1,6 @@
6051. [bug] Improve thread safety in the dns_dispatch unit.
[GL #3178] [GL #3636]
6050. [bug] Changes to the RPZ response-policy min-update-interval
and add-soa options now take effect as expected when
named is reconfigured. [GL #3740]

3
doc/notes/notes-current.rst
View file

@ -49,6 +49,9 @@ Bug Fixes
structures used for "housekeeping") and exclude recently used (<= 10 seconds)
ADB names and entries from the overmem memory cleaner. :gl:`#3739`
- Fix a rare assertion failure in the outgoing TCP DNS connection handling.
:gl:`#3178` :gl:`#3636`
Known Issues
~~~~~~~~~~~~

1626
lib/dns/dispatch.c
View file

File diff suppressed because it is too large Load diff

84
lib/dns/include/dns/dispatch.h
View file

@ -53,10 +53,13 @@
#include <isc/lang.h>
#include <isc/mutex.h>
#include <isc/netmgr.h>
#include <isc/refcount.h>
#include <isc/types.h>
#include <dns/types.h>
#undef DNS_DISPATCH_TRACE
ISC_LANG_BEGINDECLS
/*%
@ -94,25 +97,22 @@ dns_dispatchmgr_create(isc_mem_t *mctx, isc_nm_t *nm, dns_dispatchmgr_t **mgrp);
*\li anything else -- failure
*/
void
dns_dispatchmgr_attach(dns_dispatchmgr_t *mgr, dns_dispatchmgr_t **mgrp);
/*%<
* Attach to a dispatch manger.
*
* Requires:
*\li is valid.
*
*\li mgrp != NULL && *mgrp == NULL
*/
#if DNS_DISPATCH_TRACE
#define dns_dispatchmgr_ref(ptr) \
dns_dispatchmgr__ref(ptr, __func__, __FILE__, __LINE__)
#define dns_dispatchmgr_unref(ptr) \
dns_dispatchmgr__unref(ptr, __func__, __FILE__, __LINE__)
#define dns_dispatchmgr_attach(ptr, ptrp) \
dns_dispatchmgr__attach(ptr, ptrp, __func__, __FILE__, __LINE__)
#define dns_dispatchmgr_detach(ptrp) \
dns_dispatchmgr__detach(ptrp, __func__, __FILE__, __LINE__)
ISC_REFCOUNT_TRACE_DECL(dns_dispatchmgr);
#else
ISC_REFCOUNT_DECL(dns_dispatchmgr);
#endif
void
dns_dispatchmgr_detach(dns_dispatchmgr_t **mgrp);
/*%<
* Detach from the dispatch manager, and destroy it if no references
* remain.
*
* Requires:
*\li mgrp != NULL && *mgrp is a valid dispatchmgr.
* Attach/Detach to a dispatch manager.
*/
void
@ -201,10 +201,21 @@ dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *localaddr,
*\li Anything else -- failure.
*/
void
dns_dispatch_attach(dns_dispatch_t *disp, dns_dispatch_t **dispp);
#if DNS_DISPATCH_TRACE
#define dns_dispatch_ref(ptr) \
dns_dispatch__ref(ptr, __func__, __FILE__, __LINE__)
#define dns_dispatch_unref(ptr) \
dns_dispatch__unref(ptr, __func__, __FILE__, __LINE__)
#define dns_dispatch_attach(ptr, ptrp) \
dns_dispatch__attach(ptr, ptrp, __func__, __FILE__, __LINE__)
#define dns_dispatch_detach(ptrp) \
dns_dispatch__detach(ptrp, __func__, __FILE__, __LINE__)
ISC_REFCOUNT_TRACE_DECL(dns_dispatch);
#else
ISC_REFCOUNT_DECL(dns_dispatch);
#endif
/*%<
* Attach to a dispatch handle.
* Attach/Detach to a dispatch handle.
*
* Requires:
*\li disp is valid.
@ -212,15 +223,6 @@ dns_dispatch_attach(dns_dispatch_t *disp, dns_dispatch_t **dispp);
*\li dispp != NULL && *dispp == NULL
*/
void
dns_dispatch_detach(dns_dispatch_t **dispp);
/*%<
* Detaches from the dispatch.
*
* Requires:
*\li dispp != NULL and *dispp be a valid dispatch.
*/
isc_result_t
dns_dispatch_connect(dns_dispentry_t *resp);
/*%<
@ -253,11 +255,9 @@ dns_dispatch_resume(dns_dispentry_t *resp, uint16_t timeout);
isc_result_t
dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr,
const isc_sockaddr_t *localaddr, bool *connected,
dns_dispatch_t **dispp);
const isc_sockaddr_t *localaddr, dns_dispatch_t **dispp);
/*
* Attempt to connect to a existing TCP connection (connection completed
* if connected == NULL).
* Attempt to connect to a existing TCP connection.
*/
typedef void (*dispatch_cb_t)(isc_result_t eresult, isc_region_t *region,
@ -307,22 +307,10 @@ dns_dispatch_add(dns_dispatch_t *disp, unsigned int options,
void
dns_dispatch_done(dns_dispentry_t **respp);
/*%<
* Disconnects a dispatch response entry from its dispatch and shuts it
* down. This is called when the dispatch is complete; use
* dns_dispatch_cancel() if it is still pending.
*
* Requires:
*\li "resp" != NULL and "*resp" contain a value previously allocated
* by dns_dispatch_add();
*/
/*<
* Disconnect a dispatch response entry from its dispatch, cancel all
* pending connects and reads in a dispatch entry and shut it down.
void
dns_dispatch_cancel(dns_dispentry_t **respp);
/*%<
* Cancel all pending connects and reads in a dispatch entry,
* then call dns_dispatch_done(). This is used when the caller
* cancels a dispatch response before it has completed.
*
* Requires:
*\li "resp" != NULL and "*resp" contain a value previously allocated

46
lib/dns/request.c
View file

@ -400,20 +400,18 @@ isblackholed(dns_dispatchmgr_t *dispatchmgr, const isc_sockaddr_t *destaddr) {
static isc_result_t
tcp_dispatch(bool newtcp, dns_requestmgr_t *requestmgr,
const isc_sockaddr_t *srcaddr, const isc_sockaddr_t *destaddr,
isc_dscp_t dscp, bool *connected, dns_dispatch_t **dispatchp) {
isc_dscp_t dscp, dns_dispatch_t **dispatchp) {
isc_result_t result;
if (!newtcp) {
result = dns_dispatch_gettcp(requestmgr->dispatchmgr, destaddr,
srcaddr, connected, dispatchp);
srcaddr, dispatchp);
if (result == ISC_R_SUCCESS) {
char peer[ISC_SOCKADDR_FORMATSIZE];
isc_sockaddr_format(destaddr, peer, sizeof(peer));
req_log(ISC_LOG_DEBUG(1),
"attached to %s TCP "
"connection to %s",
*connected ? "existing" : "pending", peer);
"attached to TCP connection to %s", peer);
return (result);
}
}
@ -455,12 +453,12 @@ udp_dispatch(dns_requestmgr_t *requestmgr, const isc_sockaddr_t *srcaddr,
static isc_result_t
get_dispatch(bool tcp, bool newtcp, dns_requestmgr_t *requestmgr,
const isc_sockaddr_t *srcaddr, const isc_sockaddr_t *destaddr,
isc_dscp_t dscp, bool *connected, dns_dispatch_t **dispatchp) {
isc_dscp_t dscp, dns_dispatch_t **dispatchp) {
isc_result_t result;
if (tcp) {
result = tcp_dispatch(newtcp, requestmgr, srcaddr, destaddr,
dscp, connected, dispatchp);
dscp, dispatchp);
} else {
result = udp_dispatch(requestmgr, srcaddr, destaddr, dispatchp);
}
@ -482,7 +480,6 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf,
bool tcp = false;
bool newtcp = false;
isc_region_t r;
bool connected = false;
unsigned int dispopt = 0;
REQUIRE(VALID_REQUESTMGR(requestmgr));
@ -556,7 +553,7 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf,
again:
result = get_dispatch(tcp, newtcp, requestmgr, srcaddr, destaddr, dscp,
&connected, &request->dispatch);
&request->dispatch);
if (result != ISC_R_SUCCESS) {
goto detach;
}
@ -573,7 +570,6 @@ again:
if (result != ISC_R_SUCCESS) {
if ((options & DNS_REQUESTOPT_FIXEDID) != 0 && !newtcp) {
newtcp = true;
connected = false;
dns_dispatch_detach(&request->dispatch);
goto again;
}
@ -593,21 +589,15 @@ again:
UNLOCK(&requestmgr->lock);
request->destaddr = *destaddr;
if (tcp && connected) {
req_send(request);
/* no need to call req_connected(), detach here */
req_detach(&(dns_request_t *){ request });
} else {
request->flags |= DNS_REQUEST_F_CONNECTING;
if (tcp) {
request->flags |= DNS_REQUEST_F_TCP;
}
request->flags |= DNS_REQUEST_F_CONNECTING;
if (tcp) {
request->flags |= DNS_REQUEST_F_TCP;
}
result = dns_dispatch_connect(request->dispentry);
if (result != ISC_R_SUCCESS) {
goto unlink;
}
result = dns_dispatch_connect(request->dispentry);
if (result != ISC_R_SUCCESS) {
goto unlink;
}
req_log(ISC_LOG_DEBUG(3), "dns_request_createraw: request %p", request);
@ -718,7 +708,7 @@ dns_request_create(dns_requestmgr_t *requestmgr, dns_message_t *message,
again:
result = get_dispatch(tcp, false, requestmgr, srcaddr, destaddr, dscp,
&connected, &request->dispatch);
&request->dispatch);
if (result != ISC_R_SUCCESS) {
goto detach;
}
@ -903,7 +893,7 @@ request_cancel(dns_request_t *request) {
request->flags &= ~DNS_REQUEST_F_CONNECTING;
if (request->dispentry != NULL) {
dns_dispatch_cancel(&request->dispentry);
dns_dispatch_done(&request->dispentry);
}
dns_dispatch_detach(&request->dispatch);
@ -1061,13 +1051,13 @@ static void
req_response(isc_result_t result, isc_region_t *region, void *arg) {
dns_request_t *request = (dns_request_t *)arg;
req_log(ISC_LOG_DEBUG(3), "req_response: request %p: %s", request,
isc_result_totext(result));
if (result == ISC_R_CANCELED) {
return;
}
req_log(ISC_LOG_DEBUG(3), "req_response: request %p: %s", request,
isc_result_totext(result));
if (result == ISC_R_TIMEDOUT) {
LOCK(&request->requestmgr->locks[request->hash]);
if (request->udpcount != 0) {

2
lib/dns/resolver.c
View file

@ -1499,7 +1499,7 @@ fctx_cancelquery(resquery_t **queryp, isc_time_t *finish, bool no_response,
* exist, cancel them.
*/
if (query->dispentry != NULL) {
dns_dispatch_cancel(&query->dispentry);
dns_dispatch_done(&query->dispentry);
}
LOCK(&fctx->res->buckets[fctx->bucketnum].lock);

92
lib/isc/include/isc/refcount.h
View file

@ -149,4 +149,96 @@ isc_refcount_decrement(isc_refcount_t *target) {
ISC_INSIST(_refs > 0); \
} while (0)
#define ISC_REFCOUNT_TRACE_DECL(name) \
name##_t *name##__ref(name##_t *ptr, const char *func, \
const char *file, unsigned int line); \
void name##__unref(name##_t *ptr, const char *func, const char *file, \
unsigned int line); \
void name##__attach(name##_t *ptr, name##_t **ptrp, const char *func, \
const char *file, unsigned int line); \
void name##__detach(name##_t **ptrp, const char *func, \
const char *file, unsigned int line)
#define ISC_REFCOUNT_TRACE_IMPL(name, destroy) \
name##_t *name##__ref(name##_t *ptr, const char *func, \
const char *file, unsigned int line) { \
REQUIRE(ptr != NULL); \
uint_fast32_t refs = \
isc_refcount_increment(&ptr->references) + 1; \
fprintf(stderr, \
"%s:%s:%s:%u:%p->references = %" PRIuFAST32 "\n", \
__func__, func, file, line, ptr, refs); \
return (ptr); \
} \
\
void name##__unref(name##_t *ptr, const char *func, const char *file, \
unsigned int line) { \
REQUIRE(ptr != NULL); \
uint_fast32_t refs = \
isc_refcount_decrement(&ptr->references) - 1; \
if (refs == 0) { \
destroy(ptr); \
} \
fprintf(stderr, \
"%s:%s:%s:%u:%p->references = %" PRIuFAST32 "\n", \
__func__, func, file, line, ptr, refs); \
} \
void name##__attach(name##_t *ptr, name##_t **ptrp, const char *func, \
const char *file, unsigned int line) { \
REQUIRE(ptrp != NULL && *ptrp == NULL); \
uint_fast32_t refs = \
isc_refcount_increment(&ptr->references) + 1; \
fprintf(stderr, \
"%s:%s:%s:%u:%p->references = %" PRIuFAST32 "\n", \
__func__, func, file, line, ptr, refs); \
*ptrp = ptr; \
} \
\
void name##__detach(name##_t **ptrp, const char *func, \
const char *file, unsigned int line) { \
REQUIRE(ptrp != NULL && *ptrp != NULL); \
name##_t *ptr = *ptrp; \
*ptrp = NULL; \
uint_fast32_t refs = \
isc_refcount_decrement(&ptr->references) - 1; \
if (refs == 0) { \
destroy(ptr); \
} \
fprintf(stderr, \
"%s:%s:%s:%u:%p->references = %" PRIuFAST32 "\n", \
__func__, func, file, line, ptr, refs); \
}
#define ISC_REFCOUNT_DECL(name) \
name##_t *name##_ref(name##_t *ptr); \
void name##_unref(name##_t *ptr); \
void name##_attach(name##_t *ptr, name##_t **ptrp); \
void name##_detach(name##_t **ptrp)
#define ISC_REFCOUNT_IMPL(name, destroy) \
name##_t *name##_ref(name##_t *ptr) { \
REQUIRE(ptr != NULL); \
isc_refcount_increment(&ptr->references); \
return (ptr); \
} \
\
void name##_unref(name##_t *ptr) { \
REQUIRE(ptr != NULL); \
if (isc_refcount_decrement(&ptr->references) == 1) { \
destroy(ptr); \
} \
} \
void name##_attach(name##_t *ptr, name##_t **ptrp) { \
REQUIRE(ptrp != NULL && *ptrp == NULL); \
name##_ref(ptr); \
*ptrp = ptr; \
} \
\
void name##_detach(name##_t **ptrp) { \
REQUIRE(ptrp != NULL && *ptrp != NULL); \
name##_t *ptr = *ptrp; \
*ptrp = NULL; \
name##_unref(ptr); \
}
ISC_LANG_ENDDECLS