Gitweb:
https://git.exim.org/exim.git/commitdiff/92583637b25b6bde926f9ca6be7b085e5ac8b1e6
Commit: 92583637b25b6bde926f9ca6be7b085e5ac8b1e6
Parent: f3f9fe5c6f21a00accc994e0b79480d247f9d6db
Author: Jeremy Harris <jgh146exb@???>
AuthorDate: Sun Mar 21 00:02:07 2021 +0000
Committer: Jeremy Harris <jgh146exb@???>
CommitDate: Sun Mar 21 00:02:07 2021 +0000
DNS: explicit alloc/free of workspace
---
doc/doc-txt/ChangeLog | 4 +++
src/src/acl.c | 70 ++++++++++++++++++++++++++++++++++++-------------
src/src/dkim.c | 7 ++++-
src/src/dmarc.c | 6 ++++-
src/src/dnsbl.c | 34 +++++++++++++++++-------
src/src/functions.h | 16 ++++++++++-
src/src/host.c | 36 +++++++++++++++++--------
src/src/lookups/dnsdb.c | 35 ++++++++++++++++++-------
src/src/spf.c | 3 +++
src/src/string.c | 2 +-
10 files changed, 161 insertions(+), 52 deletions(-)
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index bbd305a..89c4542 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -223,6 +223,10 @@ JH/46 Use an exponentially-increasing block size when malloc'ing store. Do it
was used which resulted in O(n^2) behaviour; now we get O(n log n) making
DOS attacks harder. The cost is wasted memory use in the larger blocks.
+JH/47 Use explicit alloc/free for DNS lookup workspace. This permits using the
+ same space repeatedly, and a smaller process footprint.
+
+
Exim version 4.94
diff --git a/src/src/acl.c b/src/src/acl.c
index fff2ac0..2f20821 100644
--- a/src/src/acl.c
+++ b/src/src/acl.c
@@ -1313,11 +1313,12 @@ acl_verify_csa(const uschar *domain)
tree_node *t;
const uschar *found;
int priority, weight, port;
-dns_answer * dnsa = store_get_dns_answer();
+dns_answer * dnsa;
dns_scan dnss;
dns_record *rr;
-int rc, type;
-uschar target[256];
+int rc, type, yield;
+#define TARGET_SIZE 256
+uschar * target = store_get(TARGET_SIZE, TRUE);
/* Work out the domain we are using for the CSA lookup. The default is the
client's HELO domain. If the client has not said HELO, use its IP address
@@ -1325,8 +1326,8 @@ instead. If it's a local client (exim -bs), CSA isn't applicable. */
while (isspace(*domain) && *domain != '\0') ++domain;
if (*domain == '\0') domain = sender_helo_name;
-if (domain == NULL) domain = sender_host_address;
-if (sender_host_address == NULL) return CSA_UNKNOWN;
+if (!domain) domain = sender_host_address;
+if (!sender_host_address) return CSA_UNKNOWN;
/* If we have an address literal, strip off the framing ready for turning it
into a domain. The framing consists of matched square brackets possibly
@@ -1356,8 +1357,8 @@ return the same result again. Otherwise build a new cached result structure
for this domain. The name is filled in now, and the value is filled in when
we return from this function. */
-t = tree_search(csa_cache, domain);
-if (t != NULL) return t->data.val;
+if ((t = tree_search(csa_cache, domain)))
+ return t->data.val;
t = store_get_perm(sizeof(tree_node) + Ustrlen(domain), is_tainted(domain));
Ustrcpy(t->name, domain);
@@ -1366,18 +1367,21 @@ Ustrcpy(t->name, domain);
/* Now we are ready to do the actual DNS lookup(s). */
found = domain;
+dnsa = store_get_dns_answer();
switch (dns_special_lookup(dnsa, domain, T_CSA, &found))
{
/* If something bad happened (most commonly DNS_AGAIN), defer. */
default:
- return t->data.val = CSA_DEFER_SRV;
+ yield = CSA_DEFER_SRV;
+ goto out;
/* If we found nothing, the client's authorization is unknown. */
case DNS_NOMATCH:
case DNS_NODATA:
- return t->data.val = CSA_UNKNOWN;
+ yield = CSA_UNKNOWN;
+ goto out;
/* We got something! Go on to look at the reply in more detail. */
@@ -1413,7 +1417,10 @@ for (rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS);
SRV records of their own. */
if (Ustrcmp(found, domain) != 0)
- return t->data.val = port & 1 ? CSA_FAIL_EXPLICIT : CSA_UNKNOWN;
+ {
+ yield = port & 1 ? CSA_FAIL_EXPLICIT : CSA_UNKNOWN;
+ goto out;
+ }
/* This CSA SRV record refers directly to our domain, so we check the value
in the weight field to work out the domain's authorization. 0 and 1 are
@@ -1421,7 +1428,11 @@ for (rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS);
address in order to authenticate it, so we treat it as unknown; values
greater than 3 are undefined. */
- if (weight < 2) return t->data.val = CSA_FAIL_DOMAIN;
+ if (weight < 2)
+ {
+ yield = CSA_FAIL_DOMAIN;
+ goto out;
+ }
if (weight > 2) continue;
@@ -1430,7 +1441,7 @@ for (rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS);
target hostname then break to scan the additional data for its addresses. */
(void)dn_expand(dnsa->answer, dnsa->answer + dnsa->answerlen, p,
- (DN_EXPAND_ARG4_TYPE)target, sizeof(target));
+ (DN_EXPAND_ARG4_TYPE)target, TARGET_SIZE);
DEBUG(D_acl) debug_printf_indent("CSA target is %s\n", target);
@@ -1439,7 +1450,11 @@ for (rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS);
/* If we didn't break the loop then no appropriate records were found. */
-if (!rr) return t->data.val = CSA_UNKNOWN;
+if (!rr)
+ {
+ yield = CSA_UNKNOWN;
+ goto out;
+ }
/* Do not check addresses if the target is ".", in accordance with RFC 2782.
A target of "." indicates there are no valid addresses, so the client cannot
@@ -1447,7 +1462,11 @@ be authorized. (This is an odd configuration because weight=2 target=. is
equivalent to weight=1, but we check for it in order to keep load off the
root name servers.) Note that dn_expand() turns "." into "". */
-if (Ustrcmp(target, "") == 0) return t->data.val = CSA_FAIL_NOADDR;
+if (Ustrcmp(target, "") == 0)
+ {
+ yield = CSA_FAIL_NOADDR;
+ goto out;
+ }
/* Scan the additional section of the CSA SRV reply for addresses belonging
to the target. If the name server didn't return any additional data (e.g.
@@ -1455,7 +1474,11 @@ because it does not fully support SRV records), we need to do another lookup
to obtain the target addresses; otherwise we have a definitive result. */
rc = acl_verify_csa_address(dnsa, &dnss, RESET_ADDITIONAL, target);
-if (rc != CSA_FAIL_NOADDR) return t->data.val = rc;
+if (rc != CSA_FAIL_NOADDR)
+ {
+ yield = rc;
+ goto out;
+ }
/* The DNS lookup type corresponds to the IP version used by the client. */
@@ -1473,13 +1496,18 @@ switch (dns_lookup(dnsa, target, type, NULL))
/* If something bad happened (most commonly DNS_AGAIN), defer. */
default:
- return t->data.val = CSA_DEFER_ADDR;
+ yield = CSA_DEFER_ADDR;
+ break;
/* If the query succeeded, scan the addresses and return the result. */
case DNS_SUCCEED:
rc = acl_verify_csa_address(dnsa, &dnss, RESET_ANSWERS, target);
- if (rc != CSA_FAIL_NOADDR) return t->data.val = rc;
+ if (rc != CSA_FAIL_NOADDR)
+ {
+ yield = rc;
+ break;
+ }
/* else fall through */
/* If the target has no IP addresses, the client cannot have an authorized
@@ -1488,8 +1516,14 @@ switch (dns_lookup(dnsa, target, type, NULL))
case DNS_NOMATCH:
case DNS_NODATA:
- return t->data.val = CSA_FAIL_NOADDR;
+ yield = CSA_FAIL_NOADDR;
+ break;
}
+
+out:
+
+store_free_dns_answer(dnsa);
+return t->data.val = yield;
}
diff --git a/src/src/dkim.c b/src/src/dkim.c
index a48f1a1..87c9c91 100644
--- a/src/src/dkim.c
+++ b/src/src/dkim.c
@@ -50,11 +50,14 @@ dkim_exim_query_dns_txt(const uschar * name)
dns_answer * dnsa = store_get_dns_answer();
dns_scan dnss;
rmark reset_point = store_mark();
-gstring * g = NULL;
+gstring * g = string_get_tainted(256, TRUE);
lookup_dnssec_authenticated = NULL;
if (dns_lookup(dnsa, name, T_TXT, NULL) != DNS_SUCCEED)
+ {
+ store_free_dns_answer(dnsa);
return NULL; /*XXX better error detail? logging? */
+ }
/* Search for TXT record */
@@ -81,6 +84,7 @@ for (dns_record * rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS);
/* check if this looks like a DKIM record */
if (Ustrncmp(g->s, "v=", 2) != 0 || strncasecmp(CS g->s, "v=dkim", 6) == 0)
{
+ store_free_dns_answer(dnsa);
gstring_release_unused(g);
return string_from_gstring(g);
}
@@ -90,6 +94,7 @@ for (dns_record * rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS);
bad:
store_reset(reset_point);
+store_free_dns_answer(dnsa);
return NULL; /*XXX better error detail? logging? */
}
diff --git a/src/src/dmarc.c b/src/src/dmarc.c
index 333aad9..5328f4f 100644
--- a/src/src/dmarc.c
+++ b/src/src/dmarc.c
@@ -218,7 +218,11 @@ if (rc == DNS_SUCCEED)
for (dns_record * rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS); rr;
rr = dns_next_rr(dnsa, &dnss, RESET_NEXT))
if (rr->type == T_TXT && rr->size > 3)
- return string_copyn(US rr->data, rr->size);
+ {
+ store_free_dns_answer(dnsa);
+ return string_copyn_taint(US rr->data, rr->size, TRUE);
+ }
+store_free_dns_answer(dnsa);
return NULL;
}
diff --git a/src/src/dnsbl.c b/src/src/dnsbl.c
index 5c6a76d..ad36e8e 100644
--- a/src/src/dnsbl.c
+++ b/src/src/dnsbl.c
@@ -74,7 +74,7 @@ tree_node *t;
dnsbl_cache_block *cb;
int old_pool = store_pool;
uschar * query;
-int qlen;
+int qlen, yield;
/* Construct the specific query domainname */
@@ -83,7 +83,8 @@ if ((qlen = Ustrlen(query)) >= 256)
{
log_write(0, LOG_MAIN|LOG_PANIC, "dnslist query is too long "
"(ignored): %s...", query);
- return FAIL;
+ yield = FAIL;
+ goto out;
}
/* Look for this query in the cache. */
@@ -305,7 +306,8 @@ if (cb->rc == DNS_SUCCEED)
match_type & MT_ALL ? "=" : "",
bitmask ? '&' : '=', iplist);
}
- return FAIL;
+ yield = FAIL;
+ goto out;
}
}
@@ -329,7 +331,11 @@ if (cb->rc == DNS_SUCCEED)
" not in 127.0/8 and discarded",
keydomain, domain, da->address);
}
- if (!ok) return FAIL;
+ if (!ok)
+ {
+ yield = FAIL;
+ goto out;
+ }
}
/* Either there was no IP list, or the record matched, implying that the
@@ -339,8 +345,11 @@ if (cb->rc == DNS_SUCCEED)
there is indeed an A record at the alternate domain. */
if (domain_txt != domain)
- return one_check_dnsbl(domain_txt, domain_txt, keydomain, prepend, NULL,
+ {
+ yield = one_check_dnsbl(domain_txt, domain_txt, keydomain, prepend, NULL,
FALSE, match_type, defer_return);
+ goto out;
+ }
/* If there is no alternate domain, look up a TXT record in the main domain
if it has not previously been cached. */
@@ -356,7 +365,7 @@ if (cb->rc == DNS_SUCCEED)
int len = (rr->data)[0];
if (len > 511) len = 127;
store_pool = POOL_PERM;
- cb->text = string_sprintf("%.*s", len, CUS (rr->data+1));
+ cb->text = string_copyn_taint(CUS (rr->data+1), len, TRUE);
store_pool = old_pool;
break;
}
@@ -364,7 +373,8 @@ if (cb->rc == DNS_SUCCEED)
dnslist_value = addlist;
dnslist_text = cb->text;
- return OK;
+ yield = OK;
+ goto out;
}
/* There was a problem with the DNS lookup */
@@ -376,7 +386,8 @@ if (cb->rc != DNS_NOMATCH && cb->rc != DNS_NODATA)
defer_return == OK ? US"assumed in list" :
defer_return == FAIL ? US"assumed not in list" :
US"returned DEFER");
- return defer_return;
+ yield = defer_return;
+ goto out;
}
/* No entry was found in the DNS; continue for next domain */
@@ -388,7 +399,12 @@ HDEBUG(D_dnsbl)
keydomain, domain);
}
-return FAIL;
+yield = FAIL;
+
+out:
+
+store_free_dns_answer(dnsa);
+return yield;
}
diff --git a/src/src/functions.h b/src/src/functions.h
index 6d16e69..83fad74 100644
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -967,13 +967,27 @@ g->s = s;
/******************************************************************************/
+/* Use store_malloc for DNSA structs, and explicit frees. Using the same pool
+for them as the strings we proceed to copy from them meant they could not be
+released, hence blowing 64k for every DNS lookup. That mounted up. With malloc
+we do have to take care over marking tainted all copied strings. A separate pool
+could be used and would handle that implicitly. */
#define store_get_dns_answer() store_get_dns_answer_trc(CUS __FUNCTION__, __LINE__)
static inline dns_answer *
store_get_dns_answer_trc(const uschar * func, unsigned line)
{
-return store_get_3(sizeof(dns_answer), TRUE, CCS func, line); /* use tainted mem */
+/* return store_get_3(sizeof(dns_answer), TRUE, CCS func, line); use tainted mem */
+return store_malloc_3(sizeof(dns_answer), CCS func, line);
+}
+
+#define store_free_dns_answer(dnsa) store_free_dns_answer_trc(dnsa, CUS __FUNCTION__, __LINE__)
+
+static inline void
+store_free_dns_answer_trc(dns_answer * dnsa, const uschar * func, unsigned line)
+{
+store_free_3(dnsa, CCS func, line);
}
/******************************************************************************/
diff --git a/src/src/host.c b/src/src/host.c
index c25ff2a..18f1b54 100644
--- a/src/src/host.c
+++ b/src/src/host.c
@@ -222,7 +222,8 @@ if ((ipa = string_is_ip_address(lname, NULL)) != 0)
else
{
*error_num = HOST_NOT_FOUND;
- return NULL;
+ yield = NULL;
+ goto out;
}
/* Handle a host name */
@@ -238,11 +239,11 @@ else
switch(rc)
{
case DNS_SUCCEED: break;
- case DNS_NOMATCH: *error_num = HOST_NOT_FOUND; return NULL;
- case DNS_NODATA: *error_num = NO_DATA; return NULL;
- case DNS_AGAIN: *error_num = TRY_AGAIN; return NULL;
+ case DNS_NOMATCH: *error_num = HOST_NOT_FOUND; yield = NULL; goto out;
+ case DNS_NODATA: *error_num = NO_DATA; yield = NULL; goto out;
+ case DNS_AGAIN: *error_num = TRY_AGAIN; yield = NULL; goto out;
default:
- case DNS_FAIL: *error_num = NO_RECOVERY; return NULL;
+ case DNS_FAIL: *error_num = NO_RECOVERY; yield = NULL; goto out;
}
for (dns_record * rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS);
@@ -280,6 +281,9 @@ else
*alist = NULL;
}
+out:
+
+store_free_dns_answer(dnsa);
return yield;
}
@@ -2262,6 +2266,7 @@ host_item *thishostlast = NULL; /* Indicates not yet filled in anything */
BOOL v6_find_again = FALSE;
BOOL dnssec_fail = FALSE;
int i;
+dns_answer * dnsa;
#ifndef DISABLE_TLS
/* Copy the host name at this point to the value which is used for
@@ -2287,6 +2292,8 @@ if (allow_ip && string_is_ip_address(host->name, NULL) != 0)
return HOST_FOUND;
}
+dnsa = store_get_dns_answer();
+
/* On an IPv6 system, unless IPv6 is disabled, go round the loop up to twice,
looking for AAAA records the first time. However, unless doing standalone
testing, we force an IPv4 lookup if the domain matches dns_ipv4_lookup global.
@@ -2318,7 +2325,6 @@ for (; i >= 0; i--)
int type = types[i];
int randoffset = i == (whichrrs & HOST_FIND_IPV4_FIRST ? 1 : 0)
? 500 : 0; /* Ensures v6/4 sort order */
- dns_answer * dnsa = store_get_dns_answer();
dns_scan dnss;
int rc = dns_lookup_timerwrap(dnsa, host->name, type, fully_qualified_name);
@@ -2341,10 +2347,13 @@ for (; i >= 0; i--)
{
if (i == 0) /* Just tried for an A record, i.e. end of loop */
{
- if (host->address != NULL) return HOST_FOUND; /* AAAA was found */
- if (rc == DNS_AGAIN || rc == DNS_FAIL || v6_find_again)
- return HOST_FIND_AGAIN;
- return HOST_FIND_FAILED; /* DNS_NOMATCH or DNS_NODATA */
+ if (host->address != NULL)
+ i = HOST_FOUND; /* AAAA was found */
+ else if (rc == DNS_AGAIN || rc == DNS_FAIL || v6_find_again)
+ i = HOST_FIND_AGAIN;
+ else
+ i = HOST_FIND_FAILED; /* DNS_NOMATCH or DNS_NODATA */
+ goto out;
}
/* Tried for an AAAA record: remember if this was a temporary
@@ -2489,11 +2498,15 @@ for (; i >= 0; i--)
/* Control gets here only if the second lookup (the A record) succeeded.
However, the address may not be filled in if it was ignored. */
-return host->address
+i = host->address
? HOST_FOUND
: dnssec_fail
? HOST_FIND_SECURITY
: HOST_IGNORED;
+
+out:
+ store_free_dns_answer(dnsa);
+ return i;
}
@@ -3162,6 +3175,7 @@ DEBUG(D_host_lookup)
out:
dns_init(FALSE, FALSE, FALSE); /* clear the dnssec bit for getaddrbyname */
+store_free_dns_answer(dnsa);
return yield;
}
diff --git a/src/src/lookups/dnsdb.c b/src/src/lookups/dnsdb.c
index d064821..8422833 100644
--- a/src/src/lookups/dnsdb.c
+++ b/src/src/lookups/dnsdb.c
@@ -190,7 +190,8 @@ for (;;)
else
{
*errmsg = US"unsupported dnsdb defer behaviour";
- return DEFER;
+ rc = DEFER;
+ goto out;
}
}
else if (strncmpic(keystring, US"dnssec_", 7) == 0)
@@ -205,7 +206,8 @@ for (;;)
else
{
*errmsg = US"unsupported dnsdb dnssec behaviour";
- return DEFER;
+ rc = DEFER;
+ goto out;
}
}
else if (strncmpic(keystring, US"retrans_", 8) == 0)
@@ -214,7 +216,8 @@ for (;;)
if ((timeout_sec = readconf_readtime(keystring += 8, ',', FALSE)) <= 0)
{
*errmsg = US"unsupported dnsdb timeout value";
- return DEFER;
+ rc = DEFER;
+ goto out;
}
dns_retrans = timeout_sec;
while (*keystring != ',') keystring++;
@@ -225,7 +228,8 @@ for (;;)
if ((retries = (int)strtol(CCS keystring + 6, CSS &keystring, 0)) < 0)
{
*errmsg = US"unsupported dnsdb retry count";
- return DEFER;
+ rc = DEFER;
+ goto out;
}
dns_retry = retries;
}
@@ -236,7 +240,8 @@ for (;;)
if (*keystring++ != ',')
{
*errmsg = US"dnsdb modifier syntax error";
- return DEFER;
+ rc = DEFER;
+ goto out;
}
while (isspace(*keystring)) keystring++;
}
@@ -264,7 +269,8 @@ if ((equals = Ustrchr(keystring, '=')) != NULL)
if (i >= nelem(type_names))
{
*errmsg = US"unsupported DNS record type";
- return DEFER;
+ rc = DEFER;
+ goto out;
}
keystring = equals + 1;
@@ -359,7 +365,8 @@ while ((domain = string_nextinlist(&keystring, &sep, NULL, 0)))
dns_retrans = save_retrans;
dns_retry = save_retry;
dns_init(FALSE, FALSE, FALSE); /* clr dnssec bit */
- return DEFER; /* always defer */
+ rc = DEFER; /* always defer */
+ goto out;
}
if (defer_mode == PASS) failrc = DEFER; /* defer only if all do */
continue; /* treat defer as fail */
@@ -549,10 +556,18 @@ dns_retrans = save_retrans;
dns_retry = save_retry;
dns_init(FALSE, FALSE, FALSE); /* clear the dnssec bit for getaddrbyname */
-if (!yield || !yield->ptr) return failrc;
+if (!yield || !yield->ptr)
+ rc = failrc;
+else
+ {
+ *result = string_from_gstring(yield);
+ rc = OK;
+ }
+
+out:
-*result = string_from_gstring(yield);
-return OK;
+store_free_dns_answer(dnsa);
+return rc;
}
diff --git a/src/src/spf.c b/src/src/spf.c
index 3a1912a..2f55fb7 100644
--- a/src/src/spf.c
+++ b/src/src/spf.c
@@ -80,6 +80,7 @@ if (rr_type == T_SPF)
HDEBUG(D_host_lookup) debug_printf("faking NO_DATA for SPF RR(99) lookup\n");
srr.herrno = NO_DATA;
SPF_dns_rr_dup(&spfrr, &srr);
+ store_free_dns_answer(dnsa);
return spfrr;
}
@@ -100,6 +101,7 @@ for (dns_record * rr = dns_next_rr(dnsa, &dnss, RESET_ANSWERS); rr;
if (found == 0)
{
SPF_dns_rr_dup(&spfrr, &srr);
+ store_free_dns_answer(dnsa);
return spfrr;
}
@@ -171,6 +173,7 @@ if (!(srr.num_rr = found))
/* spfrr->rr must have been malloc()d for this */
SPF_dns_rr_dup(&spfrr, &srr);
+store_free_dns_answer(dnsa);
return spfrr;
}
diff --git a/src/src/string.c b/src/src/string.c
index 7af87f9..afdb517 100644
--- a/src/src/string.c
+++ b/src/src/string.c
@@ -575,7 +575,7 @@ uschar *
string_copy_dnsdomain(uschar *s)
{
uschar *yield;
-uschar *ss = yield = store_get(Ustrlen(s) + 1, is_tainted(s));
+uschar *ss = yield = store_get(Ustrlen(s) + 1, TRUE); /* always treat as tainted */
while (*s != 0)
{
