sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-25 12:02:48 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge #18487: rpc: Fix rpcRunLater race in walletpassphrase

Browse Source 
7b8e15728d1ad058a4b7d7569fd5d5ba6806ca28 rpc: Fix rpcRunLater race in walletpassphrase (João Barbosa)

Pull request description:

  Release locks before calling `rpcRunLater`.

  Quick explanation: `rpcRunLater` leads to `event_free` which calls `event_del` which can wait for the event callback to finish if it's already running and that callback will try to lock wallet mutex - which is already locked in http thread.

  Fixes #14995 , fixes #18482. Best reviewed with whitespace changes hidden.

ACKs for top commit:
  MarcoFalke:
    ACK 7b8e15728d, only tested that this avoids the node freezing. Did not look at how libevent works or how the deadlock happens or if this breaks other stuff. 📞
  ryanofsky:
    Code review ACK 7b8e15728d1ad058a4b7d7569fd5d5ba6806ca28. Just updated comment since last review

Tree-SHA512: 17874a2fa7b0e164fb0d7ee4cb7d59650275b8c03476fb291d60af8b758495457660d3912623fb26259fefe84aeba21c0a9e0c6467982ba511f19344ed5413ab
This commit is contained in:
Wladimir J. van der Laan 2020-04-06 20:29:16 +02:00 committed by PastaPastaPasta
parent 76242f9d8d
commit 363b37dfbd
1 changed files with 54 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

100
src/wallet/rpcwallet.cpp
View File

@ -1934,54 +1934,62 @@ static UniValue walletpassphrase(const JSONRPCRequest& request)
if (!wallet) return NullUniValue;
CWallet* const pwallet = wallet.get();
LOCK(pwallet->cs_wallet);
int64_t nSleepTime;
{
LOCK(pwallet->cs_wallet);
if (!pwallet->IsCrypted()) {
throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrase was called.");
}
// Note that the walletpassphrase is stored in request.params[0] which is not mlock()ed
SecureString strWalletPass;
strWalletPass.reserve(100);
// TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string)
// Alternately, find a way to make request.params[0] mlock()'d to begin with.
strWalletPass = request.params[0].get_str().c_str();
// Get the timeout
nSleepTime = request.params[1].get_int64();
// Timeout cannot be negative, otherwise it will relock immediately
if (nSleepTime < 0) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Timeout cannot be negative.");
}
// Clamp timeout
constexpr int64_t MAX_SLEEP_TIME = 100000000; // larger values trigger a macos/libevent bug?
if (nSleepTime > MAX_SLEEP_TIME) {
nSleepTime = MAX_SLEEP_TIME;
}
bool fForMixingOnly = false;
if (!request.params[2].isNull())
fForMixingOnly = request.params[2].get_bool();
if (fForMixingOnly && !pwallet->IsLocked()) {
// Downgrading from "fuly unlocked" mode to "mixing only" one is not supported.
// Updating unlock time when current unlock mode is not changed or when it is upgraded
// from "mixing only" to "fuly unlocked" is ok.
throw JSONRPCError(RPC_WALLET_ALREADY_UNLOCKED, "Error: Wallet is already fully unlocked.");
}
if (strWalletPass.empty()) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase can not be empty");
}
if (!pwallet->Unlock(strWalletPass, fForMixingOnly)) {
throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect.");
}
pwallet->TopUpKeyPool();
pwallet->nRelockTime = GetTime() + nSleepTime;
if (!pwallet->IsCrypted()) {
throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrase was called.");
}
// Note that the walletpassphrase is stored in request.params[0] which is not mlock()ed
SecureString strWalletPass;
strWalletPass.reserve(100);
// TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string)
// Alternately, find a way to make request.params[0] mlock()'d to begin with.
strWalletPass = request.params[0].get_str().c_str();
// Get the timeout
int64_t nSleepTime = request.params[1].get_int64();
// Timeout cannot be negative, otherwise it will relock immediately
if (nSleepTime < 0) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Timeout cannot be negative.");
}
// Clamp timeout
constexpr int64_t MAX_SLEEP_TIME = 100000000; // larger values trigger a macos/libevent bug?
if (nSleepTime > MAX_SLEEP_TIME) {
nSleepTime = MAX_SLEEP_TIME;
}
bool fForMixingOnly = false;
if (!request.params[2].isNull())
fForMixingOnly = request.params[2].get_bool();
if (fForMixingOnly && !pwallet->IsLocked()) {
// Downgrading from "fuly unlocked" mode to "mixing only" one is not supported.
// Updating unlock time when current unlock mode is not changed or when it is upgraded
// from "mixing only" to "fuly unlocked" is ok.
throw JSONRPCError(RPC_WALLET_ALREADY_UNLOCKED, "Error: Wallet is already fully unlocked.");
}
if (strWalletPass.empty()) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase can not be empty");
}
if (!pwallet->Unlock(strWalletPass, fForMixingOnly)) {
throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect.");
}
pwallet->TopUpKeyPool();
pwallet->nRelockTime = GetTime() + nSleepTime;
// rpcRunLater must be called without cs_wallet held otherwise a deadlock
// can occur. The deadlock would happen when RPCRunLater removes the
// previous timer (and waits for the callback to finish if already running)
// and the callback locks cs_wallet.
AssertLockNotHeld(wallet->cs_wallet);
// Keep a weak pointer to the wallet so that it is possible to unload the
// wallet before the following callback is called. If a valid shared pointer
// is acquired in the callback then the wallet is still loaded.