From fd2e8957e14437a1ec547bf3323ec92e05b9a9b3 Mon Sep 17 00:00:00 2001 From: Alexey Simakov Date: Mon, 12 Feb 2024 22:08:12 +0300 Subject: [PATCH 01/14] Fix memory leak at FreeAddr --- src/ip/Address.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ip/Address.cc b/src/ip/Address.cc index 62a07cfb9c2..e6d1d9f0f20 100644 --- a/src/ip/Address.cc +++ b/src/ip/Address.cc @@ -697,7 +697,7 @@ Ip::Address::FreeAddr(struct addrinfo *&ai) { if (ai == nullptr) return; - if (ai->ai_addr) delete ai->ai_addr; + if (ai->ai_addr) delete (struct sockaddr_in6*)ai->ai_addr; ai->ai_addr = nullptr; From 6ffe15818c6f5e144017b7bb1f6f5e68b6921c75 Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Wed, 14 Feb 2024 03:37:45 +1300 Subject: [PATCH 02/14] Update Address.cc code polish --- src/ip/Address.cc | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/ip/Address.cc b/src/ip/Address.cc index e6d1d9f0f20..693765f7fc8 100644 --- a/src/ip/Address.cc +++ b/src/ip/Address.cc @@ -683,9 +683,10 @@ Ip::Address::InitAddr(struct addrinfo *&ai) } // remove any existing data. - if (ai->ai_addr) delete ai->ai_addr; + if (ai->ai_addr) + delete ai->ai_addr; - ai->ai_addr = (struct sockaddr*)new sockaddr_in6; + ai->ai_addr = static_cast(new sockaddr_in6); memset(ai->ai_addr, 0, sizeof(struct sockaddr_in6)); ai->ai_addrlen = sizeof(struct sockaddr_in6); @@ -695,9 +696,11 @@ Ip::Address::InitAddr(struct addrinfo *&ai) void Ip::Address::FreeAddr(struct addrinfo *&ai) { - if (ai == nullptr) return; + if (!ai) + return; - if (ai->ai_addr) delete (struct sockaddr_in6*)ai->ai_addr; + if (ai->ai_addr) + delete static_cast(struct sockaddr_in6*>(ai->ai_addr); ai->ai_addr = nullptr; From ea5d9eeb472a67b57a7c2348afafd0bf91ff7472 Mon Sep 17 00:00:00 2001 From: Alexey Simakov Date: Thu, 15 Feb 2024 20:43:22 +0300 Subject: [PATCH 03/14] Encapsulate ai_addr construction/destruction --- src/ip/Address.cc | 83 +++++++++++++++++++++++++++++------------------ src/ip/Address.h | 8 +++++ 2 files changed, 60 insertions(+), 31 deletions(-) diff --git a/src/ip/Address.cc b/src/ip/Address.cc index e6d1d9f0f20..3e5c0fc851b 100644 --- a/src/ip/Address.cc +++ b/src/ip/Address.cc @@ -9,6 +9,8 @@ /* DEBUG: section 14 IP Storage and Handling */ #include "squid.h" + +#include "base/TextException.h" #include "debug/Stream.h" #include "ip/Address.h" #include "ip/tools.h" @@ -634,16 +636,9 @@ Ip::Address::getAddrInfo(struct addrinfo *&dst, int force) const dst->ai_protocol = IPPROTO_UDP; if (force == AF_INET6 || (force == AF_UNSPEC && isIPv6()) ) { - dst->ai_addr = (struct sockaddr*)new sockaddr_in6; - - memset(dst->ai_addr,0,sizeof(struct sockaddr_in6)); - - getSockAddr(*((struct sockaddr_in6*)dst->ai_addr)); - - dst->ai_addrlen = sizeof(struct sockaddr_in6); - - dst->ai_family = ((struct sockaddr_in6*)dst->ai_addr)->sin6_family; - + initSockAddr(dst, SockAddrType::SockAddrIn6); + getSockAddr(*(reinterpret_cast(dst->ai_addr))); + dst->ai_family = (reinterpret_cast(dst->ai_addr))->sin6_family; #if 0 /** * Enable only if you must and please report to squid-dev if you find a need for this. @@ -659,16 +654,9 @@ Ip::Address::getAddrInfo(struct addrinfo *&dst, int force) const #endif } else if ( force == AF_INET || (force == AF_UNSPEC && isIPv4()) ) { - - dst->ai_addr = (struct sockaddr*)new sockaddr_in; - - memset(dst->ai_addr,0,sizeof(struct sockaddr_in)); - - getSockAddr(*((struct sockaddr_in*)dst->ai_addr)); - - dst->ai_addrlen = sizeof(struct sockaddr_in); - - dst->ai_family = ((struct sockaddr_in*)dst->ai_addr)->sin_family; + initSockAddr(dst, SockAddrType::SockAddrIn); + getSockAddr(*(reinterpret_cast(dst->ai_addr))); + dst->ai_family = (reinterpret_cast(dst->ai_addr))->sin_family; } else { IASSERT("false",false); } @@ -683,13 +671,9 @@ Ip::Address::InitAddr(struct addrinfo *&ai) } // remove any existing data. - if (ai->ai_addr) delete ai->ai_addr; - - ai->ai_addr = (struct sockaddr*)new sockaddr_in6; - memset(ai->ai_addr, 0, sizeof(struct sockaddr_in6)); - - ai->ai_addrlen = sizeof(struct sockaddr_in6); + releaseSockAddr(ai); + initSockAddr(ai, SockAddrType::SockAddrIn6); } void @@ -697,11 +681,7 @@ Ip::Address::FreeAddr(struct addrinfo *&ai) { if (ai == nullptr) return; - if (ai->ai_addr) delete (struct sockaddr_in6*)ai->ai_addr; - - ai->ai_addr = nullptr; - - ai->ai_addrlen = 0; + releaseSockAddr(ai); // NP: name fields are NOT allocated at present. delete ai; @@ -709,6 +689,47 @@ Ip::Address::FreeAddr(struct addrinfo *&ai) ai = nullptr; } +void +Ip::Address::initSockAddr(struct addrinfo* ai, SockAddrType sockaddrType) { + switch (sockaddrType) { + case Ip::SockAddrType::SockAddrIn: { + ai->ai_addr = reinterpret_cast(new struct sockaddr_in); + memset(ai->ai_addr, 0, sizeof(struct sockaddr_in)); + ai->ai_addrlen = sizeof(struct sockaddr_in); + } + break; + case Ip::SockAddrType::SockAddrIn6: { + ai->ai_addr = reinterpret_cast(new struct sockaddr_in6); + memset(ai->ai_addr, 0, sizeof(struct sockaddr_in6)); + ai->ai_addrlen = sizeof(struct sockaddr_in6); + } + break; + } +} + +void +Ip::Address::releaseSockAddr(struct addrinfo* ai) +{ + if (!ai) { + return; + } + + if (!ai->ai_addr) { + return; + } + + if (ai->ai_addrlen == sizeof(struct sockaddr_in)) { + delete reinterpret_cast(ai->ai_addr); + } else if (ai->ai_addrlen == sizeof(struct sockaddr_in6)) { + delete reinterpret_cast(ai->ai_addr); + } else { + throw TextException("Bad sockaddr structure", Here()); + } + + ai->ai_addr = nullptr; + ai->ai_addrlen = 0; +} + int Ip::Address::matchIPAddr(const Ip::Address &rhs) const { diff --git a/src/ip/Address.h b/src/ip/Address.h index 49021a77d2a..8024cea183c 100644 --- a/src/ip/Address.h +++ b/src/ip/Address.h @@ -34,6 +34,11 @@ namespace Ip { +enum class SockAddrType { + SockAddrIn, + SockAddrIn6 +}; + /** * Holds and manipulates IPv4, IPv6, and Socket Addresses. */ @@ -337,6 +342,9 @@ class Address // Worker behind GetHostName and char* converters bool lookupHostIP(const char *s, bool nodns); + static void initSockAddr(struct addrinfo* ai, SockAddrType sockaddrType); + static void releaseSockAddr(struct addrinfo* ai); + /* variables */ struct sockaddr_in6 mSocketAddr_; From bb7f0c191e5d8bfb077317230a4785971d5da55c Mon Sep 17 00:00:00 2001 From: Alexey Simakov Date: Thu, 15 Feb 2024 20:48:31 +0300 Subject: [PATCH 04/14] Fix incorrect merge result --- src/ip/Address.cc | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/ip/Address.cc b/src/ip/Address.cc index f85099695e9..e85cbe5a492 100644 --- a/src/ip/Address.cc +++ b/src/ip/Address.cc @@ -671,17 +671,7 @@ Ip::Address::InitAddr(struct addrinfo *&ai) } // remove any existing data. -<<<<<<< HEAD releaseSockAddr(ai); -======= - if (ai->ai_addr) - delete ai->ai_addr; - - ai->ai_addr = static_cast(new sockaddr_in6); - memset(ai->ai_addr, 0, sizeof(struct sockaddr_in6)); - - ai->ai_addrlen = sizeof(struct sockaddr_in6); ->>>>>>> 6ffe15818c6f5e144017b7bb1f6f5e68b6921c75 initSockAddr(ai, SockAddrType::SockAddrIn6); } @@ -692,16 +682,7 @@ Ip::Address::FreeAddr(struct addrinfo *&ai) if (!ai) return; -<<<<<<< HEAD releaseSockAddr(ai); -======= - if (ai->ai_addr) - delete static_cast(struct sockaddr_in6*>(ai->ai_addr); - - ai->ai_addr = nullptr; - - ai->ai_addrlen = 0; ->>>>>>> 6ffe15818c6f5e144017b7bb1f6f5e68b6921c75 // NP: name fields are NOT allocated at present. delete ai; From 4f9cb68edcbe063b434ad8ce1571e65c946e92f0 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Fri, 16 Feb 2024 08:55:57 -0500 Subject: [PATCH 05/14] fixup: Do not expose Ip::Address users to ai_addr matters Also fix "git diff --check" --- src/ip/Address.cc | 16 ++++++++++++++-- src/ip/Address.h | 8 -------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/ip/Address.cc b/src/ip/Address.cc index e85cbe5a492..acc2fb30514 100644 --- a/src/ip/Address.cc +++ b/src/ip/Address.cc @@ -42,6 +42,18 @@ } printf("\n"); assert(b); \ } +namespace Ip { + +enum class SockAddrType { + SockAddrIn, + SockAddrIn6 +}; + +static void initSockAddr(struct addrinfo *, SockAddrType); +static void releaseSockAddr(struct addrinfo *); + +} // namespace Ip + int Ip::Address::cidr() const { @@ -691,7 +703,7 @@ Ip::Address::FreeAddr(struct addrinfo *&ai) } void -Ip::Address::initSockAddr(struct addrinfo* ai, SockAddrType sockaddrType) { +Ip::initSockAddr(struct addrinfo* ai, SockAddrType sockaddrType) { switch (sockaddrType) { case Ip::SockAddrType::SockAddrIn: { ai->ai_addr = reinterpret_cast(new struct sockaddr_in); @@ -709,7 +721,7 @@ Ip::Address::initSockAddr(struct addrinfo* ai, SockAddrType sockaddrType) { } void -Ip::Address::releaseSockAddr(struct addrinfo* ai) +Ip::releaseSockAddr(struct addrinfo* ai) { if (!ai) { return; diff --git a/src/ip/Address.h b/src/ip/Address.h index 8024cea183c..49021a77d2a 100644 --- a/src/ip/Address.h +++ b/src/ip/Address.h @@ -34,11 +34,6 @@ namespace Ip { -enum class SockAddrType { - SockAddrIn, - SockAddrIn6 -}; - /** * Holds and manipulates IPv4, IPv6, and Socket Addresses. */ @@ -342,9 +337,6 @@ class Address // Worker behind GetHostName and char* converters bool lookupHostIP(const char *s, bool nodns); - static void initSockAddr(struct addrinfo* ai, SockAddrType sockaddrType); - static void releaseSockAddr(struct addrinfo* ai); - /* variables */ struct sockaddr_in6 mSocketAddr_; From 51f4bcafcf99dd25f30fb141b708735748b676ef Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Fri, 16 Feb 2024 09:28:07 -0500 Subject: [PATCH 06/14] fixup: Use C++ to produce safer, cleaner code Also renamed new helper functions to emphasize that memory is allocated and to avoid a false implication that ai_addr is not initialized until initSockAddr(). The new names are arguably more symmetrical as well. Also improved const-correctness of new function parameters. --- src/ip/Address.cc | 47 +++++++++++++++++++---------------------------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/src/ip/Address.cc b/src/ip/Address.cc index acc2fb30514..742dfb320c4 100644 --- a/src/ip/Address.cc +++ b/src/ip/Address.cc @@ -44,13 +44,9 @@ namespace Ip { -enum class SockAddrType { - SockAddrIn, - SockAddrIn6 -}; - -static void initSockAddr(struct addrinfo *, SockAddrType); -static void releaseSockAddr(struct addrinfo *); +template +static void AllocateAddrMember(struct addrinfo *); +static void FreeAddrMember(struct addrinfo *); } // namespace Ip @@ -648,7 +644,7 @@ Ip::Address::getAddrInfo(struct addrinfo *&dst, int force) const dst->ai_protocol = IPPROTO_UDP; if (force == AF_INET6 || (force == AF_UNSPEC && isIPv6()) ) { - initSockAddr(dst, SockAddrType::SockAddrIn6); + AllocateAddrMember(dst); getSockAddr(*(reinterpret_cast(dst->ai_addr))); dst->ai_family = (reinterpret_cast(dst->ai_addr))->sin6_family; #if 0 @@ -666,7 +662,7 @@ Ip::Address::getAddrInfo(struct addrinfo *&dst, int force) const #endif } else if ( force == AF_INET || (force == AF_UNSPEC && isIPv4()) ) { - initSockAddr(dst, SockAddrType::SockAddrIn); + AllocateAddrMember(dst); getSockAddr(*(reinterpret_cast(dst->ai_addr))); dst->ai_family = (reinterpret_cast(dst->ai_addr))->sin_family; } else { @@ -683,9 +679,9 @@ Ip::Address::InitAddr(struct addrinfo *&ai) } // remove any existing data. - releaseSockAddr(ai); + FreeAddrMember(ai); - initSockAddr(ai, SockAddrType::SockAddrIn6); + AllocateAddrMember(ai); } void @@ -694,7 +690,7 @@ Ip::Address::FreeAddr(struct addrinfo *&ai) if (!ai) return; - releaseSockAddr(ai); + FreeAddrMember(ai); // NP: name fields are NOT allocated at present. delete ai; @@ -702,26 +698,21 @@ Ip::Address::FreeAddr(struct addrinfo *&ai) ai = nullptr; } +template void -Ip::initSockAddr(struct addrinfo* ai, SockAddrType sockaddrType) { - switch (sockaddrType) { - case Ip::SockAddrType::SockAddrIn: { - ai->ai_addr = reinterpret_cast(new struct sockaddr_in); - memset(ai->ai_addr, 0, sizeof(struct sockaddr_in)); - ai->ai_addrlen = sizeof(struct sockaddr_in); - } - break; - case Ip::SockAddrType::SockAddrIn6: { - ai->ai_addr = reinterpret_cast(new struct sockaddr_in6); - memset(ai->ai_addr, 0, sizeof(struct sockaddr_in6)); - ai->ai_addrlen = sizeof(struct sockaddr_in6); - } - break; - } +Ip::AllocateAddrMember(struct addrinfo * const ai) +{ + static_assert( + std::is_same::value || + std::is_same::value, + "FreeAddrMember() supports this addrinfo::ai_addr type"); + ai->ai_addr = reinterpret_cast(new SockAddrType); + memset(ai->ai_addr, 0, sizeof(SockAddrType)); + ai->ai_addrlen = sizeof(SockAddrType); } void -Ip::releaseSockAddr(struct addrinfo* ai) +Ip::FreeAddrMember(struct addrinfo * const ai) { if (!ai) { return; From 5bbc9082633c3cc071690191f9636e7900039c3b Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Fri, 16 Feb 2024 10:10:10 -0500 Subject: [PATCH 07/14] fixup: Explain why we should continue to memset() here See excellent research at https://stackoverflow.com/q/70979077 and an insightful comment at https://stackoverflow.com/a/39431986 --- src/ip/Address.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ip/Address.cc b/src/ip/Address.cc index 742dfb320c4..4df50afd93e 100644 --- a/src/ip/Address.cc +++ b/src/ip/Address.cc @@ -707,6 +707,8 @@ Ip::AllocateAddrMember(struct addrinfo * const ai) std::is_same::value, "FreeAddrMember() supports this addrinfo::ai_addr type"); ai->ai_addr = reinterpret_cast(new SockAddrType); + // We do not use `new SockAddrType{}` above instead of memset() below + // because, since C++14, doing so may not initialize SockAddrType padding. memset(ai->ai_addr, 0, sizeof(SockAddrType)); ai->ai_addrlen = sizeof(SockAddrType); } From 83267df07227383c173f369d800603ddc6786310 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Fri, 16 Feb 2024 10:16:28 -0500 Subject: [PATCH 08/14] fixup: Fixed argument type of new functions This also reduces doubts about what FreeAddrMember() frees. --- src/ip/Address.cc | 42 +++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/src/ip/Address.cc b/src/ip/Address.cc index 4df50afd93e..22f0054ada0 100644 --- a/src/ip/Address.cc +++ b/src/ip/Address.cc @@ -45,8 +45,8 @@ namespace Ip { template -static void AllocateAddrMember(struct addrinfo *); -static void FreeAddrMember(struct addrinfo *); +static void AllocateAddrMember(struct addrinfo &); +static void FreeAddrMember(struct addrinfo &); } // namespace Ip @@ -644,7 +644,7 @@ Ip::Address::getAddrInfo(struct addrinfo *&dst, int force) const dst->ai_protocol = IPPROTO_UDP; if (force == AF_INET6 || (force == AF_UNSPEC && isIPv6()) ) { - AllocateAddrMember(dst); + AllocateAddrMember(*dst); getSockAddr(*(reinterpret_cast(dst->ai_addr))); dst->ai_family = (reinterpret_cast(dst->ai_addr))->sin6_family; #if 0 @@ -662,7 +662,7 @@ Ip::Address::getAddrInfo(struct addrinfo *&dst, int force) const #endif } else if ( force == AF_INET || (force == AF_UNSPEC && isIPv4()) ) { - AllocateAddrMember(dst); + AllocateAddrMember(*dst); getSockAddr(*(reinterpret_cast(dst->ai_addr))); dst->ai_family = (reinterpret_cast(dst->ai_addr))->sin_family; } else { @@ -679,9 +679,9 @@ Ip::Address::InitAddr(struct addrinfo *&ai) } // remove any existing data. - FreeAddrMember(ai); + FreeAddrMember(*ai); - AllocateAddrMember(ai); + AllocateAddrMember(*ai); } void @@ -690,7 +690,7 @@ Ip::Address::FreeAddr(struct addrinfo *&ai) if (!ai) return; - FreeAddrMember(ai); + FreeAddrMember(*ai); // NP: name fields are NOT allocated at present. delete ai; @@ -700,40 +700,36 @@ Ip::Address::FreeAddr(struct addrinfo *&ai) template void -Ip::AllocateAddrMember(struct addrinfo * const ai) +Ip::AllocateAddrMember(struct addrinfo &ai) { static_assert( std::is_same::value || std::is_same::value, "FreeAddrMember() supports this addrinfo::ai_addr type"); - ai->ai_addr = reinterpret_cast(new SockAddrType); + ai.ai_addr = reinterpret_cast(new SockAddrType); // We do not use `new SockAddrType{}` above instead of memset() below // because, since C++14, doing so may not initialize SockAddrType padding. - memset(ai->ai_addr, 0, sizeof(SockAddrType)); - ai->ai_addrlen = sizeof(SockAddrType); + memset(ai.ai_addr, 0, sizeof(SockAddrType)); + ai.ai_addrlen = sizeof(SockAddrType); } void -Ip::FreeAddrMember(struct addrinfo * const ai) +Ip::FreeAddrMember(struct addrinfo &ai) { - if (!ai) { + if (!ai.ai_addr) { return; } - if (!ai->ai_addr) { - return; - } - - if (ai->ai_addrlen == sizeof(struct sockaddr_in)) { - delete reinterpret_cast(ai->ai_addr); - } else if (ai->ai_addrlen == sizeof(struct sockaddr_in6)) { - delete reinterpret_cast(ai->ai_addr); + if (ai.ai_addrlen == sizeof(struct sockaddr_in)) { + delete reinterpret_cast(ai.ai_addr); + } else if (ai.ai_addrlen == sizeof(struct sockaddr_in6)) { + delete reinterpret_cast(ai.ai_addr); } else { throw TextException("Bad sockaddr structure", Here()); } - ai->ai_addr = nullptr; - ai->ai_addrlen = 0; + ai.ai_addr = nullptr; + ai.ai_addrlen = 0; } int From d408e8fedcc6dcae926c99444d325fec5ed1ba32 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Fri, 16 Feb 2024 10:23:31 -0500 Subject: [PATCH 09/14] fixup: Do not throw when cleaning up Code that frees memory is often used in destructors. Destructors must not throw. We could assert instead, but perhaps there is no need to kill worker in this case? Current ip/Address.cc code is not consistent with regard to assert-vs-report-a-bug choice. --- src/ip/Address.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ip/Address.cc b/src/ip/Address.cc index 22f0054ada0..f5b9eb262a5 100644 --- a/src/ip/Address.cc +++ b/src/ip/Address.cc @@ -9,8 +9,6 @@ /* DEBUG: section 14 IP Storage and Handling */ #include "squid.h" - -#include "base/TextException.h" #include "debug/Stream.h" #include "ip/Address.h" #include "ip/tools.h" @@ -725,7 +723,10 @@ Ip::FreeAddrMember(struct addrinfo &ai) } else if (ai.ai_addrlen == sizeof(struct sockaddr_in6)) { delete reinterpret_cast(ai.ai_addr); } else { - throw TextException("Bad sockaddr structure", Here()); + debugs(14, DBG_CRITICAL, "ERROR: Squid Bug: Unexpected addrinfo::ai_addr size: " << ai.ai_addrlen << + Debug::Extra << "sockaddr_in size: " << sizeof(struct sockaddr_in) << + Debug::Extra << "sockaddr_in6 size: " << sizeof(struct sockaddr_in6)); + // leak memory } ai.ai_addr = nullptr; From b4499ac47e1c37c031dfd18a57bc11171fad5d39 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Fri, 16 Feb 2024 10:37:27 -0500 Subject: [PATCH 10/14] fixup: Undo out-of-scope formatting improvements --- src/ip/Address.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ip/Address.cc b/src/ip/Address.cc index f5b9eb262a5..4b21e27a2b9 100644 --- a/src/ip/Address.cc +++ b/src/ip/Address.cc @@ -685,8 +685,7 @@ Ip::Address::InitAddr(struct addrinfo *&ai) void Ip::Address::FreeAddr(struct addrinfo *&ai) { - if (!ai) - return; + if (ai == nullptr) return; FreeAddrMember(*ai); From 49345e0adc02b78c33cc334c05bf09adc3cb94b5 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Fri, 16 Feb 2024 10:53:17 -0500 Subject: [PATCH 11/14] fixup: Simplified -- we know what we have allocated --- src/ip/Address.cc | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/ip/Address.cc b/src/ip/Address.cc index 4b21e27a2b9..220895915f0 100644 --- a/src/ip/Address.cc +++ b/src/ip/Address.cc @@ -43,7 +43,7 @@ namespace Ip { template -static void AllocateAddrMember(struct addrinfo &); +static SockAddrType &AllocateAddrMember(struct addrinfo &); static void FreeAddrMember(struct addrinfo &); } // namespace Ip @@ -642,9 +642,9 @@ Ip::Address::getAddrInfo(struct addrinfo *&dst, int force) const dst->ai_protocol = IPPROTO_UDP; if (force == AF_INET6 || (force == AF_UNSPEC && isIPv6()) ) { - AllocateAddrMember(*dst); - getSockAddr(*(reinterpret_cast(dst->ai_addr))); - dst->ai_family = (reinterpret_cast(dst->ai_addr))->sin6_family; + auto &ai_addr = AllocateAddrMember(*dst); + getSockAddr(ai_addr); + dst->ai_family = ai_addr.sin6_family; #if 0 /** * Enable only if you must and please report to squid-dev if you find a need for this. @@ -660,9 +660,9 @@ Ip::Address::getAddrInfo(struct addrinfo *&dst, int force) const #endif } else if ( force == AF_INET || (force == AF_UNSPEC && isIPv4()) ) { - AllocateAddrMember(*dst); - getSockAddr(*(reinterpret_cast(dst->ai_addr))); - dst->ai_family = (reinterpret_cast(dst->ai_addr))->sin_family; + auto &ai_addr = AllocateAddrMember(*dst); + getSockAddr(ai_addr); + dst->ai_family = ai_addr.sin_family; } else { IASSERT("false",false); } @@ -695,19 +695,27 @@ Ip::Address::FreeAddr(struct addrinfo *&ai) ai = nullptr; } +/// Allocates and fills with zeros a SockAddrType object. +/// Initializes addrinfo::ai_addr and ai_addrlen with the allocated object. +/// \returns the allocated object template -void +SockAddrType & Ip::AllocateAddrMember(struct addrinfo &ai) { static_assert( std::is_same::value || std::is_same::value, "FreeAddrMember() supports this addrinfo::ai_addr type"); - ai.ai_addr = reinterpret_cast(new SockAddrType); + + const auto ai_addr = new SockAddrType; // We do not use `new SockAddrType{}` above instead of memset() below // because, since C++14, doing so may not initialize SockAddrType padding. - memset(ai.ai_addr, 0, sizeof(SockAddrType)); - ai.ai_addrlen = sizeof(SockAddrType); + memset(ai_addr, 0, sizeof(*ai_addr)); + + ai.ai_addr = reinterpret_cast(ai_addr); + ai.ai_addrlen = sizeof(*ai_addr); + + return *ai_addr; } void From f6ff166240c95d6f1bc95983cd760f49c51d1d9b Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Fri, 16 Feb 2024 10:54:21 -0500 Subject: [PATCH 12/14] fiuxp: More paranoid assertions --- src/ip/Address.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ip/Address.cc b/src/ip/Address.cc index 220895915f0..e7d03b99622 100644 --- a/src/ip/Address.cc +++ b/src/ip/Address.cc @@ -710,6 +710,7 @@ Ip::AllocateAddrMember(struct addrinfo &ai) const auto ai_addr = new SockAddrType; // We do not use `new SockAddrType{}` above instead of memset() below // because, since C++14, doing so may not initialize SockAddrType padding. + static_assert(std::is_trivial::value, "can memset() this addrinfo::ai_addr type"); memset(ai_addr, 0, sizeof(*ai_addr)); ai.ai_addr = reinterpret_cast(ai_addr); From 519cb62336b62f97575b8ed44218d51b846d7aad Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Fri, 16 Feb 2024 11:08:05 -0500 Subject: [PATCH 13/14] fixup: Documented branch-added Ip::FreeAddrMember() function --- src/ip/Address.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ip/Address.cc b/src/ip/Address.cc index e7d03b99622..83b3697ee95 100644 --- a/src/ip/Address.cc +++ b/src/ip/Address.cc @@ -719,6 +719,9 @@ Ip::AllocateAddrMember(struct addrinfo &ai) return *ai_addr; } +/// Deallocates addrinfo::ai_addr member of the given structure and adjusts that +/// structure accordingly. Safe for structures with a nil ai_addr member. +/// \sa Ip::AllocateAddrMember() void Ip::FreeAddrMember(struct addrinfo &ai) { From 2cacd160654239290ba72b7c31f0cf08b7180bfd Mon Sep 17 00:00:00 2001 From: Alexey Simakov Date: Wed, 28 Feb 2024 19:36:59 +0300 Subject: [PATCH 14/14] Fix codestyle requests --- src/ip/Address.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/ip/Address.cc b/src/ip/Address.cc index 83b3697ee95..5306a6a9ef2 100644 --- a/src/ip/Address.cc +++ b/src/ip/Address.cc @@ -725,15 +725,14 @@ Ip::AllocateAddrMember(struct addrinfo &ai) void Ip::FreeAddrMember(struct addrinfo &ai) { - if (!ai.ai_addr) { + if (!ai.ai_addr) return; - } - if (ai.ai_addrlen == sizeof(struct sockaddr_in)) { + if (ai.ai_addrlen == sizeof(struct sockaddr_in)) delete reinterpret_cast(ai.ai_addr); - } else if (ai.ai_addrlen == sizeof(struct sockaddr_in6)) { + else if (ai.ai_addrlen == sizeof(struct sockaddr_in6)) delete reinterpret_cast(ai.ai_addr); - } else { + else { debugs(14, DBG_CRITICAL, "ERROR: Squid Bug: Unexpected addrinfo::ai_addr size: " << ai.ai_addrlen << Debug::Extra << "sockaddr_in size: " << sizeof(struct sockaddr_in) << Debug::Extra << "sockaddr_in6 size: " << sizeof(struct sockaddr_in6));