Memory management: when running under the testsuite, check every string variable on store_reset

On spotting data in a region being freed, panic

3 files changed, 75 insertions, 14 deletions

diff --git a/src/src/expand.c b/src/src/expand.c
index dfd62e50c..d35bf9901 100644
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -1966,6 +1966,7 @@ return;          /* Unknown variable name, fail silently */
 
 
 
+
 /*************************************************
 *           Read and expand substrings           *
 *************************************************/
@@ -7776,6 +7777,55 @@ return (  (  Ustrstr(s, "failed to expand") != NULL
 
 
 
+/*************************************************
+* Error-checking for testsuite                   *
+*************************************************/
+typedef struct {
+  const char *	filename;
+  int		linenumber;
+  uschar * 	region_start;
+  uschar *	region_end;
+  const uschar *var_name;
+} err_ctx;
+
+static void
+assert_variable_notin(uschar * var_name, uschar * var_data, void * ctx)
+{
+err_ctx * e = ctx;
+if (var_data >= e->region_start  &&  var_data < e->region_end)
+  e->var_name = CUS var_name;
+}
+
+void
+assert_no_variables(void * ptr, int len, const char * filename, int linenumber)
+{
+err_ctx e = {filename, linenumber, ptr, US ptr + len, NULL };
+int i;
+var_entry * v;
+
+/* check acl_ variables */
+tree_walk(acl_var_c, assert_variable_notin, &e);
+tree_walk(acl_var_m, assert_variable_notin, &e);
+
+/* check auth<n> variables */
+for (i = 0; i < AUTH_VARS; i++) if (auth_vars[i])
+  assert_variable_notin(US"auth<n>", auth_vars[i], &e);
+
+/* check regex<n> variables */
+for (i = 0; i < REGEX_VARS; i++) if (regex_vars[i])
+  assert_variable_notin(US"regex<n>", regex_vars[i], &e);
+
+/* check known-name variables */
+for (v = var_table; v < var_table + var_table_size; v++)
+  if (v->type == vtype_stringptr)
+    assert_variable_notin(US v->name, *(USS v->value), &e);
+
+if (e.var_name)
+  log_write(0, LOG_MAIN|LOG_PANIC_DIE, "live variable '%s' destroyed by reset_store"
+    " at %s:%d\n", e.var_name, e.filename, e.linenumber);
+}
+
+
 
 /*************************************************
 **************************************************
diff --git a/src/src/functions.h b/src/src/functions.h
index 9bc66ef7b..790d8faf7 100644
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -86,6 +86,7 @@ extern int     acl_eval(int, uschar *, uschar **, uschar **);
 
 extern tree_node *acl_var_create(uschar *);
 extern void    acl_var_write(uschar *, uschar *, void *);
+extern void    assert_no_variables(void *, int, const char *, int);
 extern int     auth_call_pam(const uschar *, uschar **);
 extern int     auth_call_pwcheck(uschar *, uschar **);
 extern int     auth_call_radius(const uschar *, uschar **);
diff --git a/src/src/store.c b/src/src/store.c
index b1a47799b..e88555cbf 100644
--- a/src/src/store.c
+++ b/src/src/store.c
@@ -325,9 +325,9 @@ Returns:      nothing
 void
 store_reset_3(void *ptr, const char *filename, int linenumber)
 {
-storeblock *bb;
-storeblock *b = current_block[store_pool];
-char *bc = (char *)b + ALIGNED_SIZEOF_STOREBLOCK;
+storeblock * bb;
+storeblock * b = current_block[store_pool];
+char * bc = CS b + ALIGNED_SIZEOF_STOREBLOCK;
 int newlength;
 
 /* Last store operation was not a get */
@@ -337,14 +337,14 @@ store_last_get[store_pool] = NULL;
 /* See if the place is in the current block - as it often will be. Otherwise,
 search for the block in which it lies. */
 
-if ((char *)ptr < bc || (char *)ptr > bc + b->length)
+if (CS ptr < bc || CS ptr > bc + b->length)
   {
-  for (b = chainbase[store_pool]; b != NULL; b = b->next)
+  for (b = chainbase[store_pool]; b; b = b->next)
     {
-    bc = (char *)b + ALIGNED_SIZEOF_STOREBLOCK;
-    if ((char *)ptr >= bc && (char *)ptr <= bc + b->length) break;
+    bc = CS b + ALIGNED_SIZEOF_STOREBLOCK;
+    if (CS ptr >= bc && CS ptr <= bc + b->length) break;
     }
-  if (b == NULL)
+  if (!b)
     log_write(0, LOG_MAIN|LOG_PANIC_DIE, "internal error: store_reset(%p) "
       "failed: pool=%d %-14s %4d", ptr, store_pool, filename, linenumber);
   }
@@ -352,17 +352,18 @@ if ((char *)ptr < bc || (char *)ptr > bc + b->length)
 /* Back up, rounding to the alignment if necessary. When testing, flatten
 the released memory. */
 
-newlength = bc + b->length - (char *)ptr;
+newlength = bc + b->length - CS ptr;
 #ifndef COMPILE_UTILITY
 if (running_in_test_harness)
   {
+  assert_no_variables(ptr, newlength, filename, linenumber);
   (void) VALGRIND_MAKE_MEM_DEFINED(ptr, newlength);
   memset(ptr, 0xF0, newlength);
   }
 #endif
 (void) VALGRIND_MAKE_MEM_NOACCESS(ptr, newlength);
 yield_length[store_pool] = newlength - (newlength % alignment);
-next_yield[store_pool] = (char *)ptr + (newlength % alignment);
+next_yield[store_pool] = CS ptr + (newlength % alignment);
 current_block[store_pool] = b;
 
 /* Free any subsequent block. Do NOT free the first successor, if our
@@ -370,20 +371,29 @@ current block has less than 256 bytes left. This should prevent us from
 flapping memory. However, keep this block only when it has the default size. */
 
 if (yield_length[store_pool] < STOREPOOL_MIN_SIZE &&
-    b->next != NULL &&
+    b->next &&
     b->next->length == STORE_BLOCK_SIZE)
   {
   b = b->next;
-  (void) VALGRIND_MAKE_MEM_NOACCESS((char *)b + ALIGNED_SIZEOF_STOREBLOCK,
+#ifndef COMPILE_UTILITY
+  if (running_in_test_harness)
+    assert_no_variables(b, b->length + ALIGNED_SIZEOF_STOREBLOCK,
+			filename, linenumber);
+#endif
+  (void) VALGRIND_MAKE_MEM_NOACCESS(CS b + ALIGNED_SIZEOF_STOREBLOCK,
 		b->length - ALIGNED_SIZEOF_STOREBLOCK);
   }
 
 bb = b->next;
 b->next = NULL;
 
-while (bb != NULL)
+while ((b = bb))
   {
-  b = bb;
+#ifndef COMPILE_UTILITY
+  if (running_in_test_harness)
+    assert_no_variables(b, b->length + ALIGNED_SIZEOF_STOREBLOCK,
+			filename, linenumber);
+#endif
   bb = bb->next;
   pool_malloc -= b->length + ALIGNED_SIZEOF_STOREBLOCK;
   store_free_3(b, filename, linenumber);