From 8927f58b5316d9130eff53fe7fd964c04dac077f Mon Sep 17 00:00:00 2001
From: Timo Beckers <timo@incline.eu>
Date: Tue, 19 May 2020 19:24:05 +0200
Subject: [PATCH] perf - reader.go: document possible garbage read from perf
 ring, add reproducer

A patch for this was submitted upstream:
https://lore.kernel.org/patchwork/patch/1244339.

Promises about the buffer's alignment were also removed because they were
inacurrate. The `perf_event_open` man page is incorrect, and there's
another bug in the kernel that causes these buffers to be unaligned.
---
 perf/reader.go      | 11 +++++-----
 perf/reader_test.go | 49 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/perf/reader.go b/perf/reader.go
index 652c622e5..151cc4a6c 100644
--- a/perf/reader.go
+++ b/perf/reader.go
@@ -59,9 +59,8 @@ type Record struct {
 	CPU int
 
 	// The data submitted via bpf_perf_event_output.
-	// They are padded with 0 to have a 64-bit alignment.
-	// If you are using variable length samples you need to take
-	// this into account.
+	// Due to a kernel bug, this can contain between 0 and 7 bytes of trailing
+	// garbage from the ring depending on the input sample's length.
 	RawSample []byte
 
 	// The number of samples which could not be output, since
@@ -317,9 +316,11 @@ func (pr *Reader) Close() error {
 // Read the next record from the perf ring buffer.
 //
 // The function blocks until there are at least Watermark bytes in one
-// of the per CPU buffers.
+// of the per CPU buffers. Records from buffers below the Watermark
+// are not returned.
 //
-// Records from buffers below the Watermark are not returned.
+// Records can contain between 0 and 7 bytes of trailing garbage from the ring
+// depending on the input sample's length.
 //
 // Calling Close interrupts the function.
 func (pr *Reader) Read() (Record, error) {
diff --git a/perf/reader_test.go b/perf/reader_test.go
index 45ccff965..95f5bebf1 100644
--- a/perf/reader_test.go
+++ b/perf/reader_test.go
@@ -5,6 +5,7 @@ import (
 	"encoding/binary"
 	"fmt"
 	"os"
+	"reflect"
 	"syscall"
 	"testing"
 	"time"
@@ -390,6 +391,54 @@ func TestPause(t *testing.T) {
 	}
 }
 
+func TestPerfReaderGarbage(t *testing.T) {
+	// This test reproduces a kernel bug that causes garbage reads from the
+	// perf ring as soon as the ring's page boundary has wrapped for the first
+	// time. Sending a 61-byte (unaligned) payload will result in 80 bytes
+	// written to the perf ring:
+	// - perf event header (8 bytes)
+	// - payload (61 + 4 incorrectly added) = 65, aligned to 72
+
+	// The 80 bytes total is already aligned to 64 bits, so won't have any
+	// extra alignment padding when written to the ring. This should cause
+	// the 52nd write to a 4096-byte ring (4096/80 = 51.2) to wrap the page
+	// and return 7 `align(61+4)-4 - 61 = 7` bytes of garbage.
+
+	// This issue might be fixed in later versions of the kernel.
+	sl := 61
+
+	prog, events := mustOutputSamplesProg(t, sl)
+	defer prog.Close()
+	defer events.Close()
+
+	rd, err := NewReader(events, 4096)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer rd.Close()
+
+	for i := 1; i <= 100; i++ {
+		if _, _, err := prog.Test(make([]byte, 14)); err != nil {
+			testutils.SkipIfNotSupported(t, err)
+			t.Fatal(err)
+		}
+
+		record, err := rd.Read()
+		if err != nil {
+			t.Fatal("Can't read samples:", err)
+		}
+
+		// Maintain a slice of zeroes the length of the padding section.
+		nb := make([]byte, len(record.RawSample)-sl)
+
+		// Compare the padding section to the empty slice.
+		if !reflect.DeepEqual(record.RawSample[sl:], nb) {
+			t.Skipf("%d non-zero bytes found in padding section of perf sample %d: %v", len(nb), i, record.RawSample[sl:])
+			break
+		}
+	}
+}
+
 func BenchmarkReader(b *testing.B) {
 	prog, events := mustOutputSamplesProg(b, 80)
 	defer prog.Close()