mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-31 14:35:26 +00:00
Code Issues Releases Wiki Activity

Properly handling dns_message_t shared references

Browse Source 
This commit fix the problems that arose when moving the dns_message_t
object from fetchctx_t to the query structure.

Since the lifetime of query objects are different than that of a fetchctx
and the dns_message_t object held by the query may be being used by some
external module, e.g. validator, even after the query may have been destroyed,
propery handling of the references to the message were added in this commit to
avoid accessing an already destroyed object.

Specifically, in rctx_done(), a reference to the message is attached at
the beginning of the function and detached at the end, since a possible call
to fctx_cancelquery() would release the dns_message_t object, and in the next
lines of code a call to rctx_nextserver() or rctx_chaseds() would require
a valid pointer to the same object.

In valcreate() a new reference is attached to the message object, this
ensures that if the corresponding query object is destroyed before the
validator attempts to access it, no invalid pointer access occurs.

In validated() we have to attach a new reference to the message, since
we destroy the validator object at the beginning of the function,
and we need access to the message in the next lines of the same function.

rctx_nextserver() and rctx_chaseds() functions were adapted to receive
a new parameter of dns_message_t* type, this was so they could receive a
valid reference to a dns_message_t since using the response context respctx_t
to access the message through rctx->query->rmessage could lead to an already
released reference due to the query being canceled.
This commit is contained in:
Diego Fronza
2020-09-21 17:44:29 -03:00
committed by Ondřej Surý
parent 02f9e125c1
commit cde6227a68
1 changed files with 30 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

46
lib/dns/resolver.c
View File

@@ -819,8 +819,8 @@ static isc_result_t
rctx_answer_none(respctx_t *rctx);
static void
rctx_nextserver(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo,
isc_result_t result);
rctx_nextserver(respctx_t *rctx, dns_message_t *message,
dns_adbaddrinfo_t *addrinfo, isc_result_t result);
static void
rctx_resend(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo);
@@ -829,7 +829,8 @@ static void
rctx_next(respctx_t *rctx);
static void
rctx_chaseds(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo, isc_result_t result);
rctx_chaseds(respctx_t *rctx, dns_message_t *message,
dns_adbaddrinfo_t *addrinfo, isc_result_t result);
static void
rctx_done(respctx_t *rctx, isc_result_t result);
@@ -894,7 +895,8 @@ valcreate(fetchctx_t *fctx, dns_message_t *message, dns_adbaddrinfo_t *addrinfo,
valarg->fctx = fctx;
valarg->addrinfo = addrinfo;
valarg->message = message;
valarg->message = NULL;
dns_message_attach(message, &valarg->message);
if (!ISC_LIST_EMPTY(fctx->validators)) {
valoptions |= DNS_VALIDATOR_DEFER;
@@ -913,6 +915,7 @@ valcreate(fetchctx_t *fctx, dns_message_t *message, dns_adbaddrinfo_t *addrinfo,
}
ISC_LIST_APPEND(fctx->validators, validator, link);
} else {
dns_message_detach(&valarg->message);
isc_mem_put(fctx->mctx, valarg, sizeof(*valarg));
}
return (result);
@@ -5538,14 +5541,14 @@ validated(isc_task_t *task, isc_event_t *event) {
uint32_t bucketnum;
dns_fixedname_t fwild;
dns_name_t *wild = NULL;
dns_message_t *message;
dns_message_t *message = NULL;
UNUSED(task); /* for now */
REQUIRE(event->ev_type == DNS_EVENT_VALIDATORDONE);
valarg = event->ev_arg;
message = valarg->message;
fctx = valarg->fctx;
dns_message_attach(valarg->message, &message);
REQUIRE(VALID_FCTX(fctx));
res = fctx->res;
@@ -5574,6 +5577,7 @@ validated(isc_task_t *task, isc_event_t *event) {
wild);
}
dns_validator_destroy(&vevent->validator);
dns_message_detach(&valarg->message);
isc_mem_put(fctx->mctx, valarg, sizeof(*valarg));
negative = (vevent->rdataset == NULL);
@@ -5719,6 +5723,7 @@ validated(isc_task_t *task, isc_event_t *event) {
fctx_try(fctx, true, true); /* Locks bucket. */
}
dns_message_detach(&message);
return;
}
@@ -9510,8 +9515,8 @@ again:
* useful to try another one.
*/
static void
rctx_nextserver(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo,
isc_result_t result) {
rctx_nextserver(respctx_t *rctx, dns_message_t *message,
dns_adbaddrinfo_t *addrinfo, isc_result_t result) {
fetchctx_t *fctx = rctx->fctx;
if (result == DNS_R_FORMERR) {
@@ -9522,8 +9527,8 @@ rctx_nextserver(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo,
* Add this server to the list of bad servers for
* this fctx.
*/
add_bad(fctx, rctx->query->rmessage, addrinfo,
rctx->broken_server, rctx->broken_type);
add_bad(fctx, message, addrinfo, rctx->broken_server,
rctx->broken_type);
}
if (rctx->get_nameservers) {
@@ -9650,13 +9655,12 @@ rctx_next(respctx_t *rctx) {
* Look up the parent zone's NS records so that DS records can be fetched.
*/
static void
rctx_chaseds(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo,
isc_result_t result) {
rctx_chaseds(respctx_t *rctx, dns_message_t *message,
dns_adbaddrinfo_t *addrinfo, isc_result_t result) {
fetchctx_t *fctx = rctx->fctx;
unsigned int n;
add_bad(fctx, rctx->query->rmessage, addrinfo, result,
rctx->broken_type);
add_bad(fctx, message, addrinfo, result, rctx->broken_type);
fctx_cancelqueries(fctx, true, false);
fctx_cleanupfinds(fctx);
fctx_cleanupforwaddrs(fctx);
@@ -9696,6 +9700,14 @@ rctx_done(respctx_t *rctx, isc_result_t result) {
resquery_t *query = rctx->query;
fetchctx_t *fctx = rctx->fctx;
dns_adbaddrinfo_t *addrinfo = query->addrinfo;
/*
* Need to attach to the message until the scope
* of this function ends, since there are many places
* where te message is used and/or may be destroyed
* before this function ends.
*/
dns_message_t *message = NULL;
dns_message_attach(query->rmessage, &message);
FCTXTRACE4("query canceled in response(); ",
rctx->no_response ? "no response" : "responding", result);
@@ -9724,13 +9736,13 @@ rctx_done(respctx_t *rctx, isc_result_t result) {
#endif /* ifdef ENABLE_AFL */
if (rctx->next_server)
{
rctx_nextserver(rctx, addrinfo, result);
rctx_nextserver(rctx, message, addrinfo, result);
} else if (rctx->resend) {
rctx_resend(rctx, addrinfo);
} else if (rctx->nextitem) {
rctx_next(rctx);
} else if (result == DNS_R_CHASEDSSERVERS) {
rctx_chaseds(rctx, addrinfo, result);
rctx_chaseds(rctx, message, addrinfo, result);
} else if (result == ISC_R_SUCCESS && !HAVE_ANSWER(fctx)) {
/*
* All has gone well so far, but we are waiting for the
@@ -9752,6 +9764,8 @@ rctx_done(respctx_t *rctx, isc_result_t result) {
*/
fctx_done(fctx, result, __LINE__);
}
dns_message_detach(&message);
}
/*