Skip to content

Commit 34f72ad

Browse files
gavinandresengavinandresen
committed
Merge pull request #3046 from sarchar/lockedpage-change
Changing LockedPageManager to use a managed instance
2 parents dc03c2d + 0b8f47d commit 34f72ad

File tree

4 files changed

+45
-11
lines changed

4 files changed

+45
-11
lines changed

‎src/allocators.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@
2424
#include <unistd.h> // for sysconf
2525
#endif
2626

27+
LockedPageManager* LockedPageManager::_instance = NULL;
28+
boost::once_flag LockedPageManager::init_flag = BOOST_ONCE_INIT;
29+
2730
/** Determine system page size in bytes */
2831
static inline size_t GetSystemPageSize()
2932
{

‎src/allocators.h‎

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <string.h>
99
#include <string>
1010
#include <boost/thread/mutex.hpp>
11+
#include <boost/thread/once.hpp>
1112
#include <map>
1213
#include <openssl/crypto.h> // for OPENSSL_cleanse()
1314

@@ -34,6 +35,12 @@ template <class Locker> class LockedPageManagerBase
3435
page_mask = ~(page_size - 1);
3536
}
3637

38+
~LockedPageManagerBase()
39+
{
40+
assert(this->GetLockedPageCount() == 0);
41+
}
42+
43+
3744
// For all pages in affected range, increase lock count
3845
void LockRange(void *p, size_t size)
3946
{
@@ -117,26 +124,52 @@ class MemoryPageLocker
117124
/**
118125
* Singleton class to keep track of locked (ie, non-swappable) memory pages, for use in
119126
* std::allocator templates.
127+
*
128+
* Some implementations of the STL allocate memory in some constructors (i.e., see
129+
* MSVC's vector<T> implementation where it allocates 1 byte of memory in the allocator.)
130+
* Due to the unpredictable order of static initializers, we have to make sure the
131+
* LockedPageManager instance exists before any other STL-based objects that use
132+
* secure_allocator are created. So instead of having LockedPageManager also be
133+
* static-intialized, it is created on demand.
120134
*/
121135
class LockedPageManager: public LockedPageManagerBase<MemoryPageLocker>
122136
{
123137
public:
124-
static LockedPageManager instance; // instantiated in util.cpp
138+
static LockedPageManager& Instance()
139+
{
140+
boost::call_once(LockedPageManager::CreateInstance, LockedPageManager::init_flag);
141+
return *LockedPageManager::_instance;
142+
}
143+
125144
private:
126145
LockedPageManager();
146+
147+
static void CreateInstance()
148+
{
149+
// Using a local static instance guarantees that the object is initialized
150+
// when it's first needed and also deinitialized after all objects that use
151+
// it are done with it. I can think of one unlikely scenario where we may
152+
// have a static deinitialization order/problem, but the check in
153+
// LockedPageManagerBase's destructor helps us detect if that ever happens.
154+
static LockedPageManager instance;
155+
LockedPageManager::_instance = &instance;
156+
}
157+
158+
static LockedPageManager* _instance;
159+
static boost::once_flag init_flag;
127160
};
128161

129162
//
130163
// Functions for directly locking/unlocking memory objects.
131164
// Intended for non-dynamically allocated structures.
132165
//
133166
template<typename T> void LockObject(const T &t) {
134-
LockedPageManager::instance.LockRange((void*)(&t), sizeof(T));
167+
LockedPageManager::Instance().LockRange((void*)(&t), sizeof(T));
135168
}
136169

137170
template<typename T> void UnlockObject(const T &t) {
138171
OPENSSL_cleanse((void*)(&t), sizeof(T));
139-
LockedPageManager::instance.UnlockRange((void*)(&t), sizeof(T));
172+
LockedPageManager::Instance().UnlockRange((void*)(&t), sizeof(T));
140173
}
141174

142175
//
@@ -168,7 +201,7 @@ struct secure_allocator : public std::allocator<T>
168201
T *p;
169202
p = std::allocator<T>::allocate(n, hint);
170203
if (p != NULL)
171-
LockedPageManager::instance.LockRange(p, sizeof(T) * n);
204+
LockedPageManager::Instance().LockRange(p, sizeof(T) * n);
172205
return p;
173206
}
174207

@@ -177,7 +210,7 @@ struct secure_allocator : public std::allocator<T>
177210
if (p != NULL)
178211
{
179212
OPENSSL_cleanse(p, sizeof(T) * n);
180-
LockedPageManager::instance.UnlockRange(p, sizeof(T) * n);
213+
LockedPageManager::Instance().UnlockRange(p, sizeof(T) * n);
181214
}
182215
std::allocator<T>::deallocate(p, n);
183216
}

‎src/crypter.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,16 +88,16 @@ class CCrypter
8888
// Try to keep the key data out of swap (and be a bit over-careful to keep the IV that we don't even use out of swap)
8989
// Note that this does nothing about suspend-to-disk (which will put all our key data on disk)
9090
// Note as well that at no point in this program is any attempt made to prevent stealing of keys by reading the memory of the running process.
91-
LockedPageManager::instance.LockRange(&chKey[0], sizeof chKey);
92-
LockedPageManager::instance.LockRange(&chIV[0], sizeof chIV);
91+
LockedPageManager::Instance().LockRange(&chKey[0], sizeof chKey);
92+
LockedPageManager::Instance().LockRange(&chIV[0], sizeof chIV);
9393
}
9494

9595
~CCrypter()
9696
{
9797
CleanKey();
9898

99-
LockedPageManager::instance.UnlockRange(&chKey[0], sizeof chKey);
100-
LockedPageManager::instance.UnlockRange(&chIV[0], sizeof chIV);
99+
LockedPageManager::Instance().UnlockRange(&chKey[0], sizeof chKey);
100+
LockedPageManager::Instance().UnlockRange(&chIV[0], sizeof chIV);
101101
}
102102
};
103103

‎src/util.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,6 @@ void locking_callback(int mode, int i, const char* file, int line)
9595
}
9696
}
9797

98-
LockedPageManager LockedPageManager::instance;
99-
10098
// Init
10199
class CInit
102100
{

0 commit comments

Comments
 (0)