diff options
author | practicalswift <practicalswift@users.noreply.github.com> | 2017-08-02 14:02:36 +0200 |
---|---|---|
committer | practicalswift <practicalswift@users.noreply.github.com> | 2017-08-02 23:28:15 +0200 |
commit | 11dd29b658f60e247069a6adb8a0dcb882858858 (patch) | |
tree | decee9a372fb88d270ffa8473c66ec694cec5e01 | |
parent | 659c096134080034b5a5cdce4bdd8cae91632f63 (diff) |
[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.
-rw-r--r-- | src/net.cpp | 31 | ||||
-rw-r--r-- | src/net.h | 19 |
2 files changed, 24 insertions, 26 deletions
diff --git a/src/net.cpp b/src/net.cpp index d4a36a0306..ca9a173abe 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2210,12 +2210,10 @@ CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In) : nSeed0(nSeed0In), nSe nReceiveFloodSize = 0; semOutbound = NULL; semAddnode = NULL; - nMaxConnections = 0; - nMaxOutbound = 0; - nMaxAddnode = 0; - nBestHeight = 0; - clientInterface = NULL; flagInterruptMsgProc = false; + + Options connOptions; + Init(connOptions); } NodeId CConnman::GetNewNodeId() @@ -2254,30 +2252,15 @@ bool CConnman::InitBinds(const std::vector<CService>& binds, const std::vector<C return fBound; } -bool CConnman::Start(CScheduler& scheduler, Options connOptions) +bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) { + Init(connOptions); + nTotalBytesRecv = 0; nTotalBytesSent = 0; nMaxOutboundTotalBytesSentInCycle = 0; nMaxOutboundCycleStartTime = 0; - nRelevantServices = connOptions.nRelevantServices; - nLocalServices = connOptions.nLocalServices; - nMaxConnections = connOptions.nMaxConnections; - nMaxOutbound = std::min((connOptions.nMaxOutbound), nMaxConnections); - nMaxAddnode = connOptions.nMaxAddnode; - nMaxFeeler = connOptions.nMaxFeeler; - - nSendBufferMaxSize = connOptions.nSendBufferMaxSize; - nReceiveFloodSize = connOptions.nReceiveFloodSize; - - nMaxOutboundLimit = connOptions.nMaxOutboundLimit; - nMaxOutboundTimeframe = connOptions.nMaxOutboundTimeframe; - - SetBestHeight(connOptions.nBestHeight); - - clientInterface = connOptions.uiInterface; - if (fListen && !InitBinds(connOptions.vBinds, connOptions.vWhiteBinds)) { if (clientInterface) { clientInterface->ThreadSafeMessageBox( @@ -2287,8 +2270,6 @@ bool CConnman::Start(CScheduler& scheduler, Options connOptions) return false; } - vWhitelistedRange = connOptions.vWhitelistedRange; - for (const auto& strDest : connOptions.vSeedNodes) { AddOneShot(strDest); } @@ -146,9 +146,26 @@ public: std::vector<CSubNet> vWhitelistedRange; std::vector<CService> vBinds, vWhiteBinds; }; + + void Init(const Options& connOptions) { + nLocalServices = connOptions.nLocalServices; + nRelevantServices = connOptions.nRelevantServices; + nMaxConnections = connOptions.nMaxConnections; + nMaxOutbound = std::min(connOptions.nMaxOutbound, connOptions.nMaxConnections); + nMaxAddnode = connOptions.nMaxAddnode; + nMaxFeeler = connOptions.nMaxFeeler; + nBestHeight = connOptions.nBestHeight; + clientInterface = connOptions.uiInterface; + nSendBufferMaxSize = connOptions.nSendBufferMaxSize; + nReceiveFloodSize = connOptions.nReceiveFloodSize; + nMaxOutboundTimeframe = connOptions.nMaxOutboundTimeframe; + nMaxOutboundLimit = connOptions.nMaxOutboundLimit; + vWhitelistedRange = connOptions.vWhitelistedRange; + } + CConnman(uint64_t seed0, uint64_t seed1); ~CConnman(); - bool Start(CScheduler& scheduler, Options options); + bool Start(CScheduler& scheduler, const Options& options); void Stop(); void Interrupt(); bool GetNetworkActive() const { return fNetworkActive; }; |