DKIM: verify using separate pool-pair, reset per message

13 files changed, 100 insertions, 22 deletions

diff --git a/src/exim_monitor/em_main.c b/src/exim_monitor/em_main.c
index 88bf1fcb7..0a89fbb38 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 92adb3589..a48f1a17a 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 8f33bde26..cb11a2a38 100644
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -1669,6 +1669,8 @@ extern char **environ;
 (void)gettimeofday(&timestamp_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 7cf2e4775..da8d5707a 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 7ca7a3073..2ae7ef44d 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 8be7bf97b..8a571b2d9 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 5ffb23d1e..6d16e69c2 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 6b9f674b8..c25ff2a74 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 2b2ffd341..885a01c0d 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 03085c9e4..6d6370ffd 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 df7078fea..9d155821b 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 51dffab78..92deabf9b 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 4726fbe4e..7af87f92c 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)
   {