[PATCH] stdmem: free: only call _gcry_secmem_free if needed

[Werner Koch]

Hi!

Thanks for this patch. I would even consider this for backporting.

> -  else if (!_gcry_secmem_free (p))
> +
> +  if (!_gcry_private_is_secure (freep) ||
> +      !_gcry_secmem_free (freep))

Not related to the above change but in general I think it is important
to add least add more comments to the code to warn about the assumptions
we make.

In particular when we allocate a new overflow pool in the secure memory
we do this (in a locked state):

--8<---------------cut here---------------start------------->8---
      /* Take care: in _gcry_private_is_secure we do not lock and thus
       * we assume that the second assignment below is atomic.  */
      pool->next = mainpool.next;
      mainpool.next = pool;
--8<---------------cut here---------------end--------------->8---

MAINPOOL is never changed and thus as long the last assignment is atomic
we should always be able to call:

--8<---------------cut here---------------start------------->8---
/* Return true if P points into the secure memory areas.  */
int
_gcry_private_is_secure (const void *p)
{
  pooldesc_t *pool;

  /* We do no lock here because once a pool is allocated it will not
   * be removed anymore (except for gcry_secmem_term).  Further,
   * adding a new pool to the list should be atomic.  */
  for (pool = &mainpool; pool; pool = pool->next)
    if (pool->okay && ptr_into_pool_p (pool, p))
      return 1;

  return 0;
}
--8<---------------cut here---------------end--------------->8---

Do we need to think about memory visibility here?  Are there case where
we pass a P which has been allocated in secure memory but the test does
not catch it because the thread in the second function and the one in
the first have a different view of the memory?  Bypassing locking or
having no guaranteed memory barrier is a difficult thing.  Just asking.


Shalom-Salam,

   Werner

-- 
Die Gedanken sind frei.  Ausnahmen regelt ein Bundesgesetz.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.gnupg.org/pipermail/gcrypt-devel/attachments/20181029/57109c40/attachment-0001.sig>