Skip to content

Commit

Permalink
Merge bitcoin#14268: Introduce SafeDbt to handle Dbt with free or mem…
Browse files Browse the repository at this point in the history 
…ory_cleanse raii-style

4a86a0a Make SafeDbt DB_DBT_MALLOC on default initialization (Ben Woosley)
1a9f9f7 Introduce SafeDbt to handle DB_DBT_MALLOC raii-style (Ben Woosley)
951a44e Drop unused setRange arg to BerkeleyBatch::ReadAtCursor (Ben Woosley)

Pull request description:

  This provides additional exception-safety and case handling for the proper
  freeing of the associated buffers.

Tree-SHA512: a038d728290cdb3905e7d881608052a6675b6425729ceaf7cfe69a6e91c2ee293cdb01e4b695a20963459ffdd9d4a1f9a08b3c07b1b5ba1aa8590a8149f686db
  • Loading branch information
@laanwj @dzutto
laanwj authored and dzutto committed Sep 30, 2021
1 parent 6e91a22 commit 0d1db5c
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 47 deletions.
39 changes: 39 additions & 0 deletions src/wallet/db.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,45 @@ BerkeleyEnvironment::VerifyResult BerkeleyEnvironment::Verify(const std::string&
return (fRecovered ? VerifyResult::RECOVER_OK : VerifyResult::RECOVER_FAIL);
}

BerkeleyBatch::SafeDbt::SafeDbt()
{
m_dbt.set_flags(DB_DBT_MALLOC);
}

BerkeleyBatch::SafeDbt::SafeDbt(void* data, size_t size)
: m_dbt(data, size)
{
}

BerkeleyBatch::SafeDbt::~SafeDbt()
{
if (m_dbt.get_data() != nullptr) {
// Clear memory, e.g. in case it was a private key
memory_cleanse(m_dbt.get_data(), m_dbt.get_size());
// under DB_DBT_MALLOC, data is malloced by the Dbt, but must be
// freed by the caller.
// https://docs.oracle.com/cd/E17275_01/html/api_reference/C/dbt.html
if (m_dbt.get_flags() & DB_DBT_MALLOC) {
free(m_dbt.get_data());
}
}
}

const void* BerkeleyBatch::SafeDbt::get_data() const
{
return m_dbt.get_data();
}

u_int32_t BerkeleyBatch::SafeDbt::get_size() const
{
return m_dbt.get_size();
}

BerkeleyBatch::SafeDbt::operator Dbt*()
{
return &m_dbt;
}

bool BerkeleyBatch::Recover(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& newFilename)
{
std::string filename;
Expand Down
81 changes: 34 additions & 47 deletions src/wallet/db.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,29 @@ class BerkeleyDatabase
bool IsDummy() { return env == nullptr; }
};


/** RAII class that provides access to a Berkeley database */
class BerkeleyBatch
{
/** RAII class that automatically cleanses its data on destruction */
class SafeDbt final
{
Dbt m_dbt;

public:
// construct Dbt with internally-managed data
SafeDbt();
// construct Dbt with provided data
SafeDbt(void* data, size_t size);
~SafeDbt();

// delegate to Dbt
const void* get_data() const;
u_int32_t get_size() const;

// conversion operator to access the underlying Dbt
operator Dbt*();
};

protected:
Db* pdb;
std::string strFile;
Expand Down Expand Up @@ -236,7 +255,6 @@ class BerkeleyBatch
/* verifies the database file */
static bool VerifyDatabaseFile(const fs::path& file_path, std::string& warningStr, std::string& errorStr, BerkeleyEnvironment::recoverFunc_type recoverFunc);

public:
template <typename K, typename T>
bool Read(const K& key, T& value)
{
Expand All @@ -247,13 +265,11 @@ class BerkeleyBatch
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(1000);
ssKey << key;
Dbt datKey(ssKey.data(), ssKey.size());
SafeDbt datKey(ssKey.data(), ssKey.size());

// Read
Dbt datValue;
datValue.set_flags(DB_DBT_MALLOC);
int ret = pdb->get(activeTxn, &datKey, &datValue, 0);
memory_cleanse(datKey.get_data(), datKey.get_size());
SafeDbt datValue;
int ret = pdb->get(activeTxn, datKey, datValue, 0);
bool success = false;
if (datValue.get_data() != nullptr) {
// Unserialize value
Expand All @@ -264,10 +280,6 @@ class BerkeleyBatch
} catch (const std::exception&) {
// In this case success remains 'false'
}

// Clear and free memory
memory_cleanse(datValue.get_data(), datValue.get_size());
free(datValue.get_data());
}
return ret == 0 && success;
}
Expand All @@ -284,20 +296,16 @@ class BerkeleyBatch
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(1000);
ssKey << key;
Dbt datKey(ssKey.data(), ssKey.size());
SafeDbt datKey(ssKey.data(), ssKey.size());

// Value
CDataStream ssValue(SER_DISK, CLIENT_VERSION);
ssValue.reserve(10000);
ssValue << value;
Dbt datValue(ssValue.data(), ssValue.size());
SafeDbt datValue(ssValue.data(), ssValue.size());

// Write
int ret = pdb->put(activeTxn, &datKey, &datValue, (fOverwrite ? 0 : DB_NOOVERWRITE));

// Clear memory in case it was a private key
memory_cleanse(datKey.get_data(), datKey.get_size());
memory_cleanse(datValue.get_data(), datValue.get_size());
int ret = pdb->put(activeTxn, datKey, datValue, (fOverwrite ? 0 : DB_NOOVERWRITE));
return (ret == 0);
}

Expand All @@ -313,13 +321,10 @@ class BerkeleyBatch
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(1000);
ssKey << key;
Dbt datKey(ssKey.data(), ssKey.size());
SafeDbt datKey(ssKey.data(), ssKey.size());

// Erase
int ret = pdb->del(activeTxn, &datKey, 0);

// Clear memory
memory_cleanse(datKey.get_data(), datKey.get_size());
int ret = pdb->del(activeTxn, datKey, 0);
return (ret == 0 || ret == DB_NOTFOUND);
}

Expand All @@ -333,13 +338,10 @@ class BerkeleyBatch
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(1000);
ssKey << key;
Dbt datKey(ssKey.data(), ssKey.size());
SafeDbt datKey(ssKey.data(), ssKey.size());

// Exists
int ret = pdb->exists(activeTxn, &datKey, 0);

// Clear memory
memory_cleanse(datKey.get_data(), datKey.get_size());
int ret = pdb->exists(activeTxn, datKey, 0);
return (ret == 0);
}

Expand All @@ -354,20 +356,12 @@ class BerkeleyBatch
return pcursor;
}

int ReadAtCursor(Dbc* pcursor, CDataStream& ssKey, CDataStream& ssValue, bool setRange = false)
int ReadAtCursor(Dbc* pcursor, CDataStream& ssKey, CDataStream& ssValue)
{
// Read at cursor
Dbt datKey;
unsigned int fFlags = DB_NEXT;
if (setRange) {
datKey.set_data(ssKey.data());
datKey.set_size(ssKey.size());
fFlags = DB_SET_RANGE;
}
Dbt datValue;
datKey.set_flags(DB_DBT_MALLOC);
datValue.set_flags(DB_DBT_MALLOC);
int ret = pcursor->get(&datKey, &datValue, fFlags);
SafeDbt datKey;
SafeDbt datValue;
int ret = pcursor->get(datKey, datValue, DB_NEXT);
if (ret != 0)
return ret;
else if (datKey.get_data() == nullptr || datValue.get_data() == nullptr)
Expand All @@ -380,16 +374,9 @@ class BerkeleyBatch
ssValue.SetType(SER_DISK);
ssValue.clear();
ssValue.write((char*)datValue.get_data(), datValue.get_size());

// Clear and free memory
memory_cleanse(datKey.get_data(), datKey.get_size());
memory_cleanse(datValue.get_data(), datValue.get_size());
free(datKey.get_data());
free(datValue.get_data());
return 0;
}

public:
bool TxnBegin()
{
if (!pdb || activeTxn)
Expand Down

0 comments on commit 0d1db5c

Please sign in to comment.