From cf0812d57c63b531e2e73187508c7ae99156043c Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Tue, 7 Feb 2017 21:48:48 +0000 Subject: [PATCH] Memory management: when running under the testsuite, check every string variable on store_reset On spotting data in a region being freed, panic --- src/src/expand.c | 50 +++++++++++++++++++++++++++++++++++++++++++++ src/src/functions.h | 1 + src/src/store.c | 38 +++++++++++++++++++++------------- 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 variables */ +for (i = 0; i < AUTH_VARS; i++) if (auth_vars[i]) + assert_variable_notin(US"auth", auth_vars[i], &e); + +/* check regex variables */ +for (i = 0; i < REGEX_VARS; i++) if (regex_vars[i]) + assert_variable_notin(US"regex", 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); -- 2.25.1