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

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 6993499e98f0c427843acea091c63788796e8857..c1f6f9b6d59b21d6ed2d343ed07377c1f6b6399c 100644 (file)
--- 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.
 
       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
 -----------------
 
 Exim version 4.94
 -----------------

diff --git a/src/exim_monitor/em_main.c b/src/exim_monitor/em_main.c
index 88bf1fcb7bb61959053a0b376926411215da48f5..0a89fbb3871c7dcb8ee34364cb9d85c8270677e1 100644 (file)
--- a/src/exim_monitor/em_main.c
+++ b/src/exim_monitor/em_main.c
@@ -570,7 +570,8 @@ return ret;
 *               Initialize                       *
 *************************************************/
 
 *               Initialize                       *
 *************************************************/
 

-int main(int argc, char **argv)
+int
+main(int argc, char **argv)

 {
 int i;
 struct stat statdata;
 {
 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. */
 
 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 */
 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 92adb35897665f650653de3622c639f72800361f..a48f1a17af47e42b309e38118484ccee53f2f247 100644 (file)
--- a/src/src/dkim.c
+++ b/src/src/dkim.c
@@ -109,12 +109,15 @@ dkim_exim_verify_init(BOOL dot_stuffing)
 {
 dkim_exim_init();
 
 {
 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;
 
 dkim_verify_oldpool = store_pool;

-store_pool = POOL_PERM;
+store_pool = POOL_MESSAGE;

 
 /* Free previous context if there is one */
 
 
 /* Free previous context if there is one */
 


@@ -139,7 +142,7 @@ dkim_exim_verify_feed(uschar * data, int len)
 {
 int rc;
 
 {
 int rc;
 

-store_pool = POOL_PERM;
+store_pool = POOL_MESSAGE;

 if (  dkim_collect_input
    && (rc = pdkim_feed(dkim_verify_ctx, data, len)) != PDKIM_OK)
   {
 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;
 
 gstring * g = NULL;
 const uschar * errstr = NULL;
 

-store_pool = POOL_PERM;
+store_pool = POOL_MESSAGE;

 
 /* Delete eventual previous signature chain */
 
 
 /* Delete eventual previous signature chain */
 

diff --git a/src/src/exim.c b/src/src/exim.c
index 8f33bde2642b0a873bfdc479d83432b04c6c1e8d..cb11a2a38bf16ef3793eda513e0a341060c46313 100644 (file)
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -1669,6 +1669,8 @@ extern char **environ;
 (void)gettimeofday(&timestamp_startup, NULL);
 #endif
 
 (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. */
 /* 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 7cf2e47756cefef3c1409828753abade47edaf3a..da8d5707a6e8f1f495e02a49389ebc51e3cb6747 100644 (file)
--- 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_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)
 { 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 7ca7a307361607b65b7d0233390120fdb469ad2b..2ae7ef44d03c5895d28430e976f50f665bb4623a 100644 (file)
--- 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];
 
 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"");
 /* 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;
 
 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
 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;
 
 uschar buffer[256];
 uschar *key;
 

+store_init();
+

 /* Scan the options */
 
 for (i = 1; i < argc; i++)
 /* Scan the options */
 
 for (i = 1; i < argc; i++)

diff --git a/src/src/expand.c b/src/src/expand.c
index 8be7bf97b074fd3b36eeb4ddf7dfaa6b77e65dbe..8a571b2d905c6ed4bdf6a35c4a8ef6b8aa2a9a4b 100644 (file)
--- 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);
 debug_file = stderr;
 debug_fd = fileno(debug_file);
 big_buffer = malloc(big_buffer_size);

+store_init();

 
 for (int i = 1; i < argc; i++)
   {
 
 for (int i = 1; i < argc; i++)
   {

diff --git a/src/src/functions.h b/src/src/functions.h
index 5ffb23d1e0cced19d0b503a754c1913eadc747e1..6d16e69c27e915e32bf9a272d6875f67125eebea 100644 (file)
--- 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 **);
                  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 *);
 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 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;
 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 6b9f674b83805e3a74c46d8913d674cc139f16c3..c25ff2a74f9a2377af805c814c655c72e8737bc5 100644 (file)
--- a/src/src/host.c
+++ b/src/src/host.c
@@ -3260,6 +3260,7 @@ uschar buffer[256];
 
 disable_ipv6 = FALSE;
 primary_hostname = US"";
 
 disable_ipv6 = FALSE;
 primary_hostname = US"";

+store_init();

 store_pool = POOL_MAIN;
 debug_selector = D_host_lookup|D_interface;
 debug_file = stdout;
 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 2b2ffd34129ad0e4aaabaf74ed98b493040f3285..885a01c0d78b4d427eef77193a26e4b7f1a48fb6 100644 (file)
--- a/src/src/parse.c
+++ b/src/src/parse.c
@@ -2083,6 +2083,7 @@ int main(void)
 int start, end, domain;
 uschar buffer[1024];
 
 int start, end, domain;
 uschar buffer[1024];
 

+store_init();

 big_buffer = store_malloc(big_buffer_size);
 
 /* strip_trailing_dot = TRUE; */
 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 03085c9e4b2f7faf6fec37a2fbf341084dbb8125..6d6370ffd9d3f2e06e4d67f34be84bf8b8606ede 100644 (file)
--- 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);
   }
   acl_warn_logged = acl_warn_logged->next;
   store_free(this);
   }

+
+message_tidyup();

 store_reset(reset_point);
 store_reset(reset_point);

+
+message_start();

 return store_mark();
 }
 
 return store_mark();
 }
 


@@ -2186,7 +2190,7 @@ while (done <= 0)
 
     case MAIL_CMD:
       smtp_mailcmd_count++;              /* Count for no-mail log */
 
     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");
 
        /* 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 */
     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 */
 
       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 df7078fea24a7c4f9055d37b9b197dc35890e4d4..9d155821b76146af232c9f4b2c801792a05672a9 100644 (file)
--- 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.
 
   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
 . 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
   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;
 
 
 int store_pool = POOL_MAIN;
 

-#define NPOOLS 6

 static storeblock *chainbase[NPOOLS];
 static storeblock *current_block[NPOOLS];
 static void *next_yield[NPOOLS];
 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
 
 /* 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_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_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",
 };
 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_MAIN] =    US"tainted",
 [POOL_TAINT_PERM] =    US"tainted",
 [POOL_TAINT_SEARCH] =  US"tainted",

+[POOL_TAINT_MESSAGE] = US"tainted",

 };
 #endif
 
 };
 #endif
 


@@ -168,6 +176,16 @@ static const uschar * poolclass[NPOOLS] = {
 static void * internal_store_malloc(int, const char *, int);
 static void   internal_store_free(void *, const char *, int linenumber);
 
 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.
 /******************************************************************************/
 
 /* Test if a pointer refers to tainted memory.


@@ -526,19 +544,19 @@ DEBUG(D_memory)
 
 
 rmark
 
 
 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;
 
 {
 void ** ptr = r;
 

-if (pool >= POOL_TAINT_BASE)
+if (store_pool >= POOL_TAINT_BASE)

   log_write(0, LOG_MAIN|LOG_PANIC_DIE,
   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);
 
 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;
 }
 
 return NULL;
 }
 


@@ -837,4 +855,29 @@ DEBUG(D_memory)
 #endif
 }
 
 #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 */
 /* End of store.c */

diff --git a/src/src/store.h b/src/src/store.h
index 51dffab78e396e965584613accf0c34c616f11b1..92deabf9bbe0e228a4a220f85042ac4ebd9c641c 100644 (file)
--- a/src/src/store.h
+++ b/src/src/store.h
@@ -13,15 +13,24 @@
 
 /* Define symbols for identifying the store pools. */
 
 
 /* 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_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. */
 
 
 /* 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. */
 
 
 /* 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) \
 #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 */
 
 
 /* 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_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 */
 
 
 #endif  /* STORE_H */
 

diff --git a/src/src/string.c b/src/src/string.c
index 4726fbe4e30e0de5b7fa5256dd4c10a6199d1dbd..7af87f92c2ce5fffff843d85e4f2c84d93e7d228 100644 (file)
--- 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");
 uschar buffer[256];
 
 printf("Testing is_ip_address\n");

+store_init();

 
 while (fgets(CS buffer, sizeof(buffer), stdin) != NULL)
   {
 
 while (fgets(CS buffer, sizeof(buffer), stdin) != NULL)
   {