Replace the store_release() internal interface, which was excessively unsafe.

The new store_newblock() includes the required safety checck, plus the alocate
and data-copy operations.

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 0ea49a280132de83a34d923389ad0c31deea3a03..0dd8ca60ffd1c0c9a08bd7df29455982583f83e5 100644 (file)
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -5,6 +5,14 @@ affect Exim's operation, with an unchanged configuration file.  For new
 options, and new features, see the NewStuff file next to this ChangeLog.
 
 
+Exim version 4.next
+-----------------
+
+JH/01 Replace the store_release() internal interface with store_newblock(),
+      which internalises the check required to safely use the old one, plus
+      the allocate and data copy operations duplicated in both (!) of the
+      extant use locations.
+
 Exim version 4.90
 -----------------
 

diff --git a/src/src/receive.c b/src/src/receive.c
index d9b500102af6cad10275713fdbd4b34a5c22d22b..93c091c808189b098dcc330100ddb234d7960031 100644 (file)
--- a/src/src/receive.c
+++ b/src/src/receive.c
@@ -1819,13 +1819,7 @@ for (;;)
     /* header_size += 256; */
     header_size *= 2;
     if (!store_extend(next->text, oldsize, header_size))
-      {
-      BOOL release_ok = store_last_get[store_pool] == next->text;
-      uschar *newtext = store_get(header_size);
-      memcpy(newtext, next->text, ptr);
-      if (release_ok) store_release(next->text);
-      next->text = newtext;
-      }
+      next->text = store_newblock(next->text, header_size, ptr);
     }
 
   /* Cope with receiving a binary zero. There is dispute about whether

diff --git a/src/src/store.c b/src/src/store.c
index c7cf33c9ac687933914d676fe494704e049876a8..8f5da3a5a7807a47927e7fdb804ea06b08c509b0 100644 (file)
--- a/src/src/store.c
+++ b/src/src/store.c
@@ -428,14 +428,8 @@ DEBUG(D_memory)
 *             Release store                     *
 ************************************************/
 
-/* This function is specifically provided for use when reading very
-long strings, e.g. header lines. When the string gets longer than a
-complete block, it gets copied to a new block. It is helpful to free
-the old block iff the previous copy of the string is at its start,
-and therefore the only thing in it. Otherwise, for very long strings,
-dead store can pile up somewhat disastrously. This function checks that
-the pointer it is given is the first thing in a block, and if so,
-releases that block.
+/* This function checks that the pointer it is given is the first thing in a
+block, and if so, releases that block.
 
 Arguments:
   block       block of store to consider
@@ -445,17 +439,17 @@ Arguments:
 Returns:      nothing
 */
 
-void
-store_release_3(void *block, const char *filename, int linenumber)
+static void
+store_release_3(void * block, const char * filename, int linenumber)
 {
-storeblock *b;
+storeblock * b;
 
 /* It will never be the first block, so no need to check that. */
 
-for (b = chainbase[store_pool]; b != NULL; b = b->next)
+for (b = chainbase[store_pool]; b; b = b->next)
   {
-  storeblock *bb = b->next;
-  if (bb != NULL && CS block == CS bb + ALIGNED_SIZEOF_STOREBLOCK)
+  storeblock * bb = b->next;
+  if (bb && CS block == CS bb + ALIGNED_SIZEOF_STOREBLOCK)
     {
     b->next = bb->next;
     pool_malloc -= bb->length + ALIGNED_SIZEOF_STOREBLOCK;
@@ -463,21 +457,20 @@ for (b = chainbase[store_pool]; b != NULL; b = b->next)
     /* Cut out the debugging stuff for utilities, but stop picky compilers
     from giving warnings. */
 
-    #ifdef COMPILE_UTILITY
+#ifdef COMPILE_UTILITY
     filename = filename;
     linenumber = linenumber;
-    #else
+#else
     DEBUG(D_memory)
-      {
       if (running_in_test_harness)
         debug_printf("-Release       %d\n", pool_malloc);
       else
         debug_printf("-Release %6p %-20s %4d %d\n", (void *)bb, filename,
           linenumber, pool_malloc);
-      }
+
     if (running_in_test_harness)
       memset(bb, 0xF0, bb->length+ALIGNED_SIZEOF_STOREBLOCK);
-    #endif  /* COMPILE_UTILITY */
+#endif  /* COMPILE_UTILITY */
 
     free(bb);
     return;
@@ -486,6 +479,43 @@ for (b = chainbase[store_pool]; b != NULL; b = b->next)
 }
 
 
+/************************************************
+*             Move store                        *
+************************************************/
+
+/* Allocate a new block big enough to expend to the given size and
+copy the current data into it.  Free the old one if possible.
+
+This function is specifically provided for use when reading very
+long strings, e.g. header lines. When the string gets longer than a
+complete block, it gets copied to a new block. It is helpful to free
+the old block iff the previous copy of the string is at its start,
+and therefore the only thing in it. Otherwise, for very long strings,
+dead store can pile up somewhat disastrously. This function checks that
+the pointer it is given is the first thing in a block, and that nothing
+has been allocated since. If so, releases that block.
+
+Arguments:
+  block
+  newsize
+  len
+
+Returns:       new location of data
+*/
+
+void *
+store_newblock_3(void * block, int newsize, int len,
+  const char * filename, int linenumber)
+{
+BOOL release_ok = store_last_get[store_pool] == block;
+uschar * newtext = store_get(newsize);
+
+memcpy(newtext, block, len);
+if (release_ok) store_release_3(block, filename, linenumber);
+return (void *)newtext;
+}
+
+
 
 
 /*************************************************

diff --git a/src/src/store.h b/src/src/store.h
index 7c860f1541647adb6d45e7dc125d833553051e3c..cb0a3cae9c4883b0948501fab8fe3e1a380ac4e7 100644 (file)
--- a/src/src/store.h
+++ b/src/src/store.h
@@ -34,19 +34,21 @@ tracing information for debugging. */
 #define store_get(size)      store_get_3(size, __FILE__, __LINE__)
 #define store_get_perm(size) store_get_perm_3(size, __FILE__, __LINE__)
 #define store_malloc(size)   store_malloc_3(size, __FILE__, __LINE__)
-#define store_release(addr)  store_release_3(addr, __FILE__, __LINE__)
+#define store_newblock(addr,newsize,datalen) \
+                            store_newblock_3(addr, newsize, datalen, __FILE__, __LINE__)
 #define store_reset(addr)    store_reset_3(addr, __FILE__, __LINE__)
 
 
 /* The real functions */
 
-extern BOOL    store_extend_3(void *, int, int, const char *, int);  /* The */
-extern void    store_free_3(void *, const char *, int);     /* value of the */
-extern void   *store_get_3(int, const char *, int);         /* 2nd arg is   */
-extern void   *store_get_perm_3(int, const char *, int);    /* __FILE__ in  */
-extern void   *store_malloc_3(int, const char *, int);      /* every call,  */
-extern void    store_release_3(void *, const char *, int);  /* so give its  */
-extern void    store_reset_3(void *, const char *, int);    /* correct type */
+/* The value of the 2nd arg is __FILE__ in every call, so give its correct type */
+extern BOOL    store_extend_3(void *, int, int, const char *, int);
+extern void    store_free_3(void *, const char *, int);
+extern void   *store_get_3(int, const char *, int);
+extern void   *store_get_perm_3(int, const char *, int);
+extern void   *store_malloc_3(int, const char *, int);
+extern void   *store_newblock_3(void *, int, int, const char *, int);
+extern void    store_reset_3(void *, const char *, int);
 
 #endif  /* STORE_H */
 

diff --git a/src/src/string.c b/src/src/string.c
index 2e919e6d9761451b7665c7f4643aa929df41a184..81aacb94b1f6bd6f9767251a75df5a0994b0e393 100644 (file)
--- a/src/src/string.c
+++ b/src/src/string.c
@@ -1102,13 +1102,7 @@ was the last item on the dynamic memory stack. This is the case if it matches
 store_last_get. */
 
 if (!store_extend(g->s, oldsize, g->size))
-  {
-  BOOL release_ok = store_last_get[store_pool] == g->s;
-  uschar *newstring = store_get(g->size);
-  memcpy(newstring, g->s, p);
-  if (release_ok) store_release(g->s);
-  g->s = newstring;
-  }
+  g->s = store_newblock(g->s, g->size, p);
 }