Skip to content

Commit 11dd29b

Browse files
[net] Fix use of uninitialized value in getnetworkinfo(const JSONRPCRequest& request)
When running test_bitcoin under Valgrind I found the following issue: ``` $ valgrind src/test/test_bitcoin ... ==10465== Use of uninitialised value of size 8 ==10465== at 0x6D09B61: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21) ==10465== by 0x6D0B1BB: std::ostreambuf_iterator<char, std::char_traits<char> > std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::_M_insert_int<unsigned long>(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21) ==10465== by 0x6D0B36C: std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::do_put(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21) ==10465== by 0x6D17699: std::ostream& std::ostream::_M_insert<unsigned long>(unsigned long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21) ==10465== by 0x4CAAD7: operator<< (ostream:171) ==10465== by 0x4CAAD7: formatValue<ServiceFlags> (tinyformat.h:345) ==10465== by 0x4CAAD7: void tinyformat::detail::FormatArg::formatImpl<ServiceFlags>(std::ostream&, char const*, char const*, int, void const*) (tinyformat.h:523) ==10465== by 0x1924D4: format (tinyformat.h:510) ==10465== by 0x1924D4: tinyformat::detail::formatImpl(std::ostream&, char const*, tinyformat::detail::FormatArg const*, int) (tinyformat.h:803) ==10465== by 0x553A55: vformat (tinyformat.h:947) ==10465== by 0x553A55: format<ServiceFlags> (tinyformat.h:957) ==10465== by 0x553A55: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > tinyformat::format<ServiceFlags>(char const*, ServiceFlags const&) (tinyformat.h:966) ==10465== by 0x54C952: getnetworkinfo(JSONRPCRequest const&) (net.cpp:462) ==10465== by 0x28EDB5: CallRPC(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (rpc_tests.cpp:31) ==10465== by 0x293947: rpc_tests::rpc_togglenetwork::test_method() (rpc_tests.cpp:88) ==10465== by 0x2950E5: rpc_tests::rpc_togglenetwork_invoker() (rpc_tests.cpp:84) ==10465== by 0x182496: invoke<void (*)()> (callback.hpp:56) ==10465== by 0x182496: boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke() (callback.hpp:89) ... ``` The read of the uninitialized variable nLocalServices is triggered by g_connman->GetLocalServices() in getnetworkinfo(const JSONRPCRequest& request) (net.cpp:462): ```c++ UniValue getnetworkinfo(const JSONRPCRequest& request) { ... if(g_connman) obj.push_back(Pair("localservices", strprintf("%016x", g_connman->GetLocalServices()))); ... } ``` The reason for the uninitialized nLocalServices is that CConnman::Start(...) is not called by the tests, and hence the initialization normally performed by CConnman::Start(...) is not done. This commit adds a method Init(const Options& connOptions) which is called by both the constructor and CConnman::Start(...). This method initializes nLocalServices and the other relevant values from the supplied Options object.
1 parent 659c096 commit 11dd29b

File tree

2 files changed

+24
-26
lines changed

2 files changed

+24
-26
lines changed

src/net.cpp

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2210,12 +2210,10 @@ CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In) : nSeed0(nSeed0In), nSe
22102210
nReceiveFloodSize = 0;
22112211
semOutbound = NULL;
22122212
semAddnode = NULL;
2213-
nMaxConnections = 0;
2214-
nMaxOutbound = 0;
2215-
nMaxAddnode = 0;
2216-
nBestHeight = 0;
2217-
clientInterface = NULL;
22182213
flagInterruptMsgProc = false;
2214+
2215+
Options connOptions;
2216+
Init(connOptions);
22192217
}
22202218

22212219
NodeId CConnman::GetNewNodeId()
@@ -2254,30 +2252,15 @@ bool CConnman::InitBinds(const std::vector<CService>& binds, const std::vector<C
22542252
return fBound;
22552253
}
22562254

2257-
bool CConnman::Start(CScheduler& scheduler, Options connOptions)
2255+
bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
22582256
{
2257+
Init(connOptions);
2258+
22592259
nTotalBytesRecv = 0;
22602260
nTotalBytesSent = 0;
22612261
nMaxOutboundTotalBytesSentInCycle = 0;
22622262
nMaxOutboundCycleStartTime = 0;
22632263

2264-
nRelevantServices = connOptions.nRelevantServices;
2265-
nLocalServices = connOptions.nLocalServices;
2266-
nMaxConnections = connOptions.nMaxConnections;
2267-
nMaxOutbound = std::min((connOptions.nMaxOutbound), nMaxConnections);
2268-
nMaxAddnode = connOptions.nMaxAddnode;
2269-
nMaxFeeler = connOptions.nMaxFeeler;
2270-
2271-
nSendBufferMaxSize = connOptions.nSendBufferMaxSize;
2272-
nReceiveFloodSize = connOptions.nReceiveFloodSize;
2273-
2274-
nMaxOutboundLimit = connOptions.nMaxOutboundLimit;
2275-
nMaxOutboundTimeframe = connOptions.nMaxOutboundTimeframe;
2276-
2277-
SetBestHeight(connOptions.nBestHeight);
2278-
2279-
clientInterface = connOptions.uiInterface;
2280-
22812264
if (fListen && !InitBinds(connOptions.vBinds, connOptions.vWhiteBinds)) {
22822265
if (clientInterface) {
22832266
clientInterface->ThreadSafeMessageBox(
@@ -2287,8 +2270,6 @@ bool CConnman::Start(CScheduler& scheduler, Options connOptions)
22872270
return false;
22882271
}
22892272

2290-
vWhitelistedRange = connOptions.vWhitelistedRange;
2291-
22922273
for (const auto& strDest : connOptions.vSeedNodes) {
22932274
AddOneShot(strDest);
22942275
}

src/net.h

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,26 @@ class CConnman
146146
std::vector<CSubNet> vWhitelistedRange;
147147
std::vector<CService> vBinds, vWhiteBinds;
148148
};
149+
150+
void Init(const Options& connOptions) {
151+
nLocalServices = connOptions.nLocalServices;
152+
nRelevantServices = connOptions.nRelevantServices;
153+
nMaxConnections = connOptions.nMaxConnections;
154+
nMaxOutbound = std::min(connOptions.nMaxOutbound, connOptions.nMaxConnections);
155+
nMaxAddnode = connOptions.nMaxAddnode;
156+
nMaxFeeler = connOptions.nMaxFeeler;
157+
nBestHeight = connOptions.nBestHeight;
158+
clientInterface = connOptions.uiInterface;
159+
nSendBufferMaxSize = connOptions.nSendBufferMaxSize;
160+
nReceiveFloodSize = connOptions.nReceiveFloodSize;
161+
nMaxOutboundTimeframe = connOptions.nMaxOutboundTimeframe;
162+
nMaxOutboundLimit = connOptions.nMaxOutboundLimit;
163+
vWhitelistedRange = connOptions.vWhitelistedRange;
164+
}
165+
149166
CConnman(uint64_t seed0, uint64_t seed1);
150167
~CConnman();
151-
bool Start(CScheduler& scheduler, Options options);
168+
bool Start(CScheduler& scheduler, const Options& options);
152169
void Stop();
153170
void Interrupt();
154171
bool GetNetworkActive() const { return fNetworkActive; };

0 commit comments

Comments
 (0)