Add memory safety guard for QuicFrame
The current memory model for QuicFrame is error-prone in that the owner of an array of QuicFrames is responsible for deleting non-inlined frames. This CL adds a check in debug and asan builds that will ensure that we crash if we accidentally attempt to delete an ACK frame that is owned by the QuicReceivedPacketManager.
Debug-only change
PiperOrigin-RevId: 328649792
Change-Id: I419cec433393d9f978dffc7f4f54e60c5293fc05
diff --git a/quic/core/frames/quic_frame.cc b/quic/core/frames/quic_frame.cc
index 99b0963..7fe322d 100644
--- a/quic/core/frames/quic_frame.cc
+++ b/quic/core/frames/quic_frame.cc
@@ -87,6 +87,16 @@
}
void DeleteFrame(QuicFrame* frame) {
+#if QUIC_FRAME_DEBUG
+ // If the frame is not inlined, check that it can be safely deleted.
+ if (frame->type != PADDING_FRAME && frame->type != MTU_DISCOVERY_FRAME &&
+ frame->type != PING_FRAME && frame->type != MAX_STREAMS_FRAME &&
+ frame->type != STOP_WAITING_FRAME &&
+ frame->type != STREAMS_BLOCKED_FRAME && frame->type != STREAM_FRAME &&
+ frame->type != HANDSHAKE_DONE_FRAME) {
+ CHECK(!frame->delete_forbidden) << *frame;
+ }
+#endif // QUIC_FRAME_DEBUG
switch (frame->type) {
// Frames smaller than a pointer are inlined, so don't need to be deleted.
case PADDING_FRAME:
diff --git a/quic/core/frames/quic_frame.h b/quic/core/frames/quic_frame.h
index b5785b8..6e096e3 100644
--- a/quic/core/frames/quic_frame.h
+++ b/quic/core/frames/quic_frame.h
@@ -36,6 +36,14 @@
#include "net/third_party/quiche/src/quic/platform/api/quic_containers.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_export.h"
+#ifndef QUIC_FRAME_DEBUG
+#if !defined(NDEBUG) || defined(ADDRESS_SANITIZER)
+#define QUIC_FRAME_DEBUG 1
+#else // !defined(NDEBUG) || defined(ADDRESS_SANITIZER)
+#define QUIC_FRAME_DEBUG 0
+#endif // !defined(NDEBUG) || defined(ADDRESS_SANITIZER)
+#endif // QUIC_FRAME_DEBUG
+
namespace quic {
struct QUIC_EXPORT_PRIVATE QuicFrame {
@@ -87,6 +95,10 @@
struct {
QuicFrameType type;
+#if QUIC_FRAME_DEBUG
+ bool delete_forbidden = false;
+#endif // QUIC_FRAME_DEBUG
+
// TODO(wub): These frames can also be inlined without increasing the size
// of QuicFrame: QuicRstStreamFrame, QuicWindowUpdateFrame,
// QuicBlockedFrame, QuicPathResponseFrame, QuicPathChallengeFrame and
diff --git a/quic/core/quic_received_packet_manager.cc b/quic/core/quic_received_packet_manager.cc
index 9cc6c29..539cdce 100644
--- a/quic/core/quic_received_packet_manager.cc
+++ b/quic/core/quic_received_packet_manager.cc
@@ -189,7 +189,13 @@
}
}
+#if QUIC_FRAME_DEBUG
+ QuicFrame frame = QuicFrame(&ack_frame_);
+ frame.delete_forbidden = true;
+ return frame;
+#else // QUIC_FRAME_DEBUG
return QuicFrame(&ack_frame_);
+#endif // QUIC_FRAME_DEBUG
}
void QuicReceivedPacketManager::DontWaitForPacketsBefore(
