Reorganize code in secmem.c.
authorWerner Koch <wk@gnupg.org>
Tue, 6 Dec 2016 20:20:54 +0000 (21:20 +0100)
committerWerner Koch <wk@gnupg.org>
Tue, 6 Dec 2016 20:20:54 +0000 (21:20 +0100)
* src/secmem.c (pooldesc_t): New type to collect information about one
pool.
(pool_size): Remove.  Now a member of pooldesc_t.
(pool_okay): Ditto.
(pool_is_mmapped): Ditto.
(pool): Rename variable ...
(mainpool): And change type to pooldesc_t.
(ptr_into_pool_p): Add arg 'pool'.
(mb_get_next): Ditto.
(mb_get_prev): Ditto.
(mb_merge): Ditto.
(mb_get_new): Ditto.
(init_pool): Ditto.
(lock_pool): Rename to ...
(look_pool_pages: this.
(secmem_init): Rename to ...
(_gcry_secmem_init_internal): this.  Add local var POOL and init with
address of MAINPOOL.
(_gcry_secmem_malloc_internal): Add local var POOL and init with
address of MAINPOOL.
(_gcry_private_is_secure): Ditto.
(_gcry_secmem_term): Ditto.
(_gcry_secmem_dump_stats): Ditto.
(_gcry_secmem_free_internal): Ditto.  Remove check for NULL arg.
(_gcry_secmem_free): Add check for NULL arg before taking the lock.
(_gcry_secmem_realloc): Factor most code out to ...
(_gcry_secmem_realloc_internal): this.
--

This change prepares future work to allow the use of several pools.

Signed-off-by: Werner Koch <wk@gnupg.org>
src/secmem.c

index c4e8414..1f92f17 100644 (file)
@@ -1,7 +1,7 @@
 /* secmem.c  - memory allocation from a secure heap
  * Copyright (C) 1998, 1999, 2000, 2001, 2002,
  *               2003, 2007 Free Software Foundation, Inc.
- * Copyright (C) 2013 g10 Code GmbH
+ * Copyright (C) 2013, 2016 g10 Code GmbH
  *
  * This file is part of Libgcrypt.
  *
@@ -59,20 +59,30 @@ typedef struct memblock
 /* This flag specifies that the memory block is in use.  */
 #define MB_FLAG_ACTIVE (1 << 0)
 
-/* The pool of secure memory.  */
-static void *pool;
+/* An object describing a memory pool.  */
+typedef struct pooldesc_s
+{
+  /* A memory buffer used as allocation pool.  */
+  void *mem;
+
+  /* The allocated size of MEM. */
+  size_t size;
+
+  /* Flag indicating that this memory pool is ready for use.  May be
+   * checked in an atexit function.  */
+  volatile int okay;
 
-/* Size of POOL in bytes.  */
-static size_t pool_size;
+  /* Flag indicating whether MEM is mmapped.  */
+  volatile int is_mmapped;
 
-/* True, if the memory pool is ready for use.  May be checked in an
-   atexit function.  */
-static volatile int pool_okay;
+} pooldesc_t;
 
-/* True, if the memory pool is mmapped.  */
-static volatile int pool_is_mmapped;
 
-/* FIXME?  */
+/* The pool of secure memory.  */
+static pooldesc_t mainpool;
+
+
+/* A couple of flags whith some beeing set early. */
 static int disable_secmem;
 static int show_warning;
 static int not_locked;
@@ -84,7 +94,7 @@ static int no_priv_drop;
 /* Stats.  */
 static unsigned int cur_alloced, cur_blocks;
 
-/* Lock protecting accesses to the memory pool.  */
+/* Lock protecting accesses to the memory pools.  */
 GPGRT_LOCK_DEFINE (secmem_lock);
 
 /* Convenient macros.  */
@@ -100,18 +110,18 @@ GPGRT_LOCK_DEFINE (secmem_lock);
 #define ADDR_TO_BLOCK(addr) \
   (memblock_t *) (void *) ((char *) addr - BLOCK_HEAD_SIZE)
 
-/* Check whether P points into the pool.  */
+/* Check whether P points into POOL.  */
 static int
-ptr_into_pool_p (const void *p)
+ptr_into_pool_p (pooldesc_t *pool, const void *p)
 {
   /* We need to convert pointers to addresses.  This is required by
      C-99 6.5.8 to avoid undefined behaviour.  See also
      http://lists.gnupg.org/pipermail/gcrypt-devel/2007-February/001102.html
   */
   uintptr_t p_addr    = (uintptr_t)p;
-  uintptr_t pool_addr = (uintptr_t)pool;
+  uintptr_t pool_addr = (uintptr_t)pool->mem;
 
-  return p_addr >= pool_addr && p_addr <  pool_addr + pool_size;
+  return p_addr >= pool_addr && p_addr <  pool_addr + pool->size;
 }
 
 /* Update the stats.  */
@@ -132,13 +142,13 @@ stats_update (size_t add, size_t sub)
 
 /* Return the block following MB or NULL, if MB is the last block.  */
 static memblock_t *
-mb_get_next (memblock_t *mb)
+mb_get_next (pooldesc_t *pool, memblock_t *mb)
 {
   memblock_t *mb_next;
 
   mb_next = (memblock_t *) (void *) ((char *) mb + BLOCK_HEAD_SIZE + mb->size);
 
-  if (! ptr_into_pool_p (mb_next))
+  if (! ptr_into_pool_p (pool, mb_next))
     mb_next = NULL;
 
   return mb_next;
@@ -147,18 +157,18 @@ mb_get_next (memblock_t *mb)
 /* Return the block preceding MB or NULL, if MB is the first
    block.  */
 static memblock_t *
-mb_get_prev (memblock_t *mb)
+mb_get_prev (pooldesc_t *pool, memblock_t *mb)
 {
   memblock_t *mb_prev, *mb_next;
 
-  if (mb == pool)
+  if (mb == pool->mem)
     mb_prev = NULL;
   else
     {
-      mb_prev = (memblock_t *) pool;
+      mb_prev = (memblock_t *) pool->mem;
       while (1)
        {
-         mb_next = mb_get_next (mb_prev);
+         mb_next = mb_get_next (pool, mb_prev);
          if (mb_next == mb)
            break;
          else
@@ -172,12 +182,12 @@ mb_get_prev (memblock_t *mb)
 /* If the preceding block of MB and/or the following block of MB
    exist and are not active, merge them to form a bigger block.  */
 static void
-mb_merge (memblock_t *mb)
+mb_merge (pooldesc_t *pool, memblock_t *mb)
 {
   memblock_t *mb_prev, *mb_next;
 
-  mb_prev = mb_get_prev (mb);
-  mb_next = mb_get_next (mb);
+  mb_prev = mb_get_prev (pool, mb);
+  mb_next = mb_get_next (pool, mb);
 
   if (mb_prev && (! (mb_prev->flags & MB_FLAG_ACTIVE)))
     {
@@ -190,11 +200,11 @@ mb_merge (memblock_t *mb)
 
 /* Return a new block, which can hold SIZE bytes.  */
 static memblock_t *
-mb_get_new (memblock_t *block, size_t size)
+mb_get_new (pooldesc_t *pool, memblock_t *block, size_t size)
 {
   memblock_t *mb, *mb_split;
 
-  for (mb = block; ptr_into_pool_p (mb); mb = mb_get_next (mb))
+  for (mb = block; ptr_into_pool_p (pool, mb); mb = mb_get_next (pool, mb))
     if (! (mb->flags & MB_FLAG_ACTIVE) && mb->size >= size)
       {
        /* Found a free block.  */
@@ -211,14 +221,14 @@ mb_get_new (memblock_t *block, size_t size)
 
            mb->size = size;
 
-           mb_merge (mb_split);
+           mb_merge (pool, mb_split);
 
          }
 
        break;
       }
 
-  if (! ptr_into_pool_p (mb))
+  if (! ptr_into_pool_p (pool, mb))
     {
       gpg_err_set_errno (ENOMEM);
       mb = NULL;
@@ -235,9 +245,11 @@ print_warn (void)
     log_info (_("Warning: using insecure memory!\n"));
 }
 
-/* Lock the memory pages into core and drop privileges.  */
+
+/* Lock the memory pages of pool P of size N into core and drop
+ * privileges.  */
 static void
-lock_pool (void *p, size_t n)
+lock_pool_pages (void *p, size_t n)
 {
 #if defined(USE_CAPABILITIES) && defined(HAVE_MLOCK)
   int err;
@@ -367,11 +379,11 @@ lock_pool (void *p, size_t n)
 
 /* Initialize POOL.  */
 static void
-init_pool (size_t n)
+init_pool (pooldesc_t *pool, size_t n)
 {
   memblock_t *mb;
 
-  pool_size = n;
+  pool->size = n;
 
   if (disable_secmem)
     log_bug ("secure memory is disabled");
@@ -391,10 +403,10 @@ init_pool (size_t n)
 # endif
     pgsize = (pgsize_val != -1 && pgsize_val > 0)? pgsize_val:DEFAULT_PAGE_SIZE;
 
-    pool_size = (pool_size + pgsize - 1) & ~(pgsize - 1);
+    pool->size = (pool->size + pgsize - 1) & ~(pgsize - 1);
 # ifdef MAP_ANONYMOUS
-    pool = mmap (0, pool_size, PROT_READ | PROT_WRITE,
-                 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+    pool->mem = mmap (0, pool->size, PROT_READ | PROT_WRITE,
+                     MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 # else /* map /dev/zero instead */
     {
       int fd;
@@ -403,40 +415,40 @@ init_pool (size_t n)
       if (fd == -1)
         {
           log_error ("can't open /dev/zero: %s\n", strerror (errno));
-          pool = (void *) -1;
+          pool->mem = (void *) -1;
         }
       else
         {
-          pool = mmap (0, pool_size,
-                       (PROT_READ | PROT_WRITE), MAP_PRIVATE, fd, 0);
+          pool->mem = mmap (0, pool->size,
+                           (PROT_READ | PROT_WRITE), MAP_PRIVATE, fd, 0);
           close (fd);
         }
     }
 # endif
-    if (pool == (void *) -1)
+    if (pool->mem == (void *) -1)
       log_info ("can't mmap pool of %u bytes: %s - using malloc\n",
-                (unsigned) pool_size, strerror (errno));
+                (unsigned) pool->size, strerror (errno));
     else
       {
-        pool_is_mmapped = 1;
-        pool_okay = 1;
+        pool->is_mmapped = 1;
+        pool->okay = 1;
       }
   }
 #endif /*HAVE_MMAP*/
 
-  if (!pool_okay)
+  if (!pool->okay)
     {
-      pool = malloc (pool_size);
-      if (!pool)
+      pool->mem = malloc (pool->size);
+      if (!pool->mem)
        log_fatal ("can't allocate memory pool of %u bytes\n",
-                  (unsigned) pool_size);
+                  (unsigned) pool->size);
       else
-       pool_okay = 1;
+       pool->okay = 1;
     }
 
   /* Initialize first memory block.  */
-  mb = (memblock_t *) pool;
-  mb->size = pool_size;
+  mb = (memblock_t *) pool->mem;
+  mb->size = pool->size;
   mb->flags = 0;
 }
 
@@ -482,11 +494,14 @@ _gcry_secmem_get_flags (void)
 }
 
 
-/* See _gcry_secmem_init.  This function is expected to be called with
  the secmem lock held. */
+/* This function initializes the main memory pool MAINPOOL.  Itis
* expected to be called with the secmem lock held.  */
 static void
-secmem_init (size_t n)
+_gcry_secmem_init_internal (size_t n)
 {
+  pooldesc_t *pool;
+
+  pool = &mainpool;
   if (!n)
     {
 #ifdef USE_CAPABILITIES
@@ -516,10 +531,10 @@ secmem_init (size_t n)
     {
       if (n < MINIMUM_POOL_SIZE)
        n = MINIMUM_POOL_SIZE;
-      if (! pool_okay)
+      if (! pool->okay)
        {
-         init_pool (n);
-         lock_pool (pool, n);
+         init_pool (pool, n);
+         lock_pool_pages (pool->mem, n);
        }
       else
        log_error ("Oops, secure memory pool already initialized\n");
@@ -537,7 +552,7 @@ _gcry_secmem_init (size_t n)
 {
   SECMEM_LOCK;
 
-  secmem_init (n);
+  _gcry_secmem_init_internal (n);
 
   SECMEM_UNLOCK;
 }
@@ -554,13 +569,16 @@ _gcry_secmem_module_init ()
 static void *
 _gcry_secmem_malloc_internal (size_t size)
 {
+  pooldesc_t *pool;
   memblock_t *mb;
 
-  if (!pool_okay)
+  pool = &mainpool;
+
+  if (!pool->okay)
     {
       /* Try to initialize the pool if the user forgot about it.  */
-      secmem_init (STANDARD_POOL_SIZE);
-      if (!pool_okay)
+      _gcry_secmem_init_internal (STANDARD_POOL_SIZE);
+      if (!pool->okay)
         {
           log_info (_("operation is not possible without "
                       "initialized secure memory\n"));
@@ -583,7 +601,7 @@ _gcry_secmem_malloc_internal (size_t size)
   /* Blocks are always a multiple of 32. */
   size = ((size + 31) / 32) * 32;
 
-  mb = mb_get_new ((memblock_t *) pool, size);
+  mb = mb_get_new (pool, (memblock_t *) pool->mem, size);
   if (mb)
     stats_update (size, 0);
 
@@ -605,11 +623,11 @@ _gcry_secmem_malloc (size_t size)
 static void
 _gcry_secmem_free_internal (void *a)
 {
+  pooldesc_t *pool;
   memblock_t *mb;
   int size;
 
-  if (!a)
-    return;
+  pool = &mainpool;
 
   mb = ADDR_TO_BLOCK (a);
   size = mb->size;
@@ -624,34 +642,35 @@ _gcry_secmem_free_internal (void *a)
   MB_WIPE_OUT (0x55);
   MB_WIPE_OUT (0x00);
 
+  /* Update stats.  */
   stats_update (0, size);
 
   mb->flags &= ~MB_FLAG_ACTIVE;
 
-  /* Update stats.  */
 
-  mb_merge (mb);
+  mb_merge (pool, mb);
 }
 
 /* Wipe out and release memory.  */
 void
 _gcry_secmem_free (void *a)
 {
+  if (!a)
+    return;
+
   SECMEM_LOCK;
   _gcry_secmem_free_internal (a);
   SECMEM_UNLOCK;
 }
 
-/* Realloc memory.  */
-void *
-_gcry_secmem_realloc (void *p, size_t newsize)
+
+static void *
+_gcry_secmem_realloc_internal (void *p, size_t newsize)
 {
   memblock_t *mb;
   size_t size;
   void *a;
 
-  SECMEM_LOCK;
-
   mb = (memblock_t *) (void *) ((char *) p
                                - ((size_t) &((memblock_t *) 0)->aligned.c));
   size = mb->size;
@@ -671,6 +690,18 @@ _gcry_secmem_realloc (void *p, size_t newsize)
        }
     }
 
+  return a;
+}
+
+
+/* Realloc memory.  */
+void *
+_gcry_secmem_realloc (void *p, size_t newsize)
+{
+  void *a;
+
+  SECMEM_LOCK;
+  a = _gcry_secmem_realloc_internal (p, newsize);
   SECMEM_UNLOCK;
 
   return a;
@@ -681,7 +712,10 @@ _gcry_secmem_realloc (void *p, size_t newsize)
 int
 _gcry_private_is_secure (const void *p)
 {
-  return pool_okay && ptr_into_pool_p (p);
+  pooldesc_t *pool;
+
+  pool = &mainpool;
+  return pool->okay && ptr_into_pool_p (pool, p);
 }
 
 
@@ -696,20 +730,23 @@ _gcry_private_is_secure (const void *p)
 void
 _gcry_secmem_term ()
 {
-  if (!pool_okay)
+  pooldesc_t *pool;
+
+  pool = &mainpool;
+  if (!pool->okay)
     return;
 
-  wipememory2 (pool, 0xff, pool_size);
-  wipememory2 (pool, 0xaa, pool_size);
-  wipememory2 (pool, 0x55, pool_size);
-  wipememory2 (pool, 0x00, pool_size);
+  wipememory2 (pool->mem, 0xff, pool->size);
+  wipememory2 (pool->mem, 0xaa, pool->size);
+  wipememory2 (pool->mem, 0x55, pool->size);
+  wipememory2 (pool->mem, 0x00, pool->size);
 #if HAVE_MMAP
-  if (pool_is_mmapped)
-    munmap (pool, pool_size);
+  if (pool->is_mmapped)
+    munmap (pool->mem, pool->size);
 #endif
-  pool = NULL;
-  pool_okay = 0;
-  pool_size = 0;
+  pool->mem = NULL;
+  pool->okay = 0;
+  pool->size = 0;
   not_locked = 0;
 }
 
@@ -717,12 +754,15 @@ _gcry_secmem_term ()
 void
 _gcry_secmem_dump_stats ()
 {
+  pooldesc_t *pool;
+
 #if 1
   SECMEM_LOCK;
 
- if (pool_okay)
+  pool = &mainpool;
+  if (pool->okay)
     log_info ("secmem usage: %u/%lu bytes in %u blocks\n",
-             cur_alloced, (unsigned long)pool_size, cur_blocks);
+             cur_alloced, (unsigned long)pool->size, cur_blocks);
   SECMEM_UNLOCK;
 #else
   memblock_t *mb;
@@ -730,9 +770,10 @@ _gcry_secmem_dump_stats ()
 
   SECMEM_LOCK;
 
-  for (i = 0, mb = (memblock_t *) pool;
-       ptr_into_pool_p (mb);
-       mb = mb_get_next (mb), i++)
+  pool = &mainpool;
+  for (i = 0, mb = (memblock_t *) pool->mem;
+       ptr_into_pool_p (pool, mb);
+       mb = mb_get_next (pool, mb), i++)
     log_info ("SECMEM: [%s] block: %i; size: %i\n",
              (mb->flags & MB_FLAG_ACTIVE) ? "used" : "free",
              i,