You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by sc...@apache.org on 2018/03/05 02:25:35 UTC
[trafficserver] branch quic-latest updated: QUIC: Mem leaking with
QUICPacketHeader
This is an automated email from the ASF dual-hosted git repository.
scw00 pushed a commit to branch quic-latest
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/quic-latest by this push:
new c3af882 QUIC: Mem leaking with QUICPacketHeader
c3af882 is described below
commit c3af882434fd279e82e5534a55f2faded535e957
Author: scw00 <sc...@apache.org>
AuthorDate: Mon Mar 5 08:31:34 2018 +0800
QUIC: Mem leaking with QUICPacketHeader
---
iocore/net/quic/QUICPacket.cc | 81 ++++++++++++++++++++++---------------------
iocore/net/quic/QUICPacket.h | 78 +++++++++++++++++++++++++++++------------
2 files changed, 98 insertions(+), 61 deletions(-)
diff --git a/iocore/net/quic/QUICPacket.cc b/iocore/net/quic/QUICPacket.cc
index a14140d..a76d068 100644
--- a/iocore/net/quic/QUICPacket.cc
+++ b/iocore/net/quic/QUICPacket.cc
@@ -61,52 +61,54 @@ QUICPacketHeader::packet_size() const
return this->_buf_len;
}
-QUICPacketHeader *
+QUICPacketHeaderUPtr
QUICPacketHeader::load(const uint8_t *buf, size_t len, QUICPacketNumber base)
{
+ QUICPacketHeaderUPtr header = QUICPacketHeaderUPtr(nullptr, &QUICPacketHeaderDeleter::delete_null_header);
if (QUICTypeUtil::has_long_header(buf)) {
QUICPacketLongHeader *long_header = quicPacketLongHeaderAllocator.alloc();
new (long_header) QUICPacketLongHeader(buf, len, base);
- return long_header;
+ header = QUICPacketHeaderUPtr(long_header, &QUICPacketHeaderDeleter::delete_long_header);
} else {
QUICPacketShortHeader *short_header = quicPacketShortHeaderAllocator.alloc();
new (short_header) QUICPacketShortHeader(buf, len, base);
- return short_header;
+ header = QUICPacketHeaderUPtr(short_header, &QUICPacketHeaderDeleter::delete_short_header);
}
+ return std::move(header);
}
-QUICPacketHeader *
+QUICPacketHeaderUPtr
QUICPacketHeader::build(QUICPacketType type, QUICConnectionId connection_id, QUICPacketNumber packet_number,
QUICPacketNumber base_packet_number, QUICVersion version, ats_unique_buf payload, size_t len)
{
QUICPacketLongHeader *long_header = quicPacketLongHeaderAllocator.alloc();
new (long_header) QUICPacketLongHeader(type, connection_id, packet_number, base_packet_number, version, std::move(payload), len);
- return long_header;
+ return QUICPacketHeaderUPtr(long_header, &QUICPacketHeaderDeleter::delete_long_header);
}
-QUICPacketHeader *
+QUICPacketHeaderUPtr
QUICPacketHeader::build(QUICPacketType type, QUICKeyPhase key_phase, QUICPacketNumber packet_number,
QUICPacketNumber base_packet_number, ats_unique_buf payload, size_t len)
{
QUICPacketShortHeader *short_header = quicPacketShortHeaderAllocator.alloc();
new (short_header) QUICPacketShortHeader(type, key_phase, packet_number, base_packet_number, std::move(payload), len);
- return short_header;
+ return QUICPacketHeaderUPtr(short_header, &QUICPacketHeaderDeleter::delete_short_header);
}
-QUICPacketHeader *
+QUICPacketHeaderUPtr
QUICPacketHeader::build(QUICPacketType type, QUICKeyPhase key_phase, QUICConnectionId connection_id, QUICPacketNumber packet_number,
QUICPacketNumber base_packet_number, ats_unique_buf payload, size_t len)
{
QUICPacketShortHeader *short_header = quicPacketShortHeaderAllocator.alloc();
new (short_header)
QUICPacketShortHeader(type, key_phase, connection_id, packet_number, base_packet_number, std::move(payload), len);
- return short_header;
+ return QUICPacketHeaderUPtr(short_header, &QUICPacketHeaderDeleter::delete_short_header);
}
-QUICPacketHeader *
+QUICPacketHeaderUPtr
QUICPacketHeader::clone() const
{
- return nullptr;
+ return QUICPacketHeaderUPtr(nullptr, &QUICPacketHeaderDeleter::delete_null_header);
}
//
@@ -473,16 +475,20 @@ QUICPacketShortHeader::store(uint8_t *buf, size_t *len) const
// QUICPacket
//
-QUICPacket::QUICPacket(QUICPacketHeader *header, ats_unique_buf payload, size_t payload_len)
+QUICPacket::QUICPacket()
{
- this->_header = header;
+}
+
+QUICPacket::QUICPacket(QUICPacketHeaderUPtr header, ats_unique_buf payload, size_t payload_len)
+{
+ this->_header = std::move(header);
this->_payload = std::move(payload);
this->_payload_size = payload_len;
}
-QUICPacket::QUICPacket(QUICPacketHeader *header, ats_unique_buf payload, size_t payload_len, bool retransmittable)
+QUICPacket::QUICPacket(QUICPacketHeaderUPtr header, ats_unique_buf payload, size_t payload_len, bool retransmittable)
{
- this->_header = header;
+ this->_header = std::move(header);
this->_payload = std::move(payload);
this->_payload_size = payload_len;
this->_is_retransmittable = retransmittable;
@@ -490,11 +496,7 @@ QUICPacket::QUICPacket(QUICPacketHeader *header, ats_unique_buf payload, size_t
QUICPacket::~QUICPacket()
{
- if (this->_header->has_version()) {
- quicPacketLongHeaderAllocator.free(static_cast<QUICPacketLongHeader *>(this->_header));
- } else {
- quicPacketShortHeaderAllocator.free(static_cast<QUICPacketShortHeader *>(this->_header));
- }
+ this->_header = nullptr;
}
/**
@@ -522,7 +524,7 @@ QUICPacket::packet_number() const
const QUICPacketHeader *
QUICPacket::header() const
{
- return this->_header;
+ return this->_header.get();
}
const uint8_t *
@@ -661,7 +663,7 @@ QUICPacketFactory::create(ats_unique_buf buf, size_t len, QUICPacketNumber base_
ats_unique_buf plain_txt = ats_unique_malloc(max_plain_txt_len);
size_t plain_txt_len = 0;
- QUICPacketHeader *header = QUICPacketHeader::load(buf.release(), len, base_packet_number);
+ QUICPacketHeaderUPtr header = QUICPacketHeader::load(buf.release(), len, base_packet_number);
if (header->has_version() && !QUICTypeUtil::is_supported_version(header->version())) {
// We can't decrypt packets that have unknown versions
@@ -733,7 +735,7 @@ QUICPacketFactory::create(ats_unique_buf buf, size_t len, QUICPacketNumber base_
QUICPacket *packet = nullptr;
if (result == QUICPacketCreationResult::SUCCESS || result == QUICPacketCreationResult::UNSUPPORTED) {
packet = quicPacketAllocator.alloc();
- new (packet) QUICPacket(header, std::move(plain_txt), plain_txt_len);
+ new (packet) QUICPacket(std::move(header), std::move(plain_txt), plain_txt_len);
}
return QUICPacketUPtr(packet, &QUICPacketDeleter::delete_packet);
@@ -752,30 +754,31 @@ QUICPacketFactory::create_version_negotiation_packet(const QUICPacket *packet_se
p += n;
}
- QUICPacketHeader *header =
+ QUICPacketHeaderUPtr header =
QUICPacketHeader::build(QUICPacketType::VERSION_NEGOTIATION, packet_sent_by_client->connection_id(),
packet_sent_by_client->packet_number(), base_packet_number, 0x00, std::move(versions), len);
- return QUICPacketFactory::_create_unprotected_packet(header);
+ return QUICPacketFactory::_create_unprotected_packet(std::move(header));
}
QUICPacketUPtr
QUICPacketFactory::create_initial_packet(QUICConnectionId connection_id, QUICPacketNumber base_packet_number, QUICVersion version,
ats_unique_buf payload, size_t len)
{
- QUICPacketHeader *header = QUICPacketHeader::build(QUICPacketType::INITIAL, connection_id, this->_packet_number_generator.next(),
- base_packet_number, version, std::move(payload), len);
- return this->_create_encrypted_packet(header, true);
+ QUICPacketHeaderUPtr header =
+ QUICPacketHeader::build(QUICPacketType::INITIAL, connection_id, this->_packet_number_generator.next(), base_packet_number,
+ version, std::move(payload), len);
+ return this->_create_encrypted_packet(std::move(header), true);
}
QUICPacketUPtr
QUICPacketFactory::create_handshake_packet(QUICConnectionId connection_id, QUICPacketNumber base_packet_number,
ats_unique_buf payload, size_t len, bool retransmittable)
{
- QUICPacketHeader *header =
+ QUICPacketHeaderUPtr header =
QUICPacketHeader::build(QUICPacketType::HANDSHAKE, connection_id, this->_packet_number_generator.next(), base_packet_number,
this->_version, std::move(payload), len);
- return this->_create_encrypted_packet(header, retransmittable);
+ return this->_create_encrypted_packet(std::move(header), retransmittable);
}
QUICPacketUPtr
@@ -783,10 +786,10 @@ QUICPacketFactory::create_server_protected_packet(QUICConnectionId connection_id
ats_unique_buf payload, size_t len, bool retransmittable)
{
// TODO Key phase should be picked up from QUICHandshakeProtocol, probably
- QUICPacketHeader *header =
+ QUICPacketHeaderUPtr header =
QUICPacketHeader::build(QUICPacketType::PROTECTED, QUICKeyPhase::PHASE_0, connection_id, this->_packet_number_generator.next(),
base_packet_number, std::move(payload), len);
- return this->_create_encrypted_packet(header, retransmittable);
+ return this->_create_encrypted_packet(std::move(header), retransmittable);
}
QUICPacketUPtr
@@ -807,26 +810,26 @@ QUICPacketFactory::create_stateless_reset_packet(QUICConnectionId connection_id,
memcpy(naked_payload + payload_len - 16, stateless_reset_token.buf(), 16);
// KeyPhase won't be used
- QUICPacketHeader *header = QUICPacketHeader::build(QUICPacketType::STATELESS_RESET, QUICKeyPhase::CLEARTEXT, connection_id,
- random_packet_number, 0, std::move(payload), payload_len);
- return QUICPacketFactory::_create_unprotected_packet(header);
+ QUICPacketHeaderUPtr header = QUICPacketHeader::build(QUICPacketType::STATELESS_RESET, QUICKeyPhase::CLEARTEXT, connection_id,
+ random_packet_number, 0, std::move(payload), payload_len);
+ return QUICPacketFactory::_create_unprotected_packet(std::move(header));
}
QUICPacketUPtr
-QUICPacketFactory::_create_unprotected_packet(QUICPacketHeader *header)
+QUICPacketFactory::_create_unprotected_packet(QUICPacketHeaderUPtr header)
{
ats_unique_buf cleartext = ats_unique_malloc(2048);
size_t cleartext_len = header->payload_size();
memcpy(cleartext.get(), header->payload(), cleartext_len);
QUICPacket *packet = quicPacketAllocator.alloc();
- new (packet) QUICPacket(header, std::move(cleartext), cleartext_len, false);
+ new (packet) QUICPacket(std::move(header), std::move(cleartext), cleartext_len, false);
return QUICPacketUPtr(packet, &QUICPacketDeleter::delete_packet);
}
QUICPacketUPtr
-QUICPacketFactory::_create_encrypted_packet(QUICPacketHeader *header, bool retransmittable)
+QUICPacketFactory::_create_encrypted_packet(QUICPacketHeaderUPtr header, bool retransmittable)
{
// TODO: use pmtu of UnixNetVConnection
size_t max_cipher_txt_len = 2048;
@@ -837,7 +840,7 @@ QUICPacketFactory::_create_encrypted_packet(QUICPacketHeader *header, bool retra
if (this->_hs_protocol->encrypt(cipher_txt.get(), cipher_txt_len, max_cipher_txt_len, header->payload(), header->payload_size(),
header->packet_number(), header->buf(), header->size(), header->key_phase())) {
packet = quicPacketAllocator.alloc();
- new (packet) QUICPacket(header, std::move(cipher_txt), cipher_txt_len, retransmittable);
+ new (packet) QUICPacket(std::move(header), std::move(cipher_txt), cipher_txt_len, retransmittable);
}
return QUICPacketUPtr(packet, &QUICPacketDeleter::delete_packet);
diff --git a/iocore/net/quic/QUICPacket.h b/iocore/net/quic/QUICPacket.h
index 2e52c6e..6c367c4 100644
--- a/iocore/net/quic/QUICPacket.h
+++ b/iocore/net/quic/QUICPacket.h
@@ -37,6 +37,18 @@
#define QUIC_FIELD_OFFSET_PACKET_NUMBER 4
#define QUIC_FIELD_OFFSET_PAYLOAD 5
+class QUICPacketHeader;
+class QUICPacket;
+class QUICPacketLongHeader;
+class QUICPacketShortHeader;
+
+extern ClassAllocator<QUICPacket> quicPacketAllocator;
+extern ClassAllocator<QUICPacketLongHeader> quicPacketLongHeaderAllocator;
+extern ClassAllocator<QUICPacketShortHeader> quicPacketShortHeaderAllocator;
+
+using QUICPacketHeaderDeleterFunc = void (*)(QUICPacketHeader *p);
+using QUICPacketHeaderUPtr = std::unique_ptr<QUICPacketHeader, QUICPacketHeaderDeleterFunc>;
+
class QUICPacketHeader
{
public:
@@ -79,7 +91,7 @@ public:
*/
virtual void store(uint8_t *buf, size_t *len) const = 0;
- QUICPacketHeader *clone() const;
+ QUICPacketHeaderUPtr clone() const;
virtual bool has_key_phase() const = 0;
virtual bool has_connection_id() const = 0;
@@ -92,32 +104,32 @@ public:
*
* This creates either a QUICPacketShortHeader or a QUICPacketLongHeader.
*/
- static QUICPacketHeader *load(const uint8_t *buf, size_t len, QUICPacketNumber base);
+ static QUICPacketHeaderUPtr load(const uint8_t *buf, size_t len, QUICPacketNumber base);
/*
* Build a QUICPacketHeader
*
* This creates a QUICPacketLongHeader.
*/
- static QUICPacketHeader *build(QUICPacketType type, QUICConnectionId connection_id, QUICPacketNumber packet_number,
- QUICPacketNumber base_packet_number, QUICVersion version, ats_unique_buf payload, size_t len);
+ static QUICPacketHeaderUPtr build(QUICPacketType type, QUICConnectionId connection_id, QUICPacketNumber packet_number,
+ QUICPacketNumber base_packet_number, QUICVersion version, ats_unique_buf payload, size_t len);
/*
* Build a QUICPacketHeader
*
* This creates a QUICPacketShortHeader that contains a ConnectionID.
*/
- static QUICPacketHeader *build(QUICPacketType type, QUICKeyPhase key_phase, QUICPacketNumber packet_number,
- QUICPacketNumber base_packet_number, ats_unique_buf payload, size_t len);
+ static QUICPacketHeaderUPtr build(QUICPacketType type, QUICKeyPhase key_phase, QUICPacketNumber packet_number,
+ QUICPacketNumber base_packet_number, ats_unique_buf payload, size_t len);
/*
* Build a QUICPacketHeader
*
* This creates a QUICPacketShortHeader that doesn't contain a ConnectionID..
*/
- static QUICPacketHeader *build(QUICPacketType type, QUICKeyPhase key_phase, QUICConnectionId connection_id,
- QUICPacketNumber packet_number, QUICPacketNumber base_packet_number, ats_unique_buf payload,
- size_t len);
+ static QUICPacketHeaderUPtr build(QUICPacketType type, QUICKeyPhase key_phase, QUICConnectionId connection_id,
+ QUICPacketNumber packet_number, QUICPacketNumber base_packet_number, ats_unique_buf payload,
+ size_t len);
protected:
QUICPacketHeader(){};
@@ -188,10 +200,36 @@ private:
QUICPacketShortHeaderType _packet_number_type = QUICPacketShortHeaderType::UNINITIALIZED;
};
+class QUICPacketHeaderDeleter
+{
+public:
+ static void
+ delete_null_header(QUICPacketHeader *header)
+ {
+ ink_assert(header == nullptr);
+ }
+
+ static void
+ delete_long_header(QUICPacketHeader *header)
+ {
+ QUICPacketLongHeader *long_header = dynamic_cast<QUICPacketLongHeader *>(header);
+ ink_assert(long_header != nullptr);
+ quicPacketLongHeaderAllocator.free(long_header);
+ }
+
+ static void
+ delete_short_header(QUICPacketHeader *header)
+ {
+ QUICPacketShortHeader *short_header = dynamic_cast<QUICPacketShortHeader *>(header);
+ ink_assert(short_header != nullptr);
+ quicPacketShortHeaderAllocator.free(short_header);
+ }
+};
+
class QUICPacket
{
public:
- QUICPacket(){};
+ QUICPacket();
/*
* Creates a QUICPacket with a QUICPacketHeader and a buffer that contains payload
@@ -199,7 +237,7 @@ public:
* This will be used for receiving packets. Therefore, it is expected that payload is already decrypted.
* However, QUICPacket class itself doesn't care about whether the payload is protected (encrypted) or not.
*/
- QUICPacket(QUICPacketHeader *header, ats_unique_buf payload, size_t payload_len);
+ QUICPacket(QUICPacketHeaderUPtr header, ats_unique_buf payload, size_t payload_len);
/*
* Creates a QUICPacket with a QUICPacketHeader, a buffer that contains payload and a flag that indicates whether the packet is
@@ -208,7 +246,7 @@ public:
* This will be used for sending packets. Therefore, it is expected that payload is already encrypted.
* However, QUICPacket class itself doesn't care about whether the payload is protected (encrypted) or not.
*/
- QUICPacket(QUICPacketHeader *header, ats_unique_buf payload, size_t payload_len, bool retransmittable);
+ QUICPacket(QUICPacketHeaderUPtr header, ats_unique_buf payload, size_t payload_len, bool retransmittable);
~QUICPacket();
@@ -249,10 +287,10 @@ public:
LINK(QUICPacket, link);
private:
- QUICPacketHeader *_header;
- ats_unique_buf _payload = ats_unique_buf(nullptr, [](void *p) { ats_free(p); });
- size_t _payload_size = 0;
- bool _is_retransmittable = false;
+ QUICPacketHeaderUPtr _header = QUICPacketHeaderUPtr(nullptr, &QUICPacketHeaderDeleter::delete_null_header);
+ ats_unique_buf _payload = ats_unique_buf(nullptr, [](void *p) { ats_free(p); });
+ size_t _payload_size = 0;
+ bool _is_retransmittable = false;
};
class QUICPacketNumberGenerator
@@ -269,10 +307,6 @@ private:
using QUICPacketDeleterFunc = void (*)(QUICPacket *p);
using QUICPacketUPtr = std::unique_ptr<QUICPacket, QUICPacketDeleterFunc>;
-extern ClassAllocator<QUICPacket> quicPacketAllocator;
-extern ClassAllocator<QUICPacketLongHeader> quicPacketLongHeaderAllocator;
-extern ClassAllocator<QUICPacketShortHeader> quicPacketShortHeaderAllocator;
-
class QUICPacketDeleter
{
public:
@@ -313,6 +347,6 @@ private:
QUICHandshakeProtocol *_hs_protocol = nullptr;
QUICPacketNumberGenerator _packet_number_generator;
- static QUICPacketUPtr _create_unprotected_packet(QUICPacketHeader *header);
- QUICPacketUPtr _create_encrypted_packet(QUICPacketHeader *header, bool retransmittable);
+ static QUICPacketUPtr _create_unprotected_packet(QUICPacketHeaderUPtr header);
+ QUICPacketUPtr _create_encrypted_packet(QUICPacketHeaderUPtr header, bool retransmittable);
};
--
To stop receiving notification emails like this one, please contact
scw00@apache.org.