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.
 
 
 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
 -----------------
 
 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))
     /* 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
     }
 
   /* 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                     *
 ************************************************/
 
 *             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
 
 Arguments:
   block       block of store to consider


@@ -445,17 +439,17 @@ Arguments:
 Returns:      nothing
 */
 
 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. */
 
 
 /* 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;
     {
     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. */
 
     /* Cut out the debugging stuff for utilities, but stop picky compilers
     from giving warnings. */
 

-    #ifdef COMPILE_UTILITY
+#ifdef COMPILE_UTILITY

     filename = filename;
     linenumber = linenumber;
     filename = filename;
     linenumber = linenumber;

-    #else
+#else

     DEBUG(D_memory)
     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)
         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);
     if (running_in_test_harness)
       memset(bb, 0xF0, bb->length+ALIGNED_SIZEOF_STOREBLOCK);

-    #endif  /* COMPILE_UTILITY */
+#endif  /* COMPILE_UTILITY */

 
     free(bb);
     return;
 
     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_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 */
 
 #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 */
 
 
 #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))
 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);

 }
 
 
 }