From dfd14bf5735fdd59bad45cc3414b9cdb825c8f4e Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Sat, 5 Aug 2017 13:23:10 +0200 Subject: [PATCH] Merge #10977: [net] Fix use of uninitialized value in getnetworkinfo(const JSONRPCRequest&) 11dd29b [net] Fix use of uninitialized value in getnetworkinfo(const JSONRPCRequest& request) (practicalswift) Pull request description: 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 > std::num_put > >::_M_insert_int(std::ostreambuf_iterator >, 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 > >::do_put(std::ostreambuf_iterator >, 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) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21) ==10465== by 0x4CAAD7: operator<< (ostream:171) ==10465== by 0x4CAAD7: formatValue (tinyformat.h:345) ==10465== by 0x4CAAD7: void tinyformat::detail::FormatArg::formatImpl(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 (tinyformat.h:957) ==10465== by 0x553A55: std::__cxx11::basic_string, std::allocator > tinyformat::format(char const*, ServiceFlags const&) (tinyformat.h:966) ==10465== by 0x54C952: getnetworkinfo(JSONRPCRequest const&) (net.cpp:462) ==10465== by 0x28EDB5: CallRPC(std::__cxx11::basic_string, std::allocator >) (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 (callback.hpp:56) ==10465== by 0x182496: boost::unit_test::ut_detail::callback0_impl_t::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. Tree-SHA512: d8742363acffd03b2ee081cc56840275569e17edc6fa4bb1dee4a5971ffe4b8ab1d2fe7b68f98a086bf133b7ec46f4e471243ca08b45bf82356e8c831a5a5f21 --- src/net.cpp | 31 ++++++------------------------- src/net.h | 19 ++++++++++++++++++- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 722f016fd5..b756f9387e 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2416,12 +2416,10 @@ CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In) : semOutbound = NULL; semAddnode = NULL; semMasternodeOutbound = NULL; - nMaxConnections = 0; - nMaxOutbound = 0; - nMaxAddnode = 0; - nBestHeight = 0; - clientInterface = NULL; flagInterruptMsgProc = false; + + Options connOptions; + Init(connOptions); } NodeId CConnman::GetNewNodeId() @@ -2460,30 +2458,15 @@ bool CConnman::InitBinds(const std::vector& binds, const std::vectorThreadSafeMessageBox( @@ -2493,8 +2476,6 @@ bool CConnman::Start(CScheduler& scheduler, Options connOptions) return false; } - vWhitelistedRange = connOptions.vWhitelistedRange; - for (const auto& strDest : connOptions.vSeedNodes) { AddOneShot(strDest); } diff --git a/src/net.h b/src/net.h index 2f7a545b80..3e5c181335 100644 --- a/src/net.h +++ b/src/net.h @@ -170,9 +170,26 @@ public: std::vector vWhitelistedRange; std::vector 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; };