mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-09-04 16:45:24 +00:00
Code Issues Releases Wiki Activity

Add attach/detach for the dns_dispatch_send()

Browse Source 
The order in which the netievents are processed on the network manager
loop is not guaranteed.  Therefore the recv/read callback can come
earlier than the send/write callback.

The dns_request API wasn't ready for this reordering and it was
destroying the dns_request_t object before the send callback has been
called.

Add additional attach/detach in the req_send()/req_senddone() functions
to make sure we don't destroy the dns_request_t while it's still being
references by asynchronous call.
This commit is contained in:
Ondřej Surý
2022-03-03 13:30:24 +01:00
parent 488b1a776c
commit f3ca90a804
1 changed files with 49 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

81
lib/dns/request.c
View File

@@ -21,6 +21,7 @@
#include <isc/netmgr.h> #include <isc/netmgr.h>
#include <isc/result.h> #include <isc/result.h>
#include <isc/task.h> #include <isc/task.h>
#include <isc/thread.h>
#include <isc/util.h> #include <isc/util.h>
#include <dns/acl.h> #include <dns/acl.h>
@@ -349,6 +350,9 @@ req_send(dns_request_t *request) {
isc_buffer_usedregion(request->query, &r); isc_buffer_usedregion(request->query, &r);
request->flags |= DNS_REQUEST_F_SENDING; request->flags |= DNS_REQUEST_F_SENDING;
/* detached in req_senddone() */
req_attach(request, &(dns_request_t *){ NULL });
dns_dispatch_send(request->dispentry, &r, request->dscp); dns_dispatch_send(request->dispentry, &r, request->dscp);
} }
@@ -472,8 +476,6 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf,
isc_task_t *task, isc_taskaction_t action, void *arg, isc_task_t *task, isc_taskaction_t action, void *arg,
dns_request_t **requestp) { dns_request_t **requestp) {
dns_request_t *request = NULL; dns_request_t *request = NULL;
isc_task_t *tclone = NULL;
dns_request_t *rclone = NULL;
isc_result_t result; isc_result_t result;
isc_mem_t *mctx = NULL; isc_mem_t *mctx = NULL;
dns_messageid_t id; dns_messageid_t id;
@@ -506,6 +508,7 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf,
return (DNS_R_BLACKHOLED); return (DNS_R_BLACKHOLED);
} }
/* detached in dns_request_destroy() */
result = new_request(mctx, &request); result = new_request(mctx, &request);
if (result != ISC_R_SUCCESS) { if (result != ISC_R_SUCCESS) {
return (result); return (result);
@@ -517,7 +520,7 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf,
request->event = (dns_requestevent_t *)isc_event_allocate( request->event = (dns_requestevent_t *)isc_event_allocate(
mctx, task, DNS_EVENT_REQUESTDONE, action, arg, mctx, task, DNS_EVENT_REQUESTDONE, action, arg,
sizeof(dns_requestevent_t)); sizeof(dns_requestevent_t));
isc_task_attach(task, &tclone); isc_task_attach(task, &(isc_task_t *){ NULL });
request->event->ev_sender = task; request->event->ev_sender = task;
request->event->request = request; request->event->request = request;
request->event->result = ISC_R_FAILURE; request->event->result = ISC_R_FAILURE;
@@ -547,7 +550,11 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf,
goto cleanup; goto cleanup;
} }
/* detached in req_connected() */
req_attach(request, &(dns_request_t *){ NULL });
again: again:
result = get_dispatch(tcp, newtcp, requestmgr, srcaddr, destaddr, dscp, result = get_dispatch(tcp, newtcp, requestmgr, srcaddr, destaddr, dscp,
&connected, &request->dispatch); &connected, &request->dispatch);
if (result != ISC_R_SUCCESS) { if (result != ISC_R_SUCCESS) {
@@ -559,7 +566,6 @@ again:
dispopt |= DNS_DISPATCHOPT_FIXEDID; dispopt |= DNS_DISPATCHOPT_FIXEDID;
} }
req_attach(request, &rclone);
result = dns_dispatch_add(request->dispatch, dispopt, request->timeout, result = dns_dispatch_add(request->dispatch, dispopt, request->timeout,
destaddr, req_connected, req_senddone, destaddr, req_connected, req_senddone,
req_response, request, &id, req_response, request, &id,
@@ -568,11 +574,11 @@ again:
if ((options & DNS_REQUESTOPT_FIXEDID) != 0 && !newtcp) { if ((options & DNS_REQUESTOPT_FIXEDID) != 0 && !newtcp) {
newtcp = true; newtcp = true;
connected = false; connected = false;
req_detach(&rclone);
dns_dispatch_detach(&request->dispatch); dns_dispatch_detach(&request->dispatch);
goto again; goto again;
} }
goto cleanup;
goto detach;
} }
/* Add message ID. */ /* Add message ID. */
@@ -589,7 +595,9 @@ again:
request->destaddr = *destaddr; request->destaddr = *destaddr;
if (tcp && connected) { if (tcp && connected) {
req_send(request); req_send(request);
req_detach(&rclone);
/* no need to call req_connected(), detach here */
req_detach(&(dns_request_t *){ request });
} else { } else {
request->flags |= DNS_REQUEST_F_CONNECTING; request->flags |= DNS_REQUEST_F_CONNECTING;
if (tcp) { if (tcp) {
@@ -611,13 +619,13 @@ unlink:
ISC_LIST_UNLINK(requestmgr->requests, request, link); ISC_LIST_UNLINK(requestmgr->requests, request, link);
UNLOCK(&requestmgr->lock); UNLOCK(&requestmgr->lock);
detach:
/* connect failed, detach here */
req_detach(&(dns_request_t *){ request });
cleanup: cleanup:
if (tclone != NULL) { isc_task_detach(&(isc_task_t *){ task });
isc_task_detach(&tclone); /* final detach to shut down request */
}
if (rclone != NULL) {
req_detach(&rclone);
}
req_detach(&request); req_detach(&request);
req_log(ISC_LOG_DEBUG(3), "dns_request_createraw: failed %s", req_log(ISC_LOG_DEBUG(3), "dns_request_createraw: failed %s",
isc_result_totext(result)); isc_result_totext(result));
@@ -645,8 +653,6 @@ dns_request_createvia(dns_requestmgr_t *requestmgr, dns_message_t *message,
isc_taskaction_t action, void *arg, isc_taskaction_t action, void *arg,
dns_request_t **requestp) { dns_request_t **requestp) {
dns_request_t *request = NULL; dns_request_t *request = NULL;
isc_task_t *tclone = NULL;
dns_request_t *rclone = NULL;
isc_result_t result; isc_result_t result;
isc_mem_t *mctx = NULL; isc_mem_t *mctx = NULL;
dns_messageid_t id; dns_messageid_t id;
@@ -678,6 +684,7 @@ dns_request_createvia(dns_requestmgr_t *requestmgr, dns_message_t *message,
return (DNS_R_BLACKHOLED); return (DNS_R_BLACKHOLED);
} }
/* detached in dns_request_destroy() */
result = new_request(mctx, &request); result = new_request(mctx, &request);
if (result != ISC_R_SUCCESS) { if (result != ISC_R_SUCCESS) {
return (result); return (result);
@@ -689,10 +696,11 @@ dns_request_createvia(dns_requestmgr_t *requestmgr, dns_message_t *message,
request->event = (dns_requestevent_t *)isc_event_allocate( request->event = (dns_requestevent_t *)isc_event_allocate(
mctx, task, DNS_EVENT_REQUESTDONE, action, arg, mctx, task, DNS_EVENT_REQUESTDONE, action, arg,
sizeof(dns_requestevent_t)); sizeof(dns_requestevent_t));
isc_task_attach(task, &tclone); isc_task_attach(task, &(isc_task_t *){ NULL });
request->event->ev_sender = task; request->event->ev_sender = task;
request->event->request = request; request->event->request = request;
request->event->result = ISC_R_FAILURE; request->event->result = ISC_R_FAILURE;
if (key != NULL) { if (key != NULL) {
dns_tsigkey_attach(key, &request->tsigkey); dns_tsigkey_attach(key, &request->tsigkey);
} }
@@ -715,19 +723,21 @@ dns_request_createvia(dns_requestmgr_t *requestmgr, dns_message_t *message,
request->timeout = udptimeout * 1000; request->timeout = udptimeout * 1000;
} }
use_tcp: /* detached in req_connected() */
req_attach(request, &(dns_request_t *){ NULL });
again:
result = get_dispatch(tcp, false, requestmgr, srcaddr, destaddr, dscp, result = get_dispatch(tcp, false, requestmgr, srcaddr, destaddr, dscp,
&connected, &request->dispatch); &connected, &request->dispatch);
if (result != ISC_R_SUCCESS) { if (result != ISC_R_SUCCESS) {
goto cleanup; goto detach;
} }
req_attach(request, &rclone);
result = dns_dispatch_add( result = dns_dispatch_add(
request->dispatch, 0, request->timeout, destaddr, req_connected, request->dispatch, 0, request->timeout, destaddr, req_connected,
req_senddone, req_response, request, &id, &request->dispentry); req_senddone, req_response, request, &id, &request->dispentry);
if (result != ISC_R_SUCCESS) { if (result != ISC_R_SUCCESS) {
goto cleanup; goto detach;
} }
message->id = id; message->id = id;
@@ -736,21 +746,20 @@ use_tcp:
/* /*
* Try again using TCP. * Try again using TCP.
*/ */
req_detach(&rclone);
dns_message_renderreset(message); dns_message_renderreset(message);
dns_dispatch_done(&request->dispentry); dns_dispatch_done(&request->dispentry);
dns_dispatch_detach(&request->dispatch); dns_dispatch_detach(&request->dispatch);
options |= DNS_REQUESTOPT_TCP; options |= DNS_REQUESTOPT_TCP;
tcp = true; tcp = true;
goto use_tcp; goto again;
} }
if (result != ISC_R_SUCCESS) { if (result != ISC_R_SUCCESS) {
goto cleanup; goto detach;
} }
result = dns_message_getquerytsig(message, mctx, &request->tsig); result = dns_message_getquerytsig(message, mctx, &request->tsig);
if (result != ISC_R_SUCCESS) { if (result != ISC_R_SUCCESS) {
goto cleanup; goto detach;
} }
LOCK(&requestmgr->lock); LOCK(&requestmgr->lock);
@@ -762,7 +771,9 @@ use_tcp:
request->destaddr = *destaddr; request->destaddr = *destaddr;
if (tcp && connected) { if (tcp && connected) {
req_send(request); req_send(request);
req_detach(&rclone);
/* no need to call req_connected(), detach here */
req_detach(&(dns_request_t *){ request });
} else { } else {
request->flags |= DNS_REQUEST_F_CONNECTING; request->flags |= DNS_REQUEST_F_CONNECTING;
if (tcp) { if (tcp) {
@@ -784,13 +795,13 @@ unlink:
ISC_LIST_UNLINK(requestmgr->requests, request, link); ISC_LIST_UNLINK(requestmgr->requests, request, link);
UNLOCK(&requestmgr->lock); UNLOCK(&requestmgr->lock);
detach:
/* connect failed, detach here */
req_detach(&(dns_request_t *){ request });
cleanup: cleanup:
if (tclone != NULL) { isc_task_detach(&(isc_task_t *){ task });
isc_task_detach(&tclone); /* final detach to shut down request */
}
if (rclone != NULL) {
req_detach(&rclone);
}
req_detach(&request); req_detach(&request);
req_log(ISC_LOG_DEBUG(3), "dns_request_createvia: failed %s", req_log(ISC_LOG_DEBUG(3), "dns_request_createvia: failed %s",
isc_result_totext(result)); isc_result_totext(result));
@@ -987,6 +998,7 @@ dns_request_destroy(dns_request_t **requestp) {
INSIST(request->dispentry == NULL); INSIST(request->dispentry == NULL);
INSIST(request->dispatch == NULL); INSIST(request->dispatch == NULL);
/* final detach to shut down request */
req_detach(&request); req_detach(&request);
} }
@@ -1020,7 +1032,8 @@ req_connected(isc_result_t eresult, isc_region_t *region, void *arg) {
} }
UNLOCK(&request->requestmgr->locks[request->hash]); UNLOCK(&request->requestmgr->locks[request->hash]);
req_detach(&request); /* attached in dns_request_createvia/_createraw() */
req_detach(&(dns_request_t *){ request });
} }
static void static void
@@ -1049,6 +1062,9 @@ req_senddone(isc_result_t eresult, isc_region_t *region, void *arg) {
} }
UNLOCK(&request->requestmgr->locks[request->hash]); UNLOCK(&request->requestmgr->locks[request->hash]);
/* attached in req_send() */
req_detach(&request);
} }
static void static void
@@ -1129,6 +1145,7 @@ req_sendevent(dns_request_t *request, isc_result_t result) {
task = request->event->ev_sender; task = request->event->ev_sender;
request->event->ev_sender = request; request->event->ev_sender = request;
request->event->result = result; request->event->result = result;
isc_task_sendanddetach(&task, (isc_event_t **)&request->event); isc_task_sendanddetach(&task, (isc_event_t **)&request->event);
} }