From 5db7d2ca0aa51ff25f97bf21ce0cbc9e6b741cbd Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Tue, 5 Jul 2022 12:15:59 +0200 Subject: [PATCH 1/4] moveonly: move IsSelectableSocket() from compat.h to sock.{h,cpp} To be converted to a method of the `Sock` class. --- src/compat/compat.h | 8 -------- src/util/sock.cpp | 8 ++++++++ src/util/sock.h | 2 ++ 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/compat/compat.h b/src/compat/compat.h index a8e5552c0a..cc37797577 100644 --- a/src/compat/compat.h +++ b/src/compat/compat.h @@ -109,14 +109,6 @@ typedef char* sockopt_arg_type; #define USE_POLL #endif -bool static inline IsSelectableSocket(const SOCKET& s) { -#if defined(USE_POLL) || defined(WIN32) - return true; -#else - return (s < FD_SETSIZE); -#endif -} - // MSG_NOSIGNAL is not available on some platforms, if it doesn't exist define it as 0 #if !defined(MSG_NOSIGNAL) #define MSG_NOSIGNAL 0 diff --git a/src/util/sock.cpp b/src/util/sock.cpp index 125dbc7f18..fed50444a9 100644 --- a/src/util/sock.cpp +++ b/src/util/sock.cpp @@ -117,6 +117,14 @@ int Sock::GetSockName(sockaddr* name, socklen_t* name_len) const return getsockname(m_socket, name, name_len); } +bool IsSelectableSocket(const SOCKET& s) { +#if defined(USE_POLL) || defined(WIN32) + return true; +#else + return (s < FD_SETSIZE); +#endif +} + bool Sock::Wait(std::chrono::milliseconds timeout, Event requested, Event* occurred) const { // We need a `shared_ptr` owning `this` for `WaitMany()`, but don't want diff --git a/src/util/sock.h b/src/util/sock.h index 38a7dc80d6..9ec53ec91d 100644 --- a/src/util/sock.h +++ b/src/util/sock.h @@ -267,6 +267,8 @@ private: void Close(); }; +bool IsSelectableSocket(const SOCKET& s); + /** Return readable error string for a network error code */ std::string NetworkErrorString(int err); From b4bac556791b5bb8aa118d4c1fed42c3fe45550c Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Tue, 13 Apr 2021 14:29:14 +0200 Subject: [PATCH 2/4] net: convert standalone IsSelectableSocket() to Sock::IsSelectable() This makes the callers mockable. --- src/net.cpp | 3 +-- src/netbase.cpp | 2 +- src/test/fuzz/util.cpp | 7 ++++++- src/test/fuzz/util.h | 9 +++++++++ src/test/util/net.h | 2 ++ src/util/sock.cpp | 9 +++++---- src/util/sock.h | 8 ++++++-- 7 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index e9aa7ee43b..85bd0b529e 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -961,8 +961,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, return; } - if (!IsSelectableSocket(sock->Get())) - { + if (!sock->IsSelectable()) { LogPrintf("connection from %s dropped: non-selectable socket\n", addr.ToString()); return; } diff --git a/src/netbase.cpp b/src/netbase.cpp index 4b8d2f8d0c..22cf4415c8 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -503,7 +503,7 @@ std::unique_ptr CreateSockTCP(const CService& address_family) // Ensure that waiting for I/O on this socket won't result in undefined // behavior. - if (!IsSelectableSocket(sock->Get())) { + if (!sock->IsSelectable()) { LogPrintf("Cannot create connection: non-selectable socket created (fd >= FD_SETSIZE ?)\n"); return nullptr; } diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index fabcea22c3..1d19856671 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -16,7 +16,7 @@ #include FuzzedSock::FuzzedSock(FuzzedDataProvider& fuzzed_data_provider) - : m_fuzzed_data_provider{fuzzed_data_provider} + : m_fuzzed_data_provider{fuzzed_data_provider}, m_selectable{fuzzed_data_provider.ConsumeBool()} { m_socket = fuzzed_data_provider.ConsumeIntegralInRange(INVALID_SOCKET - 1, INVALID_SOCKET); } @@ -254,6 +254,11 @@ int FuzzedSock::GetSockName(sockaddr* name, socklen_t* name_len) const return 0; } +bool FuzzedSock::IsSelectable() const +{ + return m_selectable; +} + bool FuzzedSock::Wait(std::chrono::milliseconds timeout, Event requested, Event* occurred) const { constexpr std::array wait_errnos{ diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 60e2875953..92ad08ec59 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -48,6 +48,13 @@ class FuzzedSock : public Sock */ mutable std::optional m_peek_data; + /** + * Whether to pretend that the socket is select(2)-able. This is randomly set in the + * constructor. It should remain constant so that repeated calls to `IsSelectable()` + * return the same value. + */ + const bool m_selectable; + public: explicit FuzzedSock(FuzzedDataProvider& fuzzed_data_provider); @@ -73,6 +80,8 @@ public: int GetSockName(sockaddr* name, socklen_t* name_len) const override; + bool IsSelectable() const override; + bool Wait(std::chrono::milliseconds timeout, Event requested, Event* occurred = nullptr) const override; bool WaitMany(std::chrono::milliseconds timeout, EventsPerSock& events_per_sock) const override; diff --git a/src/test/util/net.h b/src/test/util/net.h index ec6b4e6e88..749b5b3364 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -166,6 +166,8 @@ public: return 0; } + bool IsSelectable() const override { return true; } + bool Wait(std::chrono::milliseconds timeout, Event requested, Event* occurred = nullptr) const override diff --git a/src/util/sock.cpp b/src/util/sock.cpp index fed50444a9..c312a88276 100644 --- a/src/util/sock.cpp +++ b/src/util/sock.cpp @@ -117,11 +117,12 @@ int Sock::GetSockName(sockaddr* name, socklen_t* name_len) const return getsockname(m_socket, name, name_len); } -bool IsSelectableSocket(const SOCKET& s) { +bool Sock::IsSelectable() const +{ #if defined(USE_POLL) || defined(WIN32) return true; #else - return (s < FD_SETSIZE); + return m_socket < FD_SETSIZE; #endif } @@ -193,10 +194,10 @@ bool Sock::WaitMany(std::chrono::milliseconds timeout, EventsPerSock& events_per SOCKET socket_max{0}; for (const auto& [sock, events] : events_per_sock) { - const auto& s = sock->m_socket; - if (!IsSelectableSocket(s)) { + if (!sock->IsSelectable()) { return false; } + const auto& s = sock->m_socket; if (events.requested & RECV) { FD_SET(s, &recv); } diff --git a/src/util/sock.h b/src/util/sock.h index 9ec53ec91d..ab9c6737c6 100644 --- a/src/util/sock.h +++ b/src/util/sock.h @@ -133,6 +133,12 @@ public: */ [[nodiscard]] virtual int GetSockName(sockaddr* name, socklen_t* name_len) const; + /** + * Check if the underlying socket can be used for `select(2)` (or the `Wait()` method). + * @return true if selectable + */ + [[nodiscard]] virtual bool IsSelectable() const; + using Event = uint8_t; /** @@ -267,8 +273,6 @@ private: void Close(); }; -bool IsSelectableSocket(const SOCKET& s); - /** Return readable error string for a network error code */ std::string NetworkErrorString(int err); From 29f66f76826056f53d971ac734b7ed49b39848d3 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Tue, 5 Jul 2022 12:20:42 +0200 Subject: [PATCH 3/4] moveonly: move SetSocketNonBlocking() from netbase to util/sock To be converted to a method of the `Sock` class. --- src/netbase.cpp | 15 --------------- src/netbase.h | 2 -- src/util/sock.cpp | 15 +++++++++++++++ src/util/sock.h | 3 +++ 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/netbase.cpp b/src/netbase.cpp index 22cf4415c8..b13d6b1ae0 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -717,21 +717,6 @@ bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out) return false; } -bool SetSocketNonBlocking(const SOCKET& hSocket) -{ -#ifdef WIN32 - u_long nOne = 1; - if (ioctlsocket(hSocket, FIONBIO, &nOne) == SOCKET_ERROR) { -#else - int fFlags = fcntl(hSocket, F_GETFL, 0); - if (fcntl(hSocket, F_SETFL, fFlags | O_NONBLOCK) == SOCKET_ERROR) { -#endif - return false; - } - - return true; -} - void InterruptSocks5(bool interrupt) { interruptSocks5Recv = interrupt; diff --git a/src/netbase.h b/src/netbase.h index fadc8b418e..f7816f5d1d 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -221,8 +221,6 @@ bool ConnectSocketDirectly(const CService &addrConnect, const Sock& sock, int nT */ bool ConnectThroughProxy(const Proxy& proxy, const std::string& strDest, uint16_t port, const Sock& sock, int nTimeout, bool& outProxyConnectionFailed); -/** Enable non-blocking mode for a socket */ -bool SetSocketNonBlocking(const SOCKET& hSocket); void InterruptSocks5(bool interrupt); /** diff --git a/src/util/sock.cpp b/src/util/sock.cpp index c312a88276..b2b5133629 100644 --- a/src/util/sock.cpp +++ b/src/util/sock.cpp @@ -117,6 +117,21 @@ int Sock::GetSockName(sockaddr* name, socklen_t* name_len) const return getsockname(m_socket, name, name_len); } +bool SetSocketNonBlocking(const SOCKET& hSocket) +{ +#ifdef WIN32 + u_long nOne = 1; + if (ioctlsocket(hSocket, FIONBIO, &nOne) == SOCKET_ERROR) { +#else + int fFlags = fcntl(hSocket, F_GETFL, 0); + if (fcntl(hSocket, F_SETFL, fFlags | O_NONBLOCK) == SOCKET_ERROR) { +#endif + return false; + } + + return true; +} + bool Sock::IsSelectable() const { #if defined(USE_POLL) || defined(WIN32) diff --git a/src/util/sock.h b/src/util/sock.h index ab9c6737c6..b97977da95 100644 --- a/src/util/sock.h +++ b/src/util/sock.h @@ -273,6 +273,9 @@ private: void Close(); }; +/** Enable non-blocking mode for a socket */ +bool SetSocketNonBlocking(const SOCKET& hSocket); + /** Return readable error string for a network error code */ std::string NetworkErrorString(int err); From b527b549504672704a61f70d2565b9489aaaba91 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Mon, 19 Apr 2021 11:35:09 +0200 Subject: [PATCH 4/4] net: convert standalone SetSocketNonBlocking() to Sock::SetNonBlocking() This further encapsulates syscalls inside the `Sock` class. Co-authored-by: practicalswift --- src/netbase.cpp | 5 ++--- src/test/fuzz/util.cpp | 13 +++++++++++++ src/test/fuzz/util.h | 2 ++ src/test/util/net.h | 2 ++ src/util/sock.cpp | 20 ++++++++++++-------- src/util/sock.h | 9 ++++++--- 6 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/netbase.cpp b/src/netbase.cpp index b13d6b1ae0..d504107d91 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -304,8 +304,7 @@ enum class IntrRecvError { * read. * * @see This function can be interrupted by calling InterruptSocks5(bool). - * Sockets can be made non-blocking with SetSocketNonBlocking(const - * SOCKET&). + * Sockets can be made non-blocking with Sock::SetNonBlocking(). */ static IntrRecvError InterruptibleRecv(uint8_t* data, size_t len, int timeout, const Sock& sock) { @@ -525,7 +524,7 @@ std::unique_ptr CreateSockTCP(const CService& address_family) } // Set the non-blocking option on the socket. - if (!SetSocketNonBlocking(sock->Get())) { + if (!sock->SetNonBlocking()) { LogPrintf("Error setting socket to non-blocking: %s\n", NetworkErrorString(WSAGetLastError())); return nullptr; } diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index 1d19856671..0faece5854 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -254,6 +254,19 @@ int FuzzedSock::GetSockName(sockaddr* name, socklen_t* name_len) const return 0; } +bool FuzzedSock::SetNonBlocking() const +{ + constexpr std::array setnonblocking_errnos{ + EBADF, + EPERM, + }; + if (m_fuzzed_data_provider.ConsumeBool()) { + SetFuzzedErrNo(m_fuzzed_data_provider, setnonblocking_errnos); + return false; + } + return true; +} + bool FuzzedSock::IsSelectable() const { return m_selectable; diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 92ad08ec59..c5ae78da67 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -80,6 +80,8 @@ public: int GetSockName(sockaddr* name, socklen_t* name_len) const override; + bool SetNonBlocking() const override; + bool IsSelectable() const override; bool Wait(std::chrono::milliseconds timeout, Event requested, Event* occurred = nullptr) const override; diff --git a/src/test/util/net.h b/src/test/util/net.h index 749b5b3364..814a4ff9c8 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -166,6 +166,8 @@ public: return 0; } + bool SetNonBlocking() const override { return true; } + bool IsSelectable() const override { return true; } bool Wait(std::chrono::milliseconds timeout, diff --git a/src/util/sock.cpp b/src/util/sock.cpp index b2b5133629..84ac2759fa 100644 --- a/src/util/sock.cpp +++ b/src/util/sock.cpp @@ -117,18 +117,22 @@ int Sock::GetSockName(sockaddr* name, socklen_t* name_len) const return getsockname(m_socket, name, name_len); } -bool SetSocketNonBlocking(const SOCKET& hSocket) +bool Sock::SetNonBlocking() const { #ifdef WIN32 - u_long nOne = 1; - if (ioctlsocket(hSocket, FIONBIO, &nOne) == SOCKET_ERROR) { -#else - int fFlags = fcntl(hSocket, F_GETFL, 0); - if (fcntl(hSocket, F_SETFL, fFlags | O_NONBLOCK) == SOCKET_ERROR) { -#endif + u_long on{1}; + if (ioctlsocket(m_socket, FIONBIO, &on) == SOCKET_ERROR) { return false; } - +#else + const int flags{fcntl(m_socket, F_GETFL, 0)}; + if (flags == SOCKET_ERROR) { + return false; + } + if (fcntl(m_socket, F_SETFL, flags | O_NONBLOCK) == SOCKET_ERROR) { + return false; + } +#endif return true; } diff --git a/src/util/sock.h b/src/util/sock.h index b97977da95..7912284904 100644 --- a/src/util/sock.h +++ b/src/util/sock.h @@ -133,6 +133,12 @@ public: */ [[nodiscard]] virtual int GetSockName(sockaddr* name, socklen_t* name_len) const; + /** + * Set the non-blocking option on the socket. + * @return true if set successfully + */ + [[nodiscard]] virtual bool SetNonBlocking() const; + /** * Check if the underlying socket can be used for `select(2)` (or the `Wait()` method). * @return true if selectable @@ -273,9 +279,6 @@ private: void Close(); }; -/** Enable non-blocking mode for a socket */ -bool SetSocketNonBlocking(const SOCKET& hSocket); - /** Return readable error string for a network error code */ std::string NetworkErrorString(int err);