Addt patch needed; Re: cvs commit: apr/memory/unix apr_pools.c

["William A. Rowe, Jr." <wr...@covalent.net>]

> wrowe       01/09/28 08:22:35
> 
>   Modified:    .        CHANGES
>                include  apr_pools.h
>                memory/unix apr_pools.c
>   Log:
>     Introduce apr_pool_lock for debugging, in combination with
>     ALLOC_USE_MALLOC + DEBUG_WITH_MPROTECT.  Only implemented
>     on Win32 today, very effective for debugging pool constness.
>   
>     Fixed a double-reservation for the union block_hdr size in the
>     unix mprotect_alloc mmap() call.  I suspect the mmap can be
>     modified to implement this on Unix, move the #define DO_LOCK()
>     out of the win32 block if this is implemented.


I know there is an mmap hacker on this list that could whip up this
code in five minutes or less for unix;

>   +static void mprotect_lock(void *addr, int lock)
>   +{
>   +    size_t size = SIZEOF_BLOCK(addr);
>   +    DWORD prot = (lock ? PAGE_READONLY : PAGE_READWRITE);
>   +    BOOL rv = VirtualProtect((union block_hdr *)addr - 1, size, prot, &prot);
>   +    if (!rv) {
>   +        fprintf(stderr, "could not protect. errno=%d\n", errno);
>   +        abort();
>   +    }
>   +}

Rather than VirtualProtect, simply re-mmap the anonymous swap pages that 
were originally mapped at (union block_hdr *)addr - 1 for 'size' bytes 
from a read/write mapping to a read-only mapping.

Once that patch is in, defining ALLOC_USE_MALLOC and DEBUG_WITH_MPROTECT
will cause httpd to segfault if a module attempts to make _any_ change
to pconf, which would have instantly exposed the Pane/Stoddard overwrite
source errors in the mod_mime hash table code.  Would have saved us all
hours of debugging :)

If an httpd hacker wants to go further, the parent or prior request record 
could also be apr_pool_lock(ed) if we weren't locking the subpools (have to
think about that as a option) to assure they don't change the parent
request.

And if an apr hacker wants to go further, this logic _would_ work for our
regular pools, iff we use a free-list-per-pool rather than a global free
list, based on the defines APR_POOL_DEBUG + ALLOC_USE_MALLOC 
+ DEBUG_WITH_MPROTECT.  It would reduce the memory footprint for debugging
tenfold (every 2 byte apr_palloc() is given one full page of memory with 
ALLOC_USE_MALLOC right now.)

Anyway, I'm certain this will save authors a ton of grief, if they choose
to test with it.

Bill