ComputerSurge/bind9
3
0
Fork 0
Code Pull Requests 1 Activity

Prevent the double xfrin_fail() call

Browse Source 
When we are reading from the xfrin socket, and the transfer would be
shutdown, the shutdown function would call `xfrin_fail()` which in turns
calls `xfrin_cancelio()` that causes the read callback to be invoked
with `ISC_R_CANCELED` status code and that caused yet another
`xfrin_fail()` call.

The fix here is to ensure the `xfrin_fail()` would be run only once
properly using better synchronization on xfr->shuttingdown flag.
This commit is contained in:
Ondřej Surý
2021-04-16 11:08:05 +02:00
parent 27960e4208
commit 6cf6de55bc
1 changed files with 39 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

71
lib/dns/xfrin.c
View File

@@ -103,7 +103,7 @@ struct dns_xfrin_ctx {
isc_refcount_t sends; /*%< Send in progress */
isc_refcount_t recvs; /*%< Receive in progress */
bool shuttingdown;
atomic_bool shuttingdown;
isc_result_t shutdown_result;
@@ -699,7 +699,7 @@ dns_xfrin_create(dns_zone_t *zone, dns_rdatatype_t xfrtype,
result = xfrin_start(xfr);
if (result != ISC_R_SUCCESS) {
xfr->shuttingdown = true;
atomic_store(&xfr->shuttingdown, true);
xfr->shutdown_result = result;
dns_xfrin_detach(xfrp);
}
@@ -718,12 +718,14 @@ dns_xfrin_create(dns_zone_t *zone, dns_rdatatype_t xfrtype,
return (result);
}
static void
xfrin_cancelio(dns_xfrin_ctx_t *xfr);
void
dns_xfrin_shutdown(dns_xfrin_ctx_t *xfr) {
REQUIRE(VALID_XFRIN(xfr));
if (!xfr->shuttingdown) {
xfrin_fail(xfr, ISC_R_CANCELED, "shut down");
}
xfrin_fail(xfr, ISC_R_CANCELED, "shut down");
}
void
@@ -790,27 +792,32 @@ xfrin_reset(dns_xfrin_ctx_t *xfr) {
static void
xfrin_fail(dns_xfrin_ctx_t *xfr, isc_result_t result, const char *msg) {
if (result != DNS_R_UPTODATE && result != DNS_R_TOOMANYRECORDS) {
xfrin_log(xfr, ISC_LOG_ERROR, "%s: %s", msg,
isc_result_totext(result));
if (xfr->is_ixfr) {
/* Pass special result code to force AXFR retry */
result = DNS_R_BADIXFR;
/* Make sure only the first xfrin_fail() trumps */
if (atomic_compare_exchange_strong(&xfr->shuttingdown, &(bool){ false },
true)) {
if (result != DNS_R_UPTODATE && result != DNS_R_TOOMANYRECORDS)
{
xfrin_log(xfr, ISC_LOG_ERROR, "%s: %s", msg,
isc_result_totext(result));
if (xfr->is_ixfr) {
/* Pass special result code to force AXFR retry
*/
result = DNS_R_BADIXFR;
}
}
xfrin_cancelio(xfr);
/*
* Close the journal.
*/
if (xfr->ixfr.journal != NULL) {
dns_journal_destroy(&xfr->ixfr.journal);
}
if (xfr->done != NULL) {
(xfr->done)(xfr->zone, result);
xfr->done = NULL;
}
xfr->shutdown_result = result;
}
xfrin_cancelio(xfr);
/*
* Close the journal.
*/
if (xfr->ixfr.journal != NULL) {
dns_journal_destroy(&xfr->ixfr.journal);
}
if (xfr->done != NULL) {
(xfr->done)(xfr->zone, result);
xfr->done = NULL;
}
xfr->shuttingdown = true;
xfr->shutdown_result = result;
}
static void
@@ -968,7 +975,7 @@ xfrin_connect_done(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
isc_tlsctx_free(&xfr->tlsctx);
}
if (xfr->shuttingdown) {
if (atomic_load(&xfr->shuttingdown)) {
result = ISC_R_SHUTTINGDOWN;
}
@@ -1004,7 +1011,7 @@ xfrin_connect_done(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
CHECK(xfrin_send_request(xfr));
failure:
if (result != ISC_R_SUCCESS && result != ISC_R_SHUTTINGDOWN) {
if (result != ISC_R_SUCCESS) {
xfrin_fail(xfr, result, "failed to connect");
}
@@ -1176,7 +1183,7 @@ xfrin_send_done(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
REQUIRE(VALID_XFRIN(xfr));
isc_refcount_decrement0(&xfr->sends);
if (xfr->shuttingdown) {
if (atomic_load(&xfr->shuttingdown)) {
result = ISC_R_SHUTTINGDOWN;
}
@@ -1190,7 +1197,7 @@ xfrin_send_done(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
isc_nm_read(recv_xfr->handle, xfrin_recv_done, recv_xfr);
failure:
if (result != ISC_R_SUCCESS && result != ISC_R_SHUTTINGDOWN) {
if (result != ISC_R_SUCCESS) {
xfrin_fail(xfr, result, "failed sending request data");
}
@@ -1212,7 +1219,7 @@ xfrin_recv_done(isc_nmhandle_t *handle, isc_result_t result,
isc_refcount_decrement0(&xfr->recvs);
if (xfr->shuttingdown) {
if (atomic_load(&xfr->shuttingdown)) {
result = ISC_R_SHUTTINGDOWN;
}
@@ -1465,7 +1472,7 @@ xfrin_recv_done(isc_nmhandle_t *handle, isc_result_t result,
xfr->done = NULL;
}
xfr->shuttingdown = true;
atomic_store(&xfr->shuttingdown, true);
xfr->shutdown_result = ISC_R_SUCCESS;
break;
default:
@@ -1481,7 +1488,7 @@ xfrin_recv_done(isc_nmhandle_t *handle, isc_result_t result,
}
failure:
if (result != ISC_R_SUCCESS && result != ISC_R_SHUTTINGDOWN) {
if (result != ISC_R_SUCCESS) {
xfrin_fail(xfr, result, "failed while receiving responses");
}
@@ -1501,7 +1508,7 @@ xfrin_destroy(dns_xfrin_ctx_t *xfr) {
REQUIRE(VALID_XFRIN(xfr));
/* Safe-guards */
REQUIRE(xfr->shuttingdown);
REQUIRE(atomic_load(&xfr->shuttingdown));
isc_refcount_destroy(&xfr->references);
isc_refcount_destroy(&xfr->connects);
isc_refcount_destroy(&xfr->recvs);