Prevent a crash in GMP_Engine if the library is shutdown and then

reinitialized. It would cache an Allocator pointer on first use,
and then never zero it, so after the reinit the pointer would be going
to a now deallocated Allocator object.

Encountered in the SoftHSM test suite, reported by Ondrej Sury.

Use a simple reference counting scheme to zero the pointer, and reset
the GNU MP memory functions. This also fixes a quite obscure and never
reported bug, that if the GMP engine was used, and if the library was
deinitialized but then the program tried to use GNU MP, the allocator
functions would crash. Now after deinit the allocator funcs revert to the
defaults.

The reference count is not updated atomically so this is not thread safe,
but seems a non-issue; the only time this could happen (especially now that
the GMP engine header is internal-only) is if multiple threads were attempting
to initialize / shutdown the library at once - which won't work anyway for
a variety of reasons, including contention on the (unlocked) global_lib_state
pointer.

If at some point thread safety is useful here, the refcnt can be locked by
a mutex, or kept in an atomic<unsigned int>.

3 files changed, 18 insertions, 12 deletions

diff --git a/doc/log.txt b/doc/log.txt
index 3db4205f3..ef16d85bf 100644
--- a/doc/log.txt
+++ b/doc/log.txt
@@ -7,6 +7,7 @@
  - Add SQLite3 db encryption codec, contributed by Olivier de Gaalon
  - Add a block cipher cascade construction
  - Add support for Win32 high resolution system timers
+ - Fix crash in GMP_Engine if library is shutdown and reinitialized
  - Remove Timer class entirely
  - Switch default PKCS #8 encryption algorithm from 3DES to AES-256
  - New option --gen-amalgamation for creating a SQLite-style amalgamation
diff --git a/src/engine/gnump/gmp_mem.cpp b/src/engine/gnump/gmp_mem.cpp
index 59e0cc4c5..f3650e716 100644
--- a/src/engine/gnump/gmp_mem.cpp
+++ b/src/engine/gnump/gmp_mem.cpp
@@ -1,6 +1,6 @@
 /*
 * GNU MP Memory Handlers
-* (C) 1999-2007 Jack Lloyd
+* (C) 1999-2010 Jack Lloyd
 *
 * Distributed under the terms of the Botan license
 */
@@ -17,6 +17,7 @@ namespace {
 * Allocator used by GNU MP
 */
 Allocator* gmp_alloc = 0;
+u32bit gmp_alloc_refcnt = 0;
 
 /*
 * Allocation Function for GNU MP
@@ -48,23 +49,28 @@ void gmp_free(void* ptr, size_t n)
 }
 
 /*
-* Set the GNU MP memory functions
+* GMP_Engine Constructor
 */
-void GMP_Engine::set_memory_hooks()
+GMP_Engine::GMP_Engine()
    {
    if(gmp_alloc == 0)
       {
       gmp_alloc = Allocator::get(true);
       mp_set_memory_functions(gmp_malloc, gmp_realloc, gmp_free);
       }
+
+   ++gmp_alloc_refcnt;
    }
 
-/*
-* GMP_Engine Constructor
-*/
-GMP_Engine::GMP_Engine()
+GMP_Engine::~GMP_Engine()
    {
-   set_memory_hooks();
+   --gmp_alloc_refcnt;
+
+   if(gmp_alloc_refcnt == 0)
+      {
+      mp_set_memory_functions(NULL, NULL, NULL);
+      gmp_alloc = 0;
+      }
    }
 
 }
diff --git a/src/engine/gnump/gnump_engine.h b/src/engine/gnump/gnump_engine.h
index ec4a7e721..d0b070441 100644
--- a/src/engine/gnump/gnump_engine.h
+++ b/src/engine/gnump/gnump_engine.h
@@ -18,6 +18,9 @@ namespace Botan {
 class GMP_Engine : public Engine
    {
    public:
+      GMP_Engine();
+      ~GMP_Engine();
+
       std::string provider_name() const { return "gmp"; }
 
 #if defined(BOTAN_HAS_IF_PUBLIC_KEY_FAMILY)
@@ -46,10 +49,6 @@ class GMP_Engine : public Engine
 
       Modular_Exponentiator* mod_exp(const BigInt&,
                                      Power_Mod::Usage_Hints) const;
-
-      GMP_Engine();
-   private:
-      static void set_memory_hooks();
    };
 
 }