Gitweb:
https://git.exim.org/exim.git/commitdiff/ee3c2fea18d0c940c2256c6bf041f546c703c375
Commit: ee3c2fea18d0c940c2256c6bf041f546c703c375
Parent: 4ac2988b554c6817acd0233c7a5c8a72698b8474
Author: Jeremy Harris <jgh146exb@???>
AuthorDate: Sun Mar 14 17:25:11 2021 +0000
Committer: Jeremy Harris <jgh146exb@???>
CommitDate: Sat Mar 20 00:17:21 2021 +0000
DKIM: verify using separate pool-pair, reset per message
---
doc/doc-txt/ChangeLog | 5 ++++
src/exim_monitor/em_main.c | 4 +++-
src/src/dkim.c | 15 +++++++-----
src/src/exim.c | 2 ++
src/src/exim_dbmbuild.c | 2 +-
src/src/exim_dbutil.c | 5 ++++
src/src/expand.c | 1 +
src/src/functions.h | 5 ++++
src/src/host.c | 1 +
src/src/parse.c | 1 +
src/src/smtp_in.c | 7 +++++-
src/src/store.c | 59 +++++++++++++++++++++++++++++++++++++++-------
src/src/store.h | 19 +++++++++++----
src/src/string.c | 1 +
14 files changed, 105 insertions(+), 22 deletions(-)
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 6993499..c1f6f9b 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -213,6 +213,11 @@ JH/44 Bug 2701: Fix list-expansion of dns_ipv4_lookup. Previously, it did
dnssec_require_domains, dnssec_request_domains, srv_fail_domains,
mx_fail_domains.
+JH/45 Use a (new) separate store pool-pair for DKIM verify working data.
+ Previously the permanent pool was used, so the sore could not be freed.
+ This meant a connection with many messages would use continually-growing
+ memory.
+
Exim version 4.94
-----------------
diff --git a/src/exim_monitor/em_main.c b/src/exim_monitor/em_main.c
index 88bf1fc..0a89fbb 100644
--- a/src/exim_monitor/em_main.c
+++ b/src/exim_monitor/em_main.c
@@ -570,7 +570,8 @@ return ret;
* Initialize *
*************************************************/
-int main(int argc, char **argv)
+int
+main(int argc, char **argv)
{
int i;
struct stat statdata;
@@ -591,6 +592,7 @@ message_subdir[1] = 0;
constructing file names and things. This call will initialize
the store_get() function. */
+store_init();
big_buffer = store_get(big_buffer_size, FALSE);
/* Set up the version string and date and output them */
diff --git a/src/src/dkim.c b/src/src/dkim.c
index 92adb35..a48f1a1 100644
--- a/src/src/dkim.c
+++ b/src/src/dkim.c
@@ -109,12 +109,15 @@ dkim_exim_verify_init(BOOL dot_stuffing)
{
dkim_exim_init();
-/* There is a store-reset between header & body reception
-so cannot use the main pool. Any allocs done by Exim
-memory-handling must use the perm pool. */
+/* There is a store-reset between header & body reception for the main pool
+(actually, after every header line) so cannot use that as we need the data we
+store per-header, during header processing, at the end of body reception
+for evaluating the signature. Any allocs done for dkim verify
+memory-handling must use a different pool. We use a separate one that we
+can reset per message. */
dkim_verify_oldpool = store_pool;
-store_pool = POOL_PERM;
+store_pool = POOL_MESSAGE;
/* Free previous context if there is one */
@@ -139,7 +142,7 @@ dkim_exim_verify_feed(uschar * data, int len)
{
int rc;
-store_pool = POOL_PERM;
+store_pool = POOL_MESSAGE;
if ( dkim_collect_input
&& (rc = pdkim_feed(dkim_verify_ctx, data, len)) != PDKIM_OK)
{
@@ -305,7 +308,7 @@ int rc;
gstring * g = NULL;
const uschar * errstr = NULL;
-store_pool = POOL_PERM;
+store_pool = POOL_MESSAGE;
/* Delete eventual previous signature chain */
diff --git a/src/src/exim.c b/src/src/exim.c
index 8f33bde..cb11a2a 100644
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -1669,6 +1669,8 @@ extern char **environ;
(void)gettimeofday(×tamp_startup, NULL);
#endif
+store_init(); /* Initialise the memory allocation susbsystem */
+
/* If the Exim user and/or group and/or the configuration file owner/group were
defined by ref:name at build time, we must now find the actual uid/gid values.
This is a feature to make the lives of binary distributors easier. */
diff --git a/src/src/exim_dbmbuild.c b/src/src/exim_dbmbuild.c
index 7cf2e47..da8d570 100644
--- a/src/src/exim_dbmbuild.c
+++ b/src/src/exim_dbmbuild.c
@@ -45,7 +45,7 @@ void *
store_get_3(int size, BOOL tainted, const char *filename, int linenumber)
{ return NULL; }
void **
-store_reset_3(void **ptr, int pool, const char *filename, int linenumber)
+store_reset_3(void **ptr, const char *filename, int linenumber)
{ return NULL; }
void
store_release_above_3(void *ptr, const char *func, int linenumber)
diff --git a/src/src/exim_dbutil.c b/src/src/exim_dbutil.c
index 7ca7a30..2ae7ef4 100644
--- a/src/src/exim_dbutil.c
+++ b/src/src/exim_dbutil.c
@@ -551,6 +551,8 @@ EXIM_CURSOR *cursor;
uschar **argv = USS cargv;
uschar keybuffer[1024];
+store_init();
+
/* Check the arguments, and open the database */
dbdata_type = check_args(argc, argv, US"dumpdb", US"");
@@ -765,6 +767,7 @@ uschar buffer[256];
uschar name[256];
rmark reset_point;
+store_init();
name[0] = 0; /* No name set */
/* Sort out the database type, verify what we are working on and then process
@@ -1134,6 +1137,8 @@ uschar **argv = USS cargv;
uschar buffer[256];
uschar *key;
+store_init();
+
/* Scan the options */
for (i = 1; i < argc; i++)
diff --git a/src/src/expand.c b/src/src/expand.c
index 8be7bf9..8a571b2 100644
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -8641,6 +8641,7 @@ debug_selector = D_v;
debug_file = stderr;
debug_fd = fileno(debug_file);
big_buffer = malloc(big_buffer_size);
+store_init();
for (int i = 1; i < argc; i++)
{
diff --git a/src/src/functions.h b/src/src/functions.h
index 5ffb23d..6d16e69 100644
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -340,6 +340,9 @@ extern int match_isinlist(const uschar *, const uschar **, int, tree_node **
unsigned int *, int, BOOL, const uschar **);
extern int match_check_string(const uschar *, const uschar *, int, BOOL, BOOL, BOOL,
const uschar **);
+
+extern void message_start(void);
+extern void message_tidyup(void);
extern void md5_end(md5 *, const uschar *, int, uschar *);
extern void md5_mid(md5 *, const uschar *);
extern void md5_start(md5 *);
@@ -522,6 +525,8 @@ extern int stdin_ferror(void);
extern int stdin_ungetc(int);
extern void store_exit(void);
+extern void store_init(void);
+
extern gstring *string_append(gstring *, int, ...) WARN_UNUSED_RESULT;
extern gstring *string_append_listele(gstring *, uschar, const uschar *) WARN_UNUSED_RESULT;
extern gstring *string_append_listele_n(gstring *, uschar, const uschar *, unsigned) WARN_UNUSED_RESULT;
diff --git a/src/src/host.c b/src/src/host.c
index 6b9f674..c25ff2a 100644
--- a/src/src/host.c
+++ b/src/src/host.c
@@ -3260,6 +3260,7 @@ uschar buffer[256];
disable_ipv6 = FALSE;
primary_hostname = US"";
+store_init();
store_pool = POOL_MAIN;
debug_selector = D_host_lookup|D_interface;
debug_file = stdout;
diff --git a/src/src/parse.c b/src/src/parse.c
index 2b2ffd3..885a01c 100644
--- a/src/src/parse.c
+++ b/src/src/parse.c
@@ -2083,6 +2083,7 @@ int main(void)
int start, end, domain;
uschar buffer[1024];
+store_init();
big_buffer = store_malloc(big_buffer_size);
/* strip_trailing_dot = TRUE; */
diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c
index 03085c9..6d6370f 100644
--- a/src/src/smtp_in.c
+++ b/src/src/smtp_in.c
@@ -2109,7 +2109,11 @@ while (acl_warn_logged)
acl_warn_logged = acl_warn_logged->next;
store_free(this);
}
+
+message_tidyup();
store_reset(reset_point);
+
+message_start();
return store_mark();
}
@@ -2186,7 +2190,7 @@ while (done <= 0)
case MAIL_CMD:
smtp_mailcmd_count++; /* Count for no-mail log */
- if (sender_address != NULL)
+ if (sender_address)
/* The function moan_smtp_batch() does not return. */
moan_smtp_batch(smtp_cmd_buffer, "503 Sender already given");
@@ -4533,6 +4537,7 @@ while (done <= 0)
case MAIL_CMD:
HAD(SCH_MAIL);
smtp_mailcmd_count++; /* Count for limit and ratelimit */
+ message_start();
was_rej_mail = TRUE; /* Reset if accepted */
env_mail_type_t * mail_args; /* Sanity check & validate args */
diff --git a/src/src/store.c b/src/src/store.c
index df7078f..9d15582 100644
--- a/src/src/store.c
+++ b/src/src/store.c
@@ -37,11 +37,15 @@ The following different types of store are recognized:
This means it can be freed when search_tidyup() is called to close down all
the lookup caching.
+- There is another pool (POOL_MESSAGE) used for medium-lifetime objects; within
+ a single message transaction but needed for longer than the use of the main
+ pool permits. Currently this means only receive-time DKIM information.
+
. Orthogonal to the three pool types, there are two classes of memory: untainted
and tainted. The latter is used for values derived from untrusted input, and
the string-expansion mechanism refuses to operate on such values (obviously,
it can expand an untainted value to return a tainted result). The classes
- are implemented by duplicating the three pool types. Pool resets are requested
+ are implemented by duplicating the four pool types. Pool resets are requested
against the nontainted sibling and apply to both siblings.
Only memory blocks requested for tainted use are regarded as tainted; anything
@@ -113,11 +117,10 @@ even if the length is zero (which is used for getting a point to reset to). */
int store_pool = POOL_MAIN;
-#define NPOOLS 6
static storeblock *chainbase[NPOOLS];
static storeblock *current_block[NPOOLS];
static void *next_yield[NPOOLS];
-static int yield_length[NPOOLS] = { -1, -1, -1, -1, -1, -1 };
+static int yield_length[NPOOLS];
/* pool_malloc holds the amount of memory used by the store pools; this goes up
and down as store is reset or released. nonpool_malloc is the total got by
@@ -150,17 +153,22 @@ static const uschar * pooluse[NPOOLS] = {
[POOL_MAIN] = US"main",
[POOL_PERM] = US"perm",
[POOL_SEARCH] = US"search",
+[POOL_MESSAGE] = US"message",
[POOL_TAINT_MAIN] = US"main",
[POOL_TAINT_PERM] = US"perm",
[POOL_TAINT_SEARCH] = US"search",
+[POOL_TAINT_SEARCH] = US"search",
+[POOL_TAINT_MESSAGE] = US"message",
};
static const uschar * poolclass[NPOOLS] = {
[POOL_MAIN] = US"untainted",
[POOL_PERM] = US"untainted",
[POOL_SEARCH] = US"untainted",
+[POOL_MESSAGE] = US"untainted",
[POOL_TAINT_MAIN] = US"tainted",
[POOL_TAINT_PERM] = US"tainted",
[POOL_TAINT_SEARCH] = US"tainted",
+[POOL_TAINT_MESSAGE] = US"tainted",
};
#endif
@@ -169,6 +177,16 @@ static void * internal_store_malloc(int, const char *, int);
static void internal_store_free(void *, const char *, int linenumber);
/******************************************************************************/
+/* Initialisation, for things fragile with parameter channges when using
+static initialisers. */
+
+void
+store_init(void)
+{
+for (int i = 0; i < NPOOLS; i++) yield_length[i] = -1;
+}
+
+/******************************************************************************/
/* Test if a pointer refers to tainted memory.
@@ -526,19 +544,19 @@ DEBUG(D_memory)
rmark
-store_reset_3(rmark r, int pool, const char *func, int linenumber)
+store_reset_3(rmark r, const char *func, int linenumber)
{
void ** ptr = r;
-if (pool >= POOL_TAINT_BASE)
+if (store_pool >= POOL_TAINT_BASE)
log_write(0, LOG_MAIN|LOG_PANIC_DIE,
- "store_reset called for pool %d: %s %d\n", pool, func, linenumber);
+ "store_reset called for pool %d: %s %d\n", store_pool, func, linenumber);
if (!r)
log_write(0, LOG_MAIN|LOG_PANIC_DIE,
"store_reset called with bad mark: %s %d\n", func, linenumber);
-internal_store_reset(*ptr, pool + POOL_TAINT_BASE, func, linenumber);
-internal_store_reset(ptr, pool, func, linenumber);
+internal_store_reset(*ptr, store_pool + POOL_TAINT_BASE, func, linenumber);
+internal_store_reset(ptr, store_pool, func, linenumber);
return NULL;
}
@@ -837,4 +855,29 @@ DEBUG(D_memory)
#endif
}
+
+/******************************************************************************/
+/* Per-message pool management */
+
+static rmark message_reset_point = NULL;
+
+void
+message_start(void)
+{
+int oldpool = store_pool;
+store_pool = POOL_MESSAGE;
+if (!message_reset_point) message_reset_point = store_mark();
+store_pool = oldpool;
+}
+
+void message_tidyup(void)
+{
+int oldpool;
+if (!message_reset_point) return;
+oldpool = store_pool;
+store_pool = POOL_MESSAGE;
+message_reset_point = store_reset(message_reset_point);
+store_pool = oldpool;
+}
+
/* End of store.c */
diff --git a/src/src/store.h b/src/src/store.h
index 51dffab..92deabf 100644
--- a/src/src/store.h
+++ b/src/src/store.h
@@ -13,15 +13,24 @@
/* Define symbols for identifying the store pools. */
-enum { POOL_MAIN, POOL_PERM, POOL_SEARCH,
+#define NPOOLS 8
+enum { POOL_MAIN,
+ POOL_PERM,
+ POOL_SEARCH,
+ POOL_MESSAGE,
+
POOL_TAINT_BASE,
- POOL_TAINT_MAIN = POOL_TAINT_BASE, POOL_TAINT_PERM, POOL_TAINT_SEARCH };
+
+ POOL_TAINT_MAIN = POOL_TAINT_BASE,
+ POOL_TAINT_PERM,
+ POOL_TAINT_SEARCH,
+ POOL_TAINT_MESSAGE };
/* This variable (the one for the current pool) is set by store_get() to its
yield, and by store_reset() to NULL. This allows string_cat() to optimize its
store handling. */
-extern void *store_last_get[6];
+extern void *store_last_get[NPOOLS];
/* This variable contains the current store pool number. */
@@ -45,7 +54,7 @@ tracing information for debugging. */
#define store_release_above(addr) \
store_release_above_3(addr, __FUNCTION__, __LINE__)
#define store_reset(mark) \
- store_reset_3(mark, store_pool, __FUNCTION__, __LINE__)
+ store_reset_3(mark, __FUNCTION__, __LINE__)
/* The real functions */
@@ -58,7 +67,7 @@ extern void *store_malloc_3(int, const char *, int) ALLOC ALLOC_SIZE(1) WARN_
extern rmark store_mark_3(const char *, int);
extern void *store_newblock_3(void *, BOOL, int, int, const char *, int);
extern void store_release_above_3(void *, const char *, int);
-extern rmark store_reset_3(rmark, int, const char *, int);
+extern rmark store_reset_3(rmark, const char *, int);
#endif /* STORE_H */
diff --git a/src/src/string.c b/src/src/string.c
index 4726fbe..7af87f9 100644
--- a/src/src/string.c
+++ b/src/src/string.c
@@ -1692,6 +1692,7 @@ int main(void)
uschar buffer[256];
printf("Testing is_ip_address\n");
+store_init();
while (fgets(CS buffer, sizeof(buffer), stdin) != NULL)
{
