[PATCH] apr_pools.c patch for bounds checking compiler

[Blair Zajac]

I'm using the bounds checking gcc 3.1.1 to check for memory issues
in Apache and Subversion.  This patch to gcc compiles the code with
extra checks for illegal memory accesses, invalid pointers, etc and
runs a lot faster than valgrind.  See

    http://web.inter.nl.net/hcc/Haj.Ten.Brugge/

There's a core dump from the bounds checking compiler when running
httpd -l with today's HEAD

% /opt/i386-linux/installed/apache-2.0-cvs-2002073101/bin/httpd -l
Bounds Checking GCC v gcc-3.1.1-3.1 Copyright (C) 1995 Richard W.M. Jones
Bounds Checking comes with ABSOLUTELY NO WARRANTY. For details see file
`COPYING' that should have come with the source to this program.
Bounds Checking is free software, and you are welcome to redistribute it
under certain conditions. See the file `COPYING' for details.
For more information, set GCC_BOUNDS_OPTS to `-help'
apr_pools.c:617:Bounds error: NULL or ILLEGAL pointer used in <, >, <= or >= of 
pointers.
apr_pools.c:617:  Left pointer value: ILLEGAL
apr_pools.c:617:  Right pointer value: 0x81a0000
Abort (core dumped)

This is at

APR_DECLARE(void *) apr_palloc(apr_pool_t *pool, apr_size_t size)
...
    size = APR_ALIGN_DEFAULT(size);
    active = pool->active;

    /* If the active node has enough bytes left, use it. */
    endp = active->first_avail + size;
    if (endp < active->endp) {

The bounds checking httpd checks if

    endp = active->first_avail + size

is a valid pointer into the buffer and sets endp to (void *)-1 if it
is not and then core dumps on the "if (endp < active->endp)" test.


endp may be set to -1 because endp points past the end of the memory
buffer (past active->endp) and hence is not a valid pointer according
to the ISO standard.

The patch changes the test to

    if (size <= active->endp - active->first_avail) {

and only computes active->first_avail + size if the pointer will be
valid.

The only question is if the <= should be a <.

Even without this issue, should the test "if (endp < active->endp)" be
"if (endp <= active->endp)"?  Otherwise there may be wasted memory in
this buffer?

Best,
Blair

-- 
Blair Zajac <[EMAIL PROTECTED]>
Web and OS performance plots - http://www.orcaware.com/orca/
Index: memory/unix/apr_pools.c
===================================================================
RCS file: /home/cvspublic/apr/memory/unix/apr_pools.c,v
retrieving revision 1.183
diff -u -r1.183 apr_pools.c
--- memory/unix/apr_pools.c     13 Jul 2002 21:38:02 -0000      1.183
+++ memory/unix/apr_pools.c     31 Jul 2002 21:14:22 -0000
@@ -606,24 +606,21 @@
 {
     apr_memnode_t *active, *node;
     void *mem;
-    char *endp;
     apr_uint32_t free_index;
 
     size = APR_ALIGN_DEFAULT(size);
     active = pool->active;
 
     /* If the active node has enough bytes left, use it. */
-    endp = active->first_avail + size;
-    if (endp < active->endp) {
+    if (size <= active->endp - active->first_avail) {
         mem = active->first_avail;
-        active->first_avail = endp;
+        active->first_avail = active->first_avail + size;
 
         return mem;
     }
 
     node = active->next;
-    endp = node->first_avail + size;
-    if (endp < node->endp) {
+    if (size <= node->endp - node->first_avail) {
         *node->ref = node->next;
         node->next->ref = node->ref;
     }
@@ -634,13 +631,12 @@
 
             return NULL;
         }
-        endp = node->first_avail + size;
     }
 
     node->free_index = 0;
 
     mem = node->first_avail;
-    node->first_avail = endp;
+    node->first_avail = node->first_avail + size;
 
     node->ref = active->ref;
     *node->ref = node;