[patch] bus error in gcry_free

[Werner Koch]

On Thu,  1 Feb 2007 14:34, christianbiere at gmx.de said:

> This patch is nonsense. It may work if you cast to size_t or unsigned long.

Huh?

  return (pool_okay
          && p >= pool
          && p < (const void*)((const char*)pool+pool_size));

I agree that the casts are not required but they don't harm either.
Actually I committed this:

  int
  _gcry_private_is_secure (const void *p)
  {
    return (pool_okay
            && p >= pool
            && p <  pool+pool_size);
  }
  
with 

  /* The pool of secure memory.  */
  static void *pool;
  
  /* Size of POOL in bytes.  */
  static size_t pool_size;
  
So what is the nonsense here?  This is a straightforward check to see
whether P points into POOL.  Compare this to the old code:

  if (pool_okay && BLOCK_VALID (ADDR_TO_BLOCK (p)))

Where the second term expands to:

  ((char*)((memblock_t*) ((char*)p - BLOCK_HEAD_SIZE)) - (char*)pool)
   < pool_size

As you can see we can easily get an address wrap here: If P == POOL+1
and assuming a BLOCK_HEAD_SIZE of 8 we get POOL + 1 - 8 - POOL == -7
and thus the test may or may not return false as it should.  With
P==POOL+8 we will even get 0 < pool_size which is always true.  The
function now falsely claims that P pointes into POOL and will act on
it with all kinds of bad consequences.



Shalom-Salam,

   Werner