From 436148d68a50e086ae7b967d6a190b3137e68ac8 Mon Sep 17 00:00:00 2001
From: Ting-Yu Wang <anivia@google.com>
Date: Fri, 14 May 2021 12:47:26 -0700
Subject: Fix panic on consume in a mixed push/consume case

headerOffset() is incorrectly taking account of previous push(), so it thinks
there is more data to consume. This change switches to use pk.reserved as
pivot point.

Reported-by: syzbot+64fef9acd509976f9ce7@syzkaller.appspotmail.com
PiperOrigin-RevId: 373846283
---
 pkg/tcpip/stack/packet_buffer.go      |  2 +-
 pkg/tcpip/stack/packet_buffer_test.go | 31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

(limited to 'pkg/tcpip/stack')

diff --git a/pkg/tcpip/stack/packet_buffer.go b/pkg/tcpip/stack/packet_buffer.go
index e2e073091..01652fbe7 100644
--- a/pkg/tcpip/stack/packet_buffer.go
+++ b/pkg/tcpip/stack/packet_buffer.go
@@ -261,7 +261,7 @@ func (pk *PacketBuffer) consume(typ headerType, size int) (v tcpipbuffer.View, c
 	if h.length > 0 {
 		panic(fmt.Sprintf("consume must not be called twice: type %s", typ))
 	}
-	if pk.headerOffset()+pk.consumed+size > int(pk.buf.Size()) {
+	if pk.reserved+pk.consumed+size > int(pk.buf.Size()) {
 		return nil, false
 	}
 	h.offset = pk.consumed
diff --git a/pkg/tcpip/stack/packet_buffer_test.go b/pkg/tcpip/stack/packet_buffer_test.go
index 1c1aeb950..a8da34992 100644
--- a/pkg/tcpip/stack/packet_buffer_test.go
+++ b/pkg/tcpip/stack/packet_buffer_test.go
@@ -259,6 +259,37 @@ func TestPacketHeaderPushConsumeMixed(t *testing.T) {
 	})
 }
 
+func TestPacketHeaderPushConsumeMixedTooLong(t *testing.T) {
+	link := makeView(10)
+	network := makeView(20)
+	data := makeView(30)
+
+	initData := concatViews(network, data)
+	pk := NewPacketBuffer(PacketBufferOptions{
+		ReserveHeaderBytes: len(link),
+		Data:               buffer.NewViewFromBytes(initData).ToVectorisedView(),
+	})
+
+	// 1. Push link header
+	copy(pk.LinkHeader().Push(len(link)), link)
+
+	checkPacketContents(t, "" /* prefix */, pk, packetContents{
+		link: link,
+		data: initData,
+	})
+
+	// 2. Consume network header, with a number of bytes too large.
+	gotNetwork, ok := pk.NetworkHeader().Consume(len(initData) + 1)
+	if ok {
+		t.Fatalf("pk.NetworkHeader().Consume(%d) = %q, true; want _, false", len(initData)+1, gotNetwork)
+	}
+
+	checkPacketContents(t, "" /* prefix */, pk, packetContents{
+		link: link,
+		data: initData,
+	})
+}
+
 func TestPacketHeaderPushCalledAtMostOnce(t *testing.T) {
 	const headerSize = 10
 
-- 
cgit v1.2.3