This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.2.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit e2fb34599335c0383b7ac18e793a5842453109c3 Author: Mo Chen <[email protected]> AuthorDate: Thu Jun 18 16:13:16 2026 -0500 ProxyProtocol: free pp_info heap on NetVConnection recycle (#13293) NetVConnection embeds a ProxyProtocol by value, and a PROXY v2 header parsed by has_proxy_protocol() heap-allocates ProxyProtocol::additional_data (the TLV blob) plus the tlv map. The NetVConnection ClassAllocators are Destruct_on_free=false, so ~ProxyProtocol never runs when a VC is recycled and UnixNetVConnection::clear() did not release pp_info -- the next placement-new on the recycled slot abandoned the buffer and map nodes, leaking once per recycled connection that carried a PROXY v2 header. Behind a PROXY-protocol load balancer that is every inbound connection. Add ProxyProtocol::reset() and call it from UnixNetVConnection::clear() (the single recycle chokepoint for the Unix, SSL and QUIC VCs). reset() swaps the members with empty containers rather than clear()ing them, because clear() retains capacity that the recycle would still abandon. Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]> (cherry picked from commit 95a13a552d02a5f4621a3c10127b7f16b4b47780) --- include/iocore/net/ProxyProtocol.h | 19 ++++++++++ src/iocore/net/UnixNetVConnection.cc | 3 ++ src/iocore/net/unit_tests/test_ProxyProtocol.cc | 50 +++++++++++++++++++++++++ 3 files changed, 72 insertions(+) diff --git a/include/iocore/net/ProxyProtocol.h b/include/iocore/net/ProxyProtocol.h index d3a7c73671..7d7fc34e46 100644 --- a/include/iocore/net/ProxyProtocol.h +++ b/include/iocore/net/ProxyProtocol.h @@ -82,6 +82,25 @@ public: other.tlv.clear(); } ~ProxyProtocol() = default; + + /** Release owned heap and reset to the default-constructed state. + * + * Swap rather than clear() the containers: clear() retains capacity, which would be abandoned + * (leaked) when the slot is reused, since the NetVConnection allocators are Destruct_on_free=false + * and so never run ~ProxyProtocol. + */ + void + reset() + { + std::string{}.swap(additional_data); + std::unordered_map<uint8_t, std::string_view>{}.swap(tlv); + version = ProxyProtocolVersion::UNDEFINED; + ip_family = AF_UNSPEC; + type = 0; + src_addr = {}; + dst_addr = {}; + } + int set_additional_data(std::string_view data); void set_ipv4_addrs(in_addr_t src_addr, uint16_t src_port, in_addr_t dst_addr, uint16_t dst_port); void set_ipv6_addrs(const in6_addr &src_addr, uint16_t src_port, const in6_addr &dst_addr, uint16_t dst_port); diff --git a/src/iocore/net/UnixNetVConnection.cc b/src/iocore/net/UnixNetVConnection.cc index 29e1b4dead..8e18df53ed 100644 --- a/src/iocore/net/UnixNetVConnection.cc +++ b/src/iocore/net/UnixNetVConnection.cc @@ -1220,6 +1220,9 @@ UnixNetVConnection::clear() read.vio.vc_server = nullptr; write.vio.vc_server = nullptr; options.reset(); + // The VC allocator is Destruct_on_free=false (~ProxyProtocol never runs on recycle), so release + // pp_info's heap explicitly here. + pp_info.reset(); if (netvc_context == NET_VCONNECTION_OUT) { read.vio.buffer.clear(); write.vio.buffer.clear(); diff --git a/src/iocore/net/unit_tests/test_ProxyProtocol.cc b/src/iocore/net/unit_tests/test_ProxyProtocol.cc index ada8605f08..cc62e63e82 100644 --- a/src/iocore/net/unit_tests/test_ProxyProtocol.cc +++ b/src/iocore/net/unit_tests/test_ProxyProtocol.cc @@ -956,3 +956,53 @@ TEST_CASE("ProxyProtocol Rule of 5", "[ProxyProtocol]") CHECK(orig_tlv.value() == "original"); } } + +TEST_CASE("ProxyProtocol reset", "[ProxyProtocol]") +{ + // A PROXY v2 / TCP over IPv4 header carrying TLVs, so the parsed pp_info owns both the + // additional_data string and the tlv map that reset() must release. + uint8_t raw_data[] = { + 0x0D, 0x0A, 0x0D, 0x0A, 0x00, 0x0D, 0x0A, 0x51, ///< preface + 0x55, 0x49, 0x54, 0x0A, ///< + 0x21, ///< version & command + 0x11, ///< protocol & family + 0x00, 0x32, ///< len + 0xC0, 0x00, 0x02, 0x01, ///< src_addr + 0xC6, 0x33, 0x64, 0x01, ///< dst_addr + 0xC3, 0x50, ///< src_port + 0x01, 0xBB, ///< dst_port + 0x01, 0x00, 0x02, 0x68, 0x32, /// PP2_TYPE_ALPN (h2) + 0x02, 0x00, 0x03, 0x61, 0x62, 0x63, /// PP2_TYPE_AUTHORITY (abc) + 0x20, 0x00, 0x18, 0x01, 0x00, 0x00, 0x00, 0x00, /// PP2_TYPE_SSL (client=0x01, verify=0) + 0x23, 0x00, 0x03, 0x58, 0x59, 0x5A, /// PP2_SUBTYPE_SSL_CIPHER (XYZ) + 0x26, 0x00, 0x04, 0x58, 0x31, 0x32, 0x33, /// PP2_SUBTYPE_SSL_GROUP (X123) + 0x21, 0x00, 0x03, 0x54, 0x4C, 0x53, /// PP2_SUBTYPE_SSL_VERSION (TLS) + }; + + swoc::TextView tv(reinterpret_cast<char *>(raw_data), sizeof(raw_data)); + + ProxyProtocol pp_info; + REQUIRE(proxy_protocol_parse(&pp_info, tv) == tv.size()); + + // Precondition: pp_info is fully populated, including its heap-owning members. + REQUIRE(pp_info.version == ProxyProtocolVersion::V2); + REQUIRE(pp_info.ip_family == AF_INET); + REQUIRE(pp_info.type == SOCK_STREAM); + REQUIRE_FALSE(pp_info.tlv.empty()); + REQUIRE(pp_info.get_tlv(PP2_TYPE_ALPN).has_value()); + + pp_info.reset(); + + // Every field is back to the default-constructed state and the backing storage is released. + CHECK(pp_info.version == ProxyProtocolVersion::UNDEFINED); + CHECK(pp_info.ip_family == AF_UNSPEC); + CHECK(pp_info.type == 0); + CHECK_FALSE(ats_is_ip(&pp_info.src_addr)); + CHECK_FALSE(ats_is_ip(&pp_info.dst_addr)); + CHECK(pp_info.tlv.empty()); + CHECK_FALSE(pp_info.get_tlv(PP2_TYPE_ALPN).has_value()); + + // reset() is idempotent and safe to call on an already-empty object. + pp_info.reset(); + CHECK(pp_info.tlv.empty()); +}
